emo/shadow
1
0
Fork 0
Code Issues Pull Requests Packages Releases Activity

login_prompt: Do not parse environment variables

Browse Source 
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>
This commit is contained in:
Samanta Navarro 2023-04-28 11:55:14 +00:00 committed by Serge Hallyn
parent c0fc4d2122
commit 8fc8de382a
1 changed files with 2 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
libmisc/loginprompt.c
View File

@ -14,7 +14,6 @@
#include <assert.h> #include <assert.h>
#include <stdio.h> #include <stdio.h>
#include <signal.h> #include <signal.h>
#include <ctype.h>
#include "alloc.h" #include "alloc.h"
#include "prototypes.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. * Skip leading whitespace. This makes " username" work right.
* Then copy the rest (up to the end or the first "non-graphic" * Then copy the rest (up to the end) into the username.
* character into the username.
*/ */
for (cp = buf; *cp == ' ' || *cp == '\t'; cp++); for (cp = buf; *cp == ' ' || *cp == '\t'; cp++);
for (i = 0; i < namesize - 1 && isgraph (*cp); name[i++] = *cp++); for (i = 0; i < namesize - 1 && *cp != '\0'; name[i++] = *cp++);
while (isgraph (*cp)) {
cp++;
}
if ('\0' != *cp) {
cp++;
}
name[i] = '\0'; 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 * Set the SIGQUIT handler back to its original value
*/ */