From 9de67eee2cf8c3024f7bee7393ea762ac7bd09ab Mon Sep 17 00:00:00 2001 From: Matt Cowell Date: Thu, 29 Aug 2019 16:36:35 -0500 Subject: 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. --- modules/pam_pwhistory/opasswd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'modules') 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'; -- cgit v1.2.3