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 <[email protected]>
shadow-maint · Jan 26, 2025 · 6f53186 · 6f53186
1 parent 8d0a2f7
commit 6f53186
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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 */