pwhistory: fix read of uninitialized data and memory leak when modifying opasswd

The glibc implementation of getline/getdelim does not guarantee a NUL
terminator in lineptr if getline returns failure (-1).  This occurs when
the opasswd file exists but is empty.  Since strdup is called
immediately afterwards, this causes strdup to read uninitialized memory
and possibly buffer overrun / crash.

This also fixes a memory leak which always occurs when reading the last
line of the opasswd file.  Since the strdup is called before checking
the return code from getline, getdelim, or fgets+strlen, it will
duplicate and never free either:
- The last successfully read line (for getline or getdelim)
- Uninitialized data (if the file is empty)
- A 0 byte string (for fgets+strlen)

Fix by always checking the return code of getline, getdelim, or
fgets+strlen before calling strdup.

diff --git a/modules/pam_pwhistory/opasswd.c b/modules/pam_pwhistory/opasswd.c
index e6cf3469781696fd78c4d1dc68e01dac9932257f..813f579c4b2666eff64dafc5dcd9e7a01c9d75b5 100644 (file)
--- a/modules/pam_pwhistory/opasswd.c
+++ b/modules/pam_pwhistory/opasswd.c
@@ -326,6 +326,9 @@ save_old_pass (pam_handle_t *pamh, const char *user, uid_t uid,
        n = strlen (buf);
 #endif /* HAVE_GETLINE / HAVE_GETDELIM */
 
+       if (n < 1)
+         break;
+
        cp = buf;
        save = strdup (buf); /* Copy to write the original data back.  */
        if (save == NULL)
@@ -336,9 +339,6 @@ save_old_pass (pam_handle_t *pamh, const char *user, uid_t uid,
            goto error_opasswd;
           }
 
-       if (n < 1)
-         break;
-
        tmp = strchr (cp, '#');  /* remove comments */
        if (tmp)
          *tmp = '\0';