From ff7c230341fc4bd2266b9ddaf39d83683f12c040 Mon Sep 17 00:00:00 2001
From: Tomas Mraz <tmraz@fedoraproject.org>
Date: Wed, 15 Jun 2011 20:48:59 +0200
Subject: Cleanups of pam_pwhistory code. Make opasswd entry parsing more
 robust.

        * modules/pam_pwhistory/opasswd.c (check_old_password): Do not
        needlessly call strdupa().
        (save_old_password): Avoid memleaks in error paths. Avoid memleak of
        buf. Make the opasswd entry parsing more robust.
        * modules/pam_pwhistory/pam_pwhistory.8.xml: Document the
        special meaning of remember=0.
---
 modules/pam_pwhistory/opasswd.c           | 57 ++++++++++++++++++-------------
 modules/pam_pwhistory/pam_pwhistory.8.xml |  4 ++-
 2 files changed, 36 insertions(+), 25 deletions(-)

(limited to 'modules/pam_pwhistory')

diff --git a/modules/pam_pwhistory/opasswd.c b/modules/pam_pwhistory/opasswd.c
index f045555f..738483ac 100644
--- a/modules/pam_pwhistory/opasswd.c
+++ b/modules/pam_pwhistory/opasswd.c
@@ -181,15 +181,13 @@ check_old_password (pam_handle_t *pamh, const char *user,
 
   fclose (oldpf);
 
-  if (found)
+  if (found && entry.old_passwords)
     {
       const char delimiters[] = ",";
       char *running;
       char *oldpass;
 
-      running = strdupa (entry.old_passwords);
-      if (running == NULL)
-	return PAM_BUF_ERR;
+      running = entry.old_passwords;
 
       do {
 	oldpass = strsep (&running, delimiters);
@@ -311,8 +309,12 @@ save_old_password (pam_handle_t *pamh, const char *user, uid_t uid,
 	    buflen = DEFAULT_BUFLEN;
 	    buf = malloc (buflen);
 	    if (buf == NULL)
-	      return PAM_BUF_ERR;
-
+              {
+		fclose (oldpf);
+		fclose (newpf);
+		retval = PAM_BUF_ERR;
+		goto error_opasswd;
+              }
 	  }
 	buf[0] = '\0';
 	fgets (buf, buflen - 1, oldpf);
@@ -322,7 +324,12 @@ save_old_password (pam_handle_t *pamh, const char *user, uid_t uid,
 	cp = buf;
 	save = strdup (buf); /* Copy to write the original data back.  */
 	if (save == NULL)
-	  return PAM_BUF_ERR;
+          {
+	    fclose (oldpf);
+	    fclose (newpf);
+	    retval = PAM_BUF_ERR;
+	    goto error_opasswd;
+          }
 
 	if (n < 1)
 	  break;
@@ -351,31 +358,30 @@ save_old_password (pam_handle_t *pamh, const char *user, uid_t uid,
 		found = 1;
 
 		/* Don't save the current password twice */
-		if (entry.old_passwords)
+		if (entry.old_passwords && entry.old_passwords[0] != '\0')
 		  {
-		    /* there is only one password */
-		    if (strcmp (entry.old_passwords, oldpass) == 0)
-		      goto write_old_data;
-		    else
+		    char *last = entry.old_passwords;
+
+		    cp = entry.old_passwords;
+		    entry.count = 1;  /* Don't believe the count */
+		    while ((cp = strchr (cp, ',')) != NULL)
 		      {
-			/* check last entry */
-			cp = strstr (entry.old_passwords, oldpass);
-
-			if (cp && strcmp (cp, oldpass) == 0)
-			  {  /* the end is the same, check that there
-				is a "," before. */
-			    --cp;
-			    if (*cp == ',')
-			      goto write_old_data;
-			  }
+			entry.count++;
+			last = ++cp;
 		      }
+
+		    /* compare the last password */
+		    if (strcmp (last, oldpass) == 0)
+		      goto write_old_data;
 		  }
+		else
+		  entry.count = 0;
 
 		/* increase count.  */
 		entry.count++;
 
 		/* check that we don't remember to many passwords.  */
-		while (entry.count > howmany)
+		while (entry.count > howmany && entry.count > 1)
 		  {
 		    char *p = strpbrk (entry.old_passwords, ",");
 		    if (p != NULL)
@@ -383,12 +389,13 @@ save_old_password (pam_handle_t *pamh, const char *user, uid_t uid,
 		    entry.count--;
 		  }
 
-		if (entry.old_passwords == NULL)
+		if (entry.count == 1)
 		  {
 		    if (asprintf (&out, "%s:%s:%d:%s\n",
 				  entry.user, entry.uid, entry.count,
 				  oldpass) < 0)
 		      {
+    		        free (save);
 			retval = PAM_AUTHTOK_ERR;
 			fclose (oldpf);
 			fclose (newpf);
@@ -401,6 +408,7 @@ save_old_password (pam_handle_t *pamh, const char *user, uid_t uid,
 				  entry.user, entry.uid, entry.count,
 				  entry.old_passwords, oldpass) < 0)
 		      {
+    		        free (save);
 			retval = PAM_AUTHTOK_ERR;
 			fclose (oldpf);
 			fclose (newpf);
@@ -493,6 +501,7 @@ save_old_password (pam_handle_t *pamh, const char *user, uid_t uid,
   rename (opasswd_tmp, OLD_PASSWORDS_FILE);
  error_opasswd:
   unlink (opasswd_tmp);
+  free (buf);
 
   return retval;
 }
diff --git a/modules/pam_pwhistory/pam_pwhistory.8.xml b/modules/pam_pwhistory/pam_pwhistory.8.xml
index 7696353f..9e1056b2 100644
--- a/modules/pam_pwhistory/pam_pwhistory.8.xml
+++ b/modules/pam_pwhistory/pam_pwhistory.8.xml
@@ -105,7 +105,9 @@
           <para>
             The last <replaceable>N</replaceable> passwords for each
             user are saved in <filename>/etc/security/opasswd</filename>.
-            The default is <emphasis>10</emphasis>.
+            The default is <emphasis>10</emphasis>. Value of
+            <emphasis>0</emphasis> makes the module to keep the existing
+            contents of the <filename>opasswd</filename> file unchanged.
           </para>
         </listitem>
       </varlistentry>
-- 
cgit v1.2.3