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>
This commit is contained in:
Tobias Stoeckmann 2023-12-06 21:42:17 +01:00 committed by Dmitry V. Levin
parent f1ed3f00e2
commit bde5b4c310
2 changed files with 22 additions and 37 deletions

View File

@ -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;

View File

@ -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;