From e8f5cd4ad89acb19a5be34ea24ab56857528a59c Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Thu, 1 Feb 2007 21:54:58 +0000 Subject: Relevant BUGIDs: Purpose of commit: bugfix Commit summary: --------------- 2007-02-01 Tomas Mraz * xtests/tst-pam_unix3.c: Fix typos in comments. * modules/pam_unix/support.c (_unix_verify_password): Explicitly disallow '!' in the beginning of password hash. Treat only 13 bytes password hash specifically. (Suggested by Solar Designer.) Fix a warning and test for allocation failure. * modules/pam_unix/unix_chkpwd.c (_unix_verify_password): Likewise. --- ChangeLog | 12 +++++++++++- modules/pam_unix/support.c | 12 ++++++------ modules/pam_unix/unix_chkpwd.c | 33 +++++++++++++++++---------------- xtests/tst-pam_unix3.c | 4 ++-- 4 files changed, 36 insertions(+), 25 deletions(-) diff --git a/ChangeLog b/ChangeLog index 62aef4fe..2845afcb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2007-02-01 Tomas Mraz + + * xtests/tst-pam_unix3.c: Fix typos in comments. + + * modules/pam_unix/support.c (_unix_verify_password): Explicitly + disallow '!' in the beginning of password hash. Treat only + 13 bytes password hash specifically. (Suggested by Solar Designer.) + Fix a warning and test for allocation failure. + * modules/pam_unix/unix_chkpwd.c (_unix_verify_password): Likewise. + 2007-01-31 Thorsten Kukuk * xtests/Makefile.am: Add new pam_unix.so tests @@ -21,7 +31,7 @@ * configure.in: Set version number to 0.99.7.1 2007-01-23 Thorsten Kukuk - Tomas Mraz + Tomas Mraz * modules/pam_unix/support.c (_unix_verify_password): Always compare full encrypted passwords (CVE-2007-0003). diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 954f2c73..fc95f2c0 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -679,7 +679,7 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name } } } else { - int salt_len = strlen(salt); + size_t salt_len = strlen(salt); if (!salt_len) { /* the stored password is NULL */ if (off(UNIX__NONULL, ctrl)) {/* this means we've succeeded */ @@ -689,19 +689,19 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name D(("user has empty password - access denied")); retval = PAM_AUTH_ERR; } - } else if (!p || (*salt == '*')) { + } else if (!p || *salt == '*' || *salt == '!') { retval = PAM_AUTH_ERR; } else { if (!strncmp(salt, "$1$", 3)) { pp = Goodcrypt_md5(p, salt); - if (strcmp(pp, salt) != 0) { + if (pp && strcmp(pp, salt) != 0) { _pam_delete(pp); pp = Brokencrypt_md5(p, salt); } } else if (*salt != '$' && salt_len >= 13) { pp = bigcrypt(p, salt); - if (strlen(pp) > salt_len) { - pp[salt_len] = '\0'; + if (pp && salt_len == 13 && strlen(pp) > salt_len) { + _pam_overwrite(pp + salt_len); } } else { /* @@ -715,7 +715,7 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name /* the moment of truth -- do we agree with the password? */ D(("comparing state of pp[%s] and salt[%s]", pp, salt)); - if (strcmp(pp, salt) == 0) { + if (pp && strcmp(pp, salt) == 0) { retval = PAM_SUCCESS; } else { retval = PAM_AUTH_ERR; diff --git a/modules/pam_unix/unix_chkpwd.c b/modules/pam_unix/unix_chkpwd.c index 87d29256..0ef2ccd8 100644 --- a/modules/pam_unix/unix_chkpwd.c +++ b/modules/pam_unix/unix_chkpwd.c @@ -144,7 +144,7 @@ static int _unix_verify_password(const char *name, const char *p, int nullok) char *salt = NULL; char *pp = NULL; int retval = PAM_AUTH_ERR; - int salt_len; + size_t salt_len; /* UNIX passwords area */ setpwent(); @@ -189,6 +189,8 @@ static int _unix_verify_password(const char *name, const char *p, int nullok) return (nullok == 0) ? PAM_AUTH_ERR : PAM_SUCCESS; } if (p == NULL || strlen(p) == 0) { + _pam_overwrite(salt); + _pam_drop(salt); return PAM_AUTHTOK_ERR; } @@ -196,11 +198,13 @@ static int _unix_verify_password(const char *name, const char *p, int nullok) retval = PAM_AUTH_ERR; if (!strncmp(salt, "$1$", 3)) { pp = Goodcrypt_md5(p, salt); - if (strcmp(pp, salt) == 0) { + if (pp && strcmp(pp, salt) == 0) { retval = PAM_SUCCESS; } else { + _pam_overwrite(pp); + _pam_drop(pp); pp = Brokencrypt_md5(p, salt); - if (strcmp(pp, salt) == 0) + if (pp && strcmp(pp, salt) == 0) retval = PAM_SUCCESS; } } else if (*salt == '$') { @@ -209,10 +213,10 @@ static int _unix_verify_password(const char *name, const char *p, int nullok) * libcrypt nows about it? We should try it. */ pp = x_strdup (crypt(p, salt)); - if (strcmp(pp, salt) == 0) { + if (pp && strcmp(pp, salt) == 0) { retval = PAM_SUCCESS; } - } else if ((*salt == '*') || (salt_len < 13)) { + } else if (*salt == '*' || *salt == '!' || salt_len < 13) { retval = PAM_AUTH_ERR; } else { pp = bigcrypt(p, salt); @@ -223,24 +227,21 @@ static int _unix_verify_password(const char *name, const char *p, int nullok) * have been truncated for storage relative to the output * of bigcrypt here. As such we need to compare only the * stored string with the subset of bigcrypt's result. - * Bug 521314: the strncmp comparison is for legacy support. + * Bug 521314. */ - if (strncmp(pp, salt, salt_len) == 0) { + if (salt_len == 13 && strlen(pp) > salt_len) { + _pam_overwrite(pp+salt_len); + } + + if (strcmp(pp, salt) == 0) { retval = PAM_SUCCESS; } } p = NULL; /* no longer needed here */ /* clean up */ - { - char *tp = pp; - if (pp != NULL) { - while (tp && *tp) - *tp++ = '\0'; - free(pp); - } - pp = tp = NULL; - } + _pam_overwrite(pp); + _pam_drop(pp); return retval; } diff --git a/xtests/tst-pam_unix3.c b/xtests/tst-pam_unix3.c index cb3c7c59..bd5ffca4 100644 --- a/xtests/tst-pam_unix3.c +++ b/xtests/tst-pam_unix3.c @@ -70,7 +70,7 @@ fake_conv (int num_msg, const struct pam_message **msgm UNUSED, for (count = 0; count < num_msg; ++count) { reply[count].resp_retcode = 0; - /* first call get a password, second one a too long one */ + /* first call get a password, second one a too short one */ if (in_test == 1) reply[count].resp = strdup ("pamunix01"); else if (in_test == 2) @@ -122,7 +122,7 @@ main(int argc, char *argv[]) return 1; } - /* Try two, second input is too long */ + /* Try two, second input is too short */ in_test = 2; retval = pam_authenticate (pamh, 0); if (retval != PAM_AUTH_ERR) -- cgit v1.2.3