passwd: fix error paths and undefined behaviour

[Tobias Stoeckmann]

Hi,

this patch fixes error paths and an undefined behaviour:

In getnewpasswd we increment "tries" every time we try to enter a new
password. The code allows this to be repeated endlessly by defining
passwordtries to be 0 in /etc/login.conf. But unfortunately we even
increment the int "tries" if pwd_tries is 0, which eventually would lead
to a signed integer overflow (and we would stop asking for passwords
after billion times).

In pwd_check we should close pipe file descriptors if fork fails. Also
it might be possible that waitpid fails if the root user tries to
sabotage the own passwd call. Let's just handle the error case here
as well to avoid accessing undefined content of "res".

Okay?


Tobias

Index: local_passwd.c
===================================================================
RCS file: /cvs/src/usr.bin/passwd/local_passwd.c,v
retrieving revision 1.63
diff -u -p -u -p -r1.63 local_passwd.c
--- local_passwd.c      10 Feb 2022 13:06:46 -0000      1.63
+++ local_passwd.c      5 May 2023 16:35:58 -0000
@@ -217,7 +217,7 @@ getnewpasswd(struct passwd *pw, login_ca
                        continue;
                }
 
-               if ((tries++ < pwd_tries || pwd_tries == 0) &&
+               if ((pwd_tries == 0 || tries++ < pwd_tries) &&
                    pwd_check(lc, p) == 0)
                        continue;
                p = readpassphrase("Retype new password:", repeat, 
sizeof(repeat),
Index: pwd_check.c
===================================================================
RCS file: /cvs/src/usr.bin/passwd/pwd_check.c,v
retrieving revision 1.17
diff -u -p -u -p -r1.17 pwd_check.c
--- pwd_check.c 28 Aug 2021 06:46:49 -0000      1.17
+++ pwd_check.c 5 May 2023 16:35:58 -0000
@@ -91,7 +91,7 @@ pwd_check(login_cap_t *lc, char *passwor
        char *checker;
        char *argp[] = { "sh", "-c", NULL, NULL};
        int pipefds[2];
-       pid_t child;
+       pid_t child, c;
        uid_t uid;
        gid_t gid;
 
@@ -114,6 +114,8 @@ pwd_check(login_cap_t *lc, char *passwor
        switch (child = fork()) {
        case -1:
                warn("fork");
+               close(pipefds[0]);
+               close(pipefds[1]);
                goto out;
        case 0:
                (void)signal(SIGINT, SIG_DFL);
@@ -183,11 +185,11 @@ pwd_check(login_cap_t *lc, char *passwor
        }
 
        /* get the return value from the child */
-       while (waitpid(child, &res, 0) == -1) {
+       while ((c = waitpid(child, &res, 0)) == -1) {
                if (errno != EINTR)
                        break;
        }
-       if (WIFEXITED(res) && WEXITSTATUS(res) == 0) {
+       if (c == child && WIFEXITED(res) && WEXITSTATUS(res) == 0) {
                free(checker);
                return (1);
        }