login_prompt: Do not parse environment variables

Parsing optional environment variables after a login name is a feature
which is neither documented nor available in util-linux or busybox
login which are other wide spread login utilities used in Linux
distributions as reference.

Removing this feature resolves two issues:

- A memory leak exists if variables without an equal sign are used,
  because set_env creates copies on its own. This could lead to OOM
  situations in privileged part of login or may lead to heap spraying.
- Environment variables are not reset between login attempts. This
  could lead to additional environment variables set for a user who
  never intended to do so.

Proof of Concept on a system with shadow login without PAM and
util-linux agetty:

1. Provoke an invalid login, e.g. user `noone` and password `invalid`.
   This starts shadow login and subsequent inputs are passed through
   the function login_prompt.
2. Provoke an invalid login with environment variables, e.g.
   user `noone HISTFILE=/tmp/owo` and password `invalid`.
3. Log in correctly with user `root`.

Now you can see with `echo $HISTFILE` that `/tmp/owo` has been set for
the root user.

This requires a malicious failed login attempt and a successful login
within the configured login timeout (default 60 seconds).

Signed-off-by: Samanta Navarro <ferivoz@riseup.net>

1 files changed, 2 insertions, 39 deletions

diff --git a/libmisc/loginprompt.c b/libmisc/loginprompt.c
index db1b7313..f1efa3f6 100644
--- a/libmisc/loginprompt.c
+++ b/libmisc/loginprompt.c
@@ -14,7 +14,6 @@
 #include <assert.h>
 #include <stdio.h>
 #include <signal.h>
-#include <ctype.h>
 
 #include "alloc.h"
 #include "prototypes.h"
@@ -95,52 +94,16 @@ void login_prompt (const char *prompt, char *name, int namesize)
 
 	/*
 	 * Skip leading whitespace.  This makes "  username" work right.
-	 * Then copy the rest (up to the end or the first "non-graphic"
-	 * character into the username.
+	 * Then copy the rest (up to the end) into the username.
 	 */
 
 	for (cp = buf; *cp == ' ' || *cp == '\t'; cp++);
 
-	for (i = 0; i < namesize - 1 && isgraph (*cp); name[i++] = *cp++);
-	while (isgraph (*cp)) {
-		cp++;
-	}
-
-	if ('\0' != *cp) {
-		cp++;
-	}
+	for (i = 0; i < namesize - 1 && *cp != '\0'; name[i++] = *cp++);
 
 	name[i] = '\0';
 
 	/*
-	 * This is a disaster, at best.  The user may have entered extra
-	 * environmental variables at the prompt.  There are several ways
-	 * to do this, and I just take the easy way out.
-	 */
-
-	if ('\0' != *cp) {	/* process new variables */
-		char *nvar;
-		int count = 1;
-		int envc;
-
-		for (envc = 0; envc < MAX_ENV; envc++) {
-			nvar = strtok ((0 != envc) ? NULL : cp, " \t,");
-			if (NULL == nvar) {
-				break;
-			}
-			if (strchr (nvar, '=') != NULL) {
-				envp[envc] = nvar;
-			} else {
-				size_t len = strlen (nvar) + 32;
-				envp[envc] = XMALLOCARRAY (len, char);
-				(void) snprintf (envp[envc], len,
-				                 "L%d=%s", count++, nvar);
-			}
-		}
-		set_env (envc, envp);
-	}
-
-	/*
 	 * Set the SIGQUIT handler back to its original value
 	 */