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>

diff --git a/libmisc/loginprompt.c b/libmisc/loginprompt.c
index db1b7313ea34da336cd55390d7fd40436f9ac552..f1efa3f6b260df22e567bbdebb9b7ecd036e74e1 100644 (file)
--- 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,51 +94,15 @@ 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
         */