Relevant BUGIDs:

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.

2 files changed, 23 insertions, 22 deletions

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;
 }