Relevant BUGIDs: 440107

Purpose of commit: bugfix/cleanup

Commit summary:
---------------
Removed superfluous use of static variables in md5 and bigcrypt routines,
bringing us a step closer to thread-safeness.  Eliminated some variable
indirection along the way.

5 files changed, 31 insertions, 30 deletions

diff --git a/modules/pam_unix/bigcrypt.c b/modules/pam_unix/bigcrypt.c
index b1568d6b..6b73f3d2 100644
--- a/modules/pam_unix/bigcrypt.c
+++ b/modules/pam_unix/bigcrypt.c
@@ -25,6 +25,7 @@
  */
 
 #include <string.h>
+#include <stdlib.h>
 #include <security/_pam_macros.h>
 
 char *crypt(const char *key, const char *salt);
@@ -45,7 +46,7 @@ char *bigcrypt(const char *key, const char *salt);
 
 char *bigcrypt(const char *key, const char *salt)
 {
-	static char dec_c2_cryptbuf[CBUF_SIZE];		/* static storage area */
+	char *dec_c2_cryptbuf;
 
 	unsigned long int keylen, n_seg, j;
 	char *cipher_ptr, *plaintext_ptr, *tmp_ptr, *salt_ptr;
@@ -54,6 +55,10 @@ char *bigcrypt(const char *key, const char *salt)
 	D(("called with key='%s', salt='%s'.", key, salt));
 
 	/* reset arrays */
+	dec_c2_cryptbuf = malloc(CBUF_SIZE);
+	if (!dec_c2_cryptbuf) {
+		return NULL;
+	}
 	memset(keybuf, 0, KEYBUF_SIZE + 1);
 	memset(dec_c2_cryptbuf, 0, CBUF_SIZE);
 
diff --git a/modules/pam_unix/md5_crypt.c b/modules/pam_unix/md5_crypt.c
index a7243a2e..53972fcc 100644
--- a/modules/pam_unix/md5_crypt.c
+++ b/modules/pam_unix/md5_crypt.c
@@ -13,6 +13,7 @@
  */
 
 #include <string.h>
+#include <stdlib.h>
 #include "md5.h"
 
 static unsigned char itoa64[] =	/* 0 ... 63 => ascii - 64 */
@@ -37,8 +38,8 @@ char *MD5Name(crypt_md5)(const char *pw, const char *salt)
 	const char *magic = "$1$";
 	/* This string is magic for this algorithm.  Having
 	 * it this way, we can get get better later on */
-	static char passwd[120], *p;
-	static const char *sp, *ep;
+	char *passwd, *p;
+	const char *sp, *ep;
 	unsigned char final[16];
 	int sl, pl, i, j;
 	MD5_CTX ctx, ctx1;
@@ -47,6 +48,10 @@ char *MD5Name(crypt_md5)(const char *pw, const char *salt)
 	/* Refine the Salt first */
 	sp = salt;
 
+	/* TODO: now that we're using malloc'ed memory, get rid of the
+	   strange constant buffer size. */
+	passwd = malloc(120);
+
 	/* If it starts with the magic string, then skip that */
 	if (!strncmp(sp, magic, strlen(magic)))
 		sp += strlen(magic);
diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c
index c85305e4..85c0a44d 100644
--- a/modules/pam_unix/pam_unix_passwd.c
+++ b/modules/pam_unix/pam_unix_passwd.c
@@ -153,7 +153,7 @@ static char *crypt_md5_wrapper(const char *pass_new)
 	char *cp = (char *) result;
 	unsigned char tmp[16];
 	int i;
-	char *x, *e = NULL;
+	char *x = NULL;
 
 	GoodMD5Init(&ctx);
 	gettimeofday(&tv, (struct timezone *) 0);
@@ -171,9 +171,7 @@ static char *crypt_md5_wrapper(const char *pass_new)
 	*cp = '\0';
 
 	/* no longer need cleartext */
-	e = Goodcrypt_md5(pass_new, (const char *) result);
-	x = x_strdup(e);	/* put e in malloc()ed memory */
-	_pam_overwrite(e);	/* clean up */
+	x = Goodcrypt_md5(pass_new, (const char *) result);
 
 	return x;
 }
@@ -227,11 +225,14 @@ static int check_old_password(const char *forwho, const char *newpass)
 			s_npas = strtok(NULL, ":,");
 			s_pas = strtok(NULL, ":,");
 			while (s_pas != NULL) {
-				if (!strcmp(Goodcrypt_md5(newpass, s_pas), s_pas)) {
+				char *md5pass = Goodcrypt_md5(newpass, s_pas);
+				if (!strcmp(md5pass, s_pas)) {
+					_pam_delete(md5pass);
 					retval = PAM_AUTHTOK_ERR;
 					break;
 				}
 				s_pas = strtok(NULL, ":,");
+				_pam_delete(md5pass);
 			}
 			break;
 		}
