From 6f531866278cdc72056d481dde7b85db861ce687 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Wed, 22 Jan 2025 16:29:25 +0100 Subject: [PATCH] src/: Call passalloca() before a loop Calling passalloca() (which is a wrapper around alloca(3)) in a loop is dangerous, as it can trigger a stack overflow. Instead, allocate the buffer before the loop, and run getpass2() within the loop, which will reuse the buffer. Also, to avoid deallocator mismatches, use `pass = passzero(pass)` in those cases, so that the compiler knows that 'pass' has changed, and we're not using the password after zeroing it; we're only re-using its storage, which is fine. Signed-off-by: Alejandro Colomar --- src/gpasswd.c | 8 +++++--- src/passwd.c | 10 ++++++---- src/sulogin.c | 8 +++++--- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/gpasswd.c b/src/gpasswd.c index 05b035117..19cc9d52f 100644 --- a/src/gpasswd.c +++ b/src/gpasswd.c @@ -28,6 +28,7 @@ #include "groupio.h" #include "nscd.h" #include "pass/getpass2.h" +#include "pass/passalloca.h" #include "pass/passzero.h" #include "prototypes.h" #ifdef SHADOWGRP @@ -831,15 +832,16 @@ static void change_passwd (struct group *gr) */ printf (_("Changing the password for group %s\n"), group); + cp = passalloca(); for (retries = 0; retries < RETRIES; retries++) { - cp = getpassa(_("New Password: ")); + cp = getpass2(cp, _("New Password: ")); if (NULL == cp) { exit (1); } STRTCPY(pass, cp); passzero(cp); - cp = getpassa(_("Re-enter new password: ")); + cp = getpass2(cp, _("Re-enter new password: ")); if (NULL == cp) { MEMZERO(pass); exit (1); @@ -850,7 +852,7 @@ static void change_passwd (struct group *gr) break; } - passzero(cp); + cp = passzero(cp); MEMZERO(pass); if (retries + 1 < RETRIES) { diff --git a/src/passwd.c b/src/passwd.c index bb95b970f..558b2cab1 100644 --- a/src/passwd.c +++ b/src/passwd.c @@ -26,6 +26,7 @@ #include "getdef.h" #include "nscd.h" #include "pass/getpass2.h" +#include "pass/passalloca.h" #include "pass/passzero.h" #include "prototypes.h" #include "pwauth.h" @@ -293,8 +294,9 @@ static int new_password (const struct passwd *pw) } } else { warned = false; + cp = passalloca(); for (i = getdef_num ("PASS_CHANGE_TRIES", 5); i > 0; i--) { - cp = getpassa(_("New password: ")); + cp = getpass2(cp, _("New password: ")); if (NULL == cp) { MEMZERO(orig); MEMZERO(pass); @@ -304,7 +306,7 @@ static int new_password (const struct passwd *pw) warned = false; } ret = STRTCPY (pass, cp); - passzero(cp); + cp = passzero(cp); if (ret == -1) { (void) fputs (_("Password is too long.\n"), stderr); MEMZERO(orig); @@ -328,14 +330,14 @@ static int new_password (const struct passwd *pw) warned = true; continue; } - cp = getpassa(_("Re-enter new password: ")); + cp = getpass2(cp, _("Re-enter new password: ")); if (NULL == cp) { MEMZERO(orig); MEMZERO(pass); return -1; } if (!streq(cp, pass)) { - passzero(cp); + cp = passzero(cp); (void) fputs (_("They don't match; try again.\n"), stderr); } else { passzero(cp); diff --git a/src/sulogin.c b/src/sulogin.c index 6f9ce1379..f1147c5d8 100644 --- a/src/sulogin.c +++ b/src/sulogin.c @@ -22,6 +22,7 @@ #include "defines.h" #include "getdef.h" #include "pass/getpass2.h" +#include "pass/passalloca.h" #include "pass/passzero.h" #include "prototypes.h" #include "pwauth.h" @@ -59,6 +60,7 @@ int main(int argc, char *argv[]) { int err = 0; + char *pass; char **envp = environ; TERMIO termio; struct passwd pwent = {}; @@ -129,8 +131,8 @@ main(int argc, char *argv[]) (void) signal (SIGALRM, catch_signals); /* exit if the timer expires */ (void) alarm (ALARM); /* only wait so long ... */ + pass = passalloca(); do { /* repeatedly get login/password pairs */ - char *pass; const char *prompt; if (pw_entry("root", &pwent) == -1) { /* get entry from password file */ @@ -151,7 +153,7 @@ main(int argc, char *argv[]) "(or give root password for system maintenance):"); /* get a password for root */ - pass = getpassa(prompt); + pass = getpass2(pass, prompt); /* * XXX - can't enter single user mode if root password is @@ -169,7 +171,7 @@ main(int argc, char *argv[]) } done = valid(pass, &pwent); - passzero(pass); + pass = passzero(pass); if (!done) { /* check encrypted passwords ... */ /* ... encrypted passwords did not match */