[sssd PR#433][comment] PAM: Multiple certificates on a Smartcard

[fidencio]

URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard

fidencio commented:
"""
So, coverity found some issues:

- **Error: USE_AFTER_FREE (CWE-825): [#def1]**
```
sssd-1.16.1/src/sss_client/pam_sss.c:154: alias: Assigning: "cai" = "cai->next". Now both point to the same storage.
sssd-1.16.1/src/sss_client/pam_sss.c:155: freed_arg: "free_cai" frees "cai".
sssd-1.16.1/src/sss_client/pam_sss.c:145:9: freed_arg: "free" frees parameter "cai".
sssd-1.16.1/src/sss_client/pam_sss.c:154: deref_after_free: Dereferencing freed pointer "cai".
#  152|   
#  153|       if (list != NULL) {
#  154|->         DLIST_FOR_EACH(cai, list) {
#  155|               free_cai(cai);
#  156|           }
```

Here it seems that a DLIST_FOR_EACH_SAFE() should be used.
Something like:
```
diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index b2968634e..e6d559b44 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -148,11 +148,13 @@ static void free_cai(struct cert_auth_info *cai)

static void free_cert_list(struct cert_auth_info *list)
 {
-    struct cert_auth_info *cai;
+    struct cert_auth_info *cai, *iter, *tmp;

if (list != NULL) {
-        DLIST_FOR_EACH(cai, list) {
-            free_cai(cai);
+        DLIST_FOR_EACH_SAFE(cai, iter, list) {
+            tmp = cai;
+            DLIST_REMOVE(list, cai);
+            free_cai(tmp);
         }
     }
 }
```

- **Error: FORWARD_NULL (CWE-476): [#def2]**
```
sssd-1.16.1/src/responder/pam/pamsrv_p11.c:433: var_compare_op: Comparing "pd->authtok" to null implies that "pd->authtok" might be null.
sssd-1.16.1/src/responder/pam/pamsrv_p11.c:461: var_deref_model: Passing null pointer "pd->authtok" to "sss_authtok_get_type", which dereferences it.
sssd-1.16.1/src/util/authtok.c:30:5: deref_parm: Directly dereferencing parameter "tok".
#   28|   enum sss_authtok_type sss_authtok_get_type(struct sss_auth_token *tok)
#   29|   {
#   30|->     return tok->type;
#   31|   }
#   32|   
```

This one could either be solved by a simple check on the called to ensure that tok is not NULL or by the suggested patch below:
```
diff --git a/src/util/authtok.c b/src/util/authtok.c
index c2f78be32..2c5a26ce3 100644
--- a/src/util/authtok.c
+++ b/src/util/authtok.c
@@ -27,6 +27,10 @@ struct sss_auth_token {

enum sss_authtok_type sss_authtok_get_type(struct sss_auth_token *tok)
 {
+    if (tok == NULL) {
+        return SSS_AUTHTOK_TYPE_EMPTY
+    }
+
     return tok->type;
 }
```

The patch above would also avoid a check done as part of the one patches to ensure that `pd->authtok != NULL`
"""

See the full comment at https://github.com/SSSD/sssd/pull/433#issuecomment-341727235