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.

diff --git a/ChangeLog b/ChangeLog
index 76428316452806b3c40f27e002be5e32ce2f3345..b3c499a5bfc10ff21122ce6935bd9952608ebbf8 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,7 +2,14 @@
 
        * modules/pam_sepermit/pam_sepermit.c (check_running): Avoid
        leaking memory and dir handle on realloc failure.
-       (sepermit_unlock) Cast fcntl() and close() calls to void.
+       (sepermit_unlock): Cast fcntl() and close() calls to void.
+
+       * 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.
 
 2011-06-14  Thorsten Kukuk  <kukuk@thkukuk.de>
 

diff --git a/modules/pam_pwhistory/opasswd.c b/modules/pam_pwhistory/opasswd.c
index f045555f6528e8e758a19d243884202dab9a94fa..738483ac960130d7e89a9a6f36ef37cd3bf265dd 100644 (file)
--- 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 7696353fa245f3b7def399ab16ec01fb584f86b7..9e1056b2bbe79b794c5d58472524470fcdf5e45f 100644 (file)
--- 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>