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.

1 files changed, 3 insertions, 3 deletions

diff --git a/modules/pam_pwhistory/opasswd.c b/modules/pam_pwhistory/opasswd.c
index e6cf3469..813f579c 100644
--- 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';