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 28, 2025 · 5381112 · 5381112
1 parent b6927d3
commit 5381112
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 10 deletions.
diff --git a/src/gpasswd.c b/src/gpasswd.c
@@ -830,15 +830,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);
@@ -849,7 +850,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
@@ -292,8 +292,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);
@@ -303,7 +304,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);
@@ -327,14 +328,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
@@ -58,6 +58,7 @@ int
 main(int argc, char *argv[])
 {
 	int            err = 0;
+	char           *pass;
 	char           **envp = environ;
 	TERMIO         termio;
 	struct passwd  pwent = {};
@@ -128,8 +129,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 */
@@ -150,7 +151,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
@@ -168,7 +169,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 */