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,10 +148,11 @@ 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, *cai_next;
if (list != NULL) {
- DLIST_FOR_EACH(cai, list) {
+ DLIST_FOR_EACH_SAFE(cai, cai_next, list) {
+ DLIST_REMOVE(list, cai);
free_cai(cai);
}
}
```
- **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
_______________________________________________
sssd-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
