[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fbfd407edbca76995b86ec45e9cf935d@risingedge.co.za>
Date: Sun, 08 Mar 2026 06:05:45 +0200
From: Justin Swartz <justin.swartz@...ingedge.co.za>
To: Solar Designer <solar@...nwall.com>
Cc: oss-security@...ts.openwall.com, bug-inetutils@....org,
ron.benyizhak@...ebreach.com, simon@...efsson.org,
auerswal@...x-ag.uni-kl.de
Subject: Re: Telnetd Vulnerability Report
On Sun, 8 Mar 2026 03:57:45 +0100, Solar Designer wrote:
> I'm not saying you should revise the list in any way - just sharing
> what
> others have. It may well be that allowing those other env vars by
> default is obsolete since use cases for telnet are now more
> specialized,
> 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
> to export arbitrary env vars. The maintainers could want to check
> this.
> 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
> environment variables via the NEW-ENVIRON option with a SEND
> ENV_USERVAR
> command."
I don't mind poking at the InetUtils telnet client once I've done my
best
to leave the telnetd implementation in a better state than I found it
in.
> > The daemon now clears the inherited environment (preserving PATH
> > and TERM, respectively, if present) before calling telnetd_setup().
>
> Inherited from inetd or the like? It's supposed to be trusted input
> and
> env vars in there may be set on purpose, so dropping them is
> unexpected.
> I think e.g. sshd doesn't do that, why would telnetd? Think things
> like
> LD_PRELOAD=/lib64/libhardened_malloc.so (although /etc/ld.so.preload is
> a more reliable way to do this when practical to do it globally).
Yes, inherited from inetd. Your reasoning makes sense, so I'll get rid
of
exorcise_env() 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 NEW_ENV_VAR:
> > case ENV_USERVAR:
...
> > } /* end of case TELOPT_NEW_ENVIRON */
>
> Some code duplication here. Not new with these changes, but could be
> worth moving to a new function e.g. set_env_var_if_allowed().
Agreed. I'll implement set_env_var_if_allowed() instead.
> > +/* A default whitelist for environment variables. */
> > +static const char *allowed_env_vars[] = {
> > + "USER",
> > + "LOGNAME",
> > + "TERM",
> > + "LANG",
> > + "LC_*",
> > + NULL
> > +};
>
> Can make not only the strings but also the pointers const:
>
> static const char * const allowed_env_vars[] = {
>
> so that both may end up in a read-only section.
OK, .rodata it is.
> > +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;
>
> 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.
Thanks for the review, I'll submit a third version of this patch set
later.
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
