Relevant BUGIDs:
authorTomas Mraz <tm@t8m.info>
Thu, 1 Feb 2007 21:54:58 +0000 (21:54 +0000)
committerTomas Mraz <tm@t8m.info>
Thu, 1 Feb 2007 21:54:58 +0000 (21:54 +0000)
Purpose of commit: bugfix

Commit summary:
---------------
2007-02-01  Tomas Mraz  <t8m@centrum.cz>

        * 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
modules/pam_unix/support.c
modules/pam_unix/unix_chkpwd.c
xtests/tst-pam_unix3.c

index 62aef4febb641c52d9ae09081598fb3d7d9a5064..2845afcb8c606226128c028e322f88e4eefca31b 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2007-02-01  Tomas Mraz  <t8m@centrum.cz>
+
+       * 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  <kukuk@thkukuk.de>
 
        * 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  <kukuk@thukuk.de>
-           Tomas Mraz  <t2m@centrum.cz>
+           Tomas Mraz  <t8m@centrum.cz>
 
        * modules/pam_unix/support.c (_unix_verify_password): Always
        compare full encrypted passwords (CVE-2007-0003).
index 954f2c73cc178b99043e0c8535778c1ebe8c9632..fc95f2c0eb4b12ab4d0710e05c0385a924679958 100644 (file)
@@ -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;
index 87d292562b04d243818dfa4cf55a874bf01155c2..0ef2ccd8f75bf2f5b571840c20854dcc741c02ab 100644 (file)
@@ -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;
 }
index cb3c7c59b4e8886488036bdeb6aeea556bf01245..bd5ffca4d98327b090ce380716e1e3cc56c3a5dc 100644 (file)
@@ -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)