@@ -287,6 +288,7 @@ static int save_old_password(const char *forwho, const char *oldpass, int howman
 				sprintf(nbuf, "%s:%s:%d:%s\n", s_luser, s_uid, npas, pass);
 			else
 				sprintf(nbuf, "%s:%s:%d:%s,%s\n", s_luser, s_uid, npas, s_pas, pass);
+			_pam_delete(pass);
 			if (fputs(nbuf, pwfile) < 0) {
 				retval = PAM_AUTHTOK_ERR;
 				err = 1;
@@ -308,6 +310,7 @@ static int save_old_password(const char *forwho, const char *oldpass, int howman
 		} else {
 			pass = crypt_md5_wrapper(oldpass);
 			sprintf(nbuf, "%s:%d:1:%s\n", forwho, pwd->pw_uid, pass);
+			_pam_delete(pass);
 			if (fputs(nbuf, pwfile) < 0) {
 				retval = PAM_AUTHTOK_ERR;
 				err = 1;
@@ -928,7 +931,6 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t * pamh, int flags,
 				 * function we truncate the newly entered password
 				 */
 				char *temp = malloc(9);
-				char *e;
 
 				if (temp == NULL) {
 					_log_err(LOG_CRIT, pamh,
@@ -944,19 +946,11 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t * pamh, int flags,
 				temp[8] = '\0';
 
 				/* no longer need cleartext */
-				e = bigcrypt(temp, salt);
-				tpass = x_strdup(e);
+				tpass = bigcrypt(temp, salt);
 
-				_pam_overwrite(e);
 				_pam_delete(temp);	/* tidy up */
 			} else {
-				char *e;
-
-				/* no longer need cleartext */
-				e = bigcrypt(pass_new, salt);
-				tpass = x_strdup(e);
-
-				_pam_overwrite(e);
+				tpass = bigcrypt(pass_new, salt);
 			}
 		}
 
diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c
index 69071408..964d1a46 100644
--- a/modules/pam_unix/support.c
+++ b/modules/pam_unix/support.c
@@ -570,6 +570,7 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name
 			if (!strncmp(salt, "$1$", 3)) {
 				pp = Goodcrypt_md5(p, salt);
 				if (strcmp(pp, salt) != 0) {
+					_pam_delete(pp);
 					pp = Brokencrypt_md5(p, salt);
 				}
 			} else {
@@ -661,7 +662,7 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name
 	if (salt)
 		_pam_delete(salt);
 	if (pp)
-		_pam_overwrite(pp);
+		_pam_delete(pp);
 
 	D(("done [%d].", retval));
 
@@ -682,7 +683,6 @@ int _unix_read_password(pam_handle_t * pamh
 {
 	int authtok_flag;
 	int retval;
-	const char *item;
 	char *token;
 
 	D(("called"));
@@ -704,16 +704,14 @@ int _unix_read_password(pam_handle_t * pamh
 	 */
 
 	if (on(UNIX_TRY_FIRST_PASS, ctrl) || on(UNIX_USE_FIRST_PASS, ctrl)) {
-		retval = pam_get_item(pamh, authtok_flag, (const void **) &item);
+		retval = pam_get_item(pamh, authtok_flag, (const void **) pass);
 		if (retval != PAM_SUCCESS) {
 			/* very strange. */
 			_log_err(LOG_ALERT, pamh
 				 ,"pam_get_item returned error to unix-read-password"
 			    );
 			return retval;
-		} else if (item != NULL) {	/* we have a password! */
-			*pass = item;
-			item = NULL;
+		} else if (*pass != NULL) {	/* we have a password! */
 			return PAM_SUCCESS;
 		} else if (on(UNIX_USE_FIRST_PASS, ctrl)) {
 			return PAM_AUTHTOK_RECOVER_ERR;		/* didn't work */
@@ -812,9 +810,10 @@ int _unix_read_password(pam_handle_t * pamh
 		_pam_delete(token);	/* clean it up */
 		if (retval != PAM_SUCCESS
 		    || (retval = pam_get_item(pamh, authtok_flag
-					      ,(const void **) &item))
+					      ,(const void **) pass))
 		    != PAM_SUCCESS) {
 
+			*pass = NULL;
 			_log_err(LOG_CRIT, pamh, "error manipulating password");
 			return retval;
 
@@ -833,13 +832,10 @@ int _unix_read_password(pam_handle_t * pamh
 			_pam_delete(token);
 			return retval;
 		}
-		item = token;
+		*pass = token;
 		token = NULL;	/* break link to password */
 	}
 
-	*pass = item;
-	item = NULL;		/* break link to password */
-
 	return PAM_SUCCESS;
 }
 
diff --git a/modules/pam_unix/unix_chkpwd.c b/modules/pam_unix/unix_chkpwd.c
index 5b9ed43e..b0509e47 100644
--- a/modules/pam_unix/unix_chkpwd.c
+++ b/modules/pam_unix/unix_chkpwd.c
@@ -155,6 +155,7 @@ static int _unix_verify_password(const char *name, const char *p, int opt)
 		if (pp != NULL) {
 			while (tp && *tp)
 				*tp++ = '\0';
+			free(pp);
 		}
 		pp = tp = NULL;
 	}