libpam: treat NUL in passwd files correctly

This already implies that the passwd file itself is broken. Yet do not
skip lines by accident due to fgets limitations.

As a positive side effect, arbitrarily long lines and user names are
supported now as well.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>

2 files changed, 22 insertions, 37 deletions

diff --git a/libpam/pam_modutil_check_user.c b/libpam/pam_modutil_check_user.c
index cf1bd1b5..92fc74b1 100644
--- a/libpam/pam_modutil_check_user.c
+++ b/libpam/pam_modutil_check_user.c
@@ -10,22 +10,15 @@ pam_modutil_check_user_in_passwd(pam_handle_t *pamh,
 				 const char *user_name,
 				 const char *file_name)
 {
-	int rc;
-	size_t user_len;
+	int rc, c = EOF;
 	FILE *fp;
-	char line[BUFSIZ];
 
 	/* Validate the user name.  */
-	if ((user_len = strlen(user_name)) == 0) {
+	if (user_name[0] == '\0') {
 		pam_syslog(pamh, LOG_NOTICE, "user name is not valid");
 		return PAM_SERVICE_ERR;
 	}
 
-	if (user_len > sizeof(line) - sizeof(":")) {
-		pam_syslog(pamh, LOG_NOTICE, "user name is too long");
-		return PAM_SERVICE_ERR;
-	}
-
 	if (strchr(user_name, ':') != NULL) {
 		/*
 		 * "root:x" is not a local user name even if the passwd file
@@ -44,48 +37,40 @@ pam_modutil_check_user_in_passwd(pam_handle_t *pamh,
 	}
 
 	/*
-	 * Scan the file using fgets() instead of fgetpwent_r() because
+	 * Scan the file using fgetc() instead of fgetpwent_r() because
 	 * the latter is not flexible enough in handling long lines
 	 * in passwd files.
 	 */
 	rc = PAM_PERM_DENIED;
-	while (fgets(line, sizeof(line), fp) != NULL) {
-		size_t line_len;
-		const char *str;
+	do {
+		const char *p;
 
 		/*
 		 * Does this line start with the user name
 		 * followed by a colon?
 		 */
-		if (strncmp(user_name, line, user_len) == 0 &&
-		    line[user_len] == ':') {
+		for (p = user_name; *p != '\0'; p++) {
+			c = fgetc(fp);
+			if (c == EOF || c == '\n' || c != *p)
+				break;
+		}
+
+		if (c != EOF && c != '\n')
+			c = fgetc(fp);
+
+		if (*p == '\0' && c == ':') {
 			rc = PAM_SUCCESS;
 			/*
 			 * Continue reading the file to avoid timing attacks.
 			 */
 		}
-		/* Has a newline been read?  */
-		line_len = strlen(line);
-		if (line_len < sizeof(line) - 1 ||
-		    line[line_len - 1] == '\n') {
-			/* Yes, continue with the next line.  */
-			continue;
-		}
 
-		/* No, read till the end of this line first.  */
-		while ((str = fgets(line, sizeof(line), fp)) != NULL) {
-			line_len = strlen(line);
-			if (line_len == 0 ||
-			    line[line_len - 1] == '\n') {
-				break;
-			}
-		}
-		if (str == NULL) {
-			/* fgets returned NULL, we are done.  */
-			break;
-		}
+		/* Read till the end of this line.  */
+		while (c != EOF && c != '\n')
+			c = fgetc(fp);
+
 		/* Continue with the next line.  */
-	}
+	} while (c != EOF);
 
 	fclose(fp);
 	return rc;
diff --git a/modules/pam_localuser/tst-pam_localuser-retval.c b/modules/pam_localuser/tst-pam_localuser-retval.c
index 5581cecc..3d2c8a42 100644
--- a/modules/pam_localuser/tst-pam_localuser-retval.c
+++ b/modules/pam_localuser/tst-pam_localuser-retval.c
@@ -55,7 +55,7 @@ main(void)
 	ASSERT_EQ(PAM_SUCCESS,
 		  pam_start_confdir(service_file, name, &conv, ".", &pamh));
 	ASSERT_NE(NULL, pamh);
-	ASSERT_EQ(PAM_SERVICE_ERR, pam_authenticate(pamh, 0));
+	ASSERT_EQ(PAM_PERM_DENIED, pam_authenticate(pamh, 0));
 	ASSERT_EQ(PAM_SUCCESS, pam_end(pamh, 0));
 	pamh = NULL;
 
@@ -105,7 +105,7 @@ main(void)
 	ASSERT_EQ(PAM_SUCCESS,
 		  pam_start_confdir(service_file, name, &conv, ".", &pamh));
 	ASSERT_NE(NULL, pamh);
-	ASSERT_EQ(PAM_SERVICE_ERR, pam_authenticate(pamh, 0));
+	ASSERT_EQ(PAM_PERM_DENIED, pam_authenticate(pamh, 0));
 	ASSERT_EQ(PAM_SUCCESS, pam_end(pamh, 0));
 	pamh = NULL;