The previous method of scrubbing environment variables, scrub_env(), and targeted calls to unsetenv() were insufficient to protect against glibc-based injection attacks, such as the recently reported CVE-1999-0073 regression.
To fix this issue, the approach suggested by Simon Josefsson in <https://lists.gnu.org/archive/html/bug-inetutils/2026-02/msg00002.html> has been taken to replace the reactive blacklist with a fairly strict default whitelist of the following allowed environment variables: USER LOGNAME TERM LANG LC_* The daemon now clears the inherited environment (preserving PATH and TERM, respectively, if present) before calling telnetd_setup(). As suggested by Solar Designer, all whitelisted variables are sanitized. This ensures that the variable will be dropped if its value contains a path separator, or a reference to the current working directory or its parent. * telnetd/telnetd.c (main): Call exorcise_env() after argument parsing. * telnetd/utility.c (is_env_var_allowed): New function. (exorcise_env): New function. Snapshot PATH and TERM, then clearenv(). (getterminaltype): Apply final whitelist validation to terminaltype. (terminaltypeok): Validate terminal type against the whitelist. * telnetd/state.c (suboption): Filter NEW_ENVIRON during parsing using is_env_var_allowed() and discard empty values. * telnetd/pty.c (start_login): Remove the obsolete scrubbing logic. * telnetd/telnetd.h: Add prototypes for new functions. --- Changes in v2: - Added value inspection to the is_env_var_allowed() validation logic. - Implemented sanitization for whitelisted variables to reject path separators, or reference to the current directory or its parent. - Modified getterminaltype() and terminaltypeok() to prevent TELOPT_TTYPE abuse. --- telnetd/pty.c | 32 ---------------- telnetd/state.c | 32 ++++++++++++---- telnetd/telnetd.c | 2 + telnetd/telnetd.h | 3 ++ telnetd/utility.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 40 deletions(-) diff --git a/telnetd/pty.c b/telnetd/pty.c index f3518049..4bf407ad 100644 --- a/telnetd/pty.c +++ b/telnetd/pty.c @@ -83,29 +83,6 @@ startslave (char *host, int autologin, char *autoname) return master; } -/* - * scrub_env() - * - * Remove a few things from the environment that - * don't need to be there. - * - * Security fix included in telnet-95.10.23.NE of David Borman <[email protected]>. - */ -static void -scrub_env (void) -{ - char **cpp, **cpp2; - - for (cpp2 = cpp = environ; *cpp; cpp++) - { - if (strncmp (*cpp, "LD_", 3) - && strncmp (*cpp, "_RLD_", 5) - && strncmp (*cpp, "LIBPATH=", 8) && strncmp (*cpp, "IFS=", 4)) - *cpp2++ = *cpp; - } - *cpp2 = 0; -} - void start_login (char *host, int autologin, char *name) { @@ -117,8 +94,6 @@ start_login (char *host, int autologin, char *name) (void) autologin; (void) name; - scrub_env (); - /* Set the environment variable "LINEMODE" to indicate our linemode */ if (lmodetype == REAL_LINEMODE) setenv ("LINEMODE", "real", 1); @@ -130,13 +105,6 @@ start_login (char *host, int autologin, char *name) fatal (net, "can't expand login command line"); argcv_get (cmd, "", &argc, &argv); - /* util-linux's "login" introduced an authentication bypass method - * via environment variable "CREDENTIALS_DIRECTORY" in version 2.40. - * Clear it from the environment before executing "login" to prevent - * abuse via Telnet. - */ - unsetenv ("CREDENTIALS_DIRECTORY"); - execv (argv[0], argv); syslog (LOG_ERR, "%s: %m\n", cmd); fatalperror (net, cmd); diff --git a/telnetd/state.c b/telnetd/state.c index a9a51e00..2efc65d4 100644 --- a/telnetd/state.c +++ b/telnetd/state.c @@ -1495,10 +1495,18 @@ suboption (void) case NEW_ENV_VAR: case ENV_USERVAR: *cp = '\0'; - if (valp) - setenv (varp, valp, 1); - else - unsetenv (varp); + if (is_env_var_allowed (varp, valp)) + { + if (valp) + { + if (valp && *valp != 0) + setenv (varp, valp, 1); + } + else + { + unsetenv (varp); + } + } cp = varp = (char *) subpointer; valp = 0; break; @@ -1514,10 +1522,18 @@ suboption (void) } } *cp = '\0'; - if (valp) - setenv (varp, valp, 1); - else - unsetenv (varp); + if (is_env_var_allowed (varp, valp)) + { + if (valp) + { + if (valp && *valp != 0) + setenv (varp, valp, 1); + } + else + { + unsetenv (varp); + } + } break; } /* end of case TELOPT_NEW_ENVIRON */ #if defined AUTHENTICATION diff --git a/telnetd/telnetd.c b/telnetd/telnetd.c index 219a19da..affa2c96 100644 --- a/telnetd/telnetd.c +++ b/telnetd/telnetd.c @@ -218,6 +218,8 @@ main (int argc, char **argv) if (argc != index) error (EXIT_FAILURE, 0, "junk arguments in the command line"); + exorcise_env (); + telnetd_setup (0); return telnetd_run (); /* Never returning. */ } diff --git a/telnetd/telnetd.h b/telnetd/telnetd.h index df31a819..5ee59e6e 100644 --- a/telnetd/telnetd.h +++ b/telnetd/telnetd.h @@ -316,6 +316,9 @@ extern void tty_setsofttab (int); extern void tty_tspeed (int); extern char *expand_line (const char *fmt); +extern void exorcise_env (void); +extern int is_env_var_allowed (const char *var, const char *val); + /* FIXME */ extern void _termstat (void); diff --git a/telnetd/utility.c b/telnetd/utility.c index 2fe6730c..cabdcdfd 100644 --- a/telnetd/utility.c +++ b/telnetd/utility.c @@ -17,6 +17,16 @@ along with this program. If not, see `http://www.gnu.org/licenses/'. */ #include <config.h> +#include <fnmatch.h> +#include <string.h> + +#ifdef HAVE_PATHS_H +# include <paths.h> +#else +# ifndef _PATH_DEFPATH +# define _PATH_DEFPATH "/usr/bin:/bin" +# endif +#endif #define TELOPTS #define TELCMDS @@ -65,6 +75,79 @@ static int pcc; extern int not42; +/* A default whitelist for environment variables. */ +static const char *allowed_env_vars[] = { + "USER", + "LOGNAME", + "TERM", + "LANG", + "LC_*", + NULL +}; + +int +is_env_var_allowed (const char *var, const char *val) +{ + const char **p; + int allowed = 0; + + for (p = allowed_env_vars; *p; p++) + { + if (fnmatch (*p, var, FNM_NOESCAPE) == 0) + { + allowed = 1; + break; + } + } + + if (!allowed) + return 0; + + if (val != NULL) + { + if (strchr (val, '/') != NULL) + return 0; + + if (strcmp (val, "..") == 0) + return 0; + + if (strcmp (val, ".") == 0) + return 0; + } + + return 1; +} + +void +exorcise_env (void) +{ + char *path, *term, *value; + + value = getenv ("PATH"); + path = value ? strdup (value) : NULL; + + value = getenv ("TERM"); + term = is_env_var_allowed ("TERM", value) ? strdup (value) : NULL; + + clearenv(); + + if (path) + { + setenv ("PATH", path, 1); + free (path); + } + else + { + setenv ("PATH", _PATH_DEFPATH, 1); + } + + if (term) + { + setenv ("TERM", term, 1); + free (term); + } +} + static int readstream (int p, char *ibuf, int bufsize) { @@ -863,6 +946,16 @@ getterminaltype (char *uname, size_t len) } free (first); free (last); + + /* Does TERM appear to be illogical? */ + if (terminaltype) + { + if (!is_env_var_allowed ("TERM", terminaltype)) + { + free (terminaltype); + terminaltype = NULL; + } + } } return retval; } @@ -876,6 +969,9 @@ getterminaltype (char *uname, size_t len) int terminaltypeok (char *s) { + if (!is_env_var_allowed ("TERM", s)) + return 0; + #ifdef HAVE_TGETENT char buf[2048]; --
