Where
-Infinity
0

On Sun, 8 Mar 2026 03:57:45 +0100, Solar Designer wrote: others have. It may well be that allowing those other env vars by and maybe allowing LANG and LC is desirable for current use cases. LANG and LC were cargo-culted in as they are honoured by OpenSSHd. Separately note that I didn't check the BSDs telnet client (which I think is still present in all BSDs) for being (hopefully not) willing And you could want to check the telnet client in InetUtils, now that we know this package missed telnet[d] security fixes in general. This was CVE-2005-0488 (and CVE-2005-1205 on Windows).

"Certain BSD-based Telnet clients, including those used on Solaris and SuSE Linux, allow remote malicious Telnet servers to read sensitive command."

The daemon now clears the inherited environment (preserving PATH and TERM, respectively, if present) before calling telnetdsetup(). LDPRELOAD=/lib64/libhardenedmalloc.so (although /etc/ld.so.preload is a more reliable way to do this when practical to do it globally).

exorciseenv() and leave the inetd/tcpd supplied environment intact for telnetd (or some site-specific wrapper) to inherit. +++ b/telnetd/state.c @@ -1495,10 +1495,18 @@ suboption (void) case NEWENVVAR: case ENVUSERVAR: ... } / end of case TELOPTNEWENVIRON /

Some code duplication here. Not new with these changes, but could be worth moving to a new function e.g. setenvvarifallowed(). Agreed. I'll implement setenvvarifallowed() instead. +/ A default whitelist for environment variables. / +static const char allowedenvvars[] = { + "USER", + "LOGNAME", + "TERM", + "LANG", + "LC", + NULL +};

Can make not only the strings but also the pointers const:

static const char const allowedenvvars[] = {

so that both may end up in a read-only section. OK, .rodata it is. +int +isenvvarallowed (const char var, const char val) +{ + const char p; + int allowed = 0; + + for (p = allowedenvvars; p; p++) + { + if (fnmatch (p, var, FNMNOESCAPE) == 0) + { + allowed = 1; + break; + } + } + + if (!allowed) + return 0;

You didn't strictly need the "allowed" variable, you could check p after the loop. But maybe it's more readable the way you wrote it. I'll keep "allowed" for the time being, but I don't mind changing it if there's a clearer way to express this logic. My review above isn't in full context - I only looked at the patches.

Contact

SecAlerts Pty Ltd.
132 Wickham Terrace
Fortitude Valley,
QLD 4006, Australia
info@secalerts.co
By using SecAlerts services, you agree to our services end-user license agreement. This website is safeguarded by reCAPTCHA and governed by the Google Privacy Policy and Terms of Service. All names, logos, and brands of products are owned by their respective owners, and any usage of these names, logos, and brands for identification purposes only does not imply endorsement. If you possess any content that requires removal, please get in touch with us.
© 2026 SecAlerts Pty Ltd.
ABN: 70 645 966 203, ACN: 645 966 203