On Fri, 05 May 2023 16:46:51 -0000, Tobias Stoeckmann wrote:
> 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".
This looks OK but I'd like to see an error message if waitpid()
really does fail. How about something like this, which also avoid
needing the extra variable?
- todd
Index: /usr/src/usr.bin/passwd/pwd_check.c
===================================================================
RCS file: /cvs/src/usr.bin/passwd/pwd_check.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 pwd_check.c
--- /usr/src/usr.bin/passwd/pwd_check.c 28 Aug 2021 06:46:49 -0000 1.17
+++ /usr/src/usr.bin/passwd/pwd_check.c 5 May 2023 16:59:20 -0000
@@ -184,8 +184,10 @@ pwd_check(login_cap_t *lc, char *passwor
/* get the return value from the child */
while (waitpid(child, &res, 0) == -1) {
- if (errno != EINTR)
- break;
+ if (errno != EINTR) {
+ warn("waitpid");
+ goto out;
+ }
}
if (WIFEXITED(res) && WEXITSTATUS(res) == 0) {
free(checker);
