[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260308080557.GA27619@openwall.com>
Date: Sun, 8 Mar 2026 09:05:57 +0100
From: Solar Designer <solar@...nwall.com>
To: Justin Swartz <justin.swartz@...ingedge.co.za>
Cc: oss-security@...ts.openwall.com, bug-inetutils@....org,
collin.funk1@...il.com, simon@...efsson.org,
auerswal@...x-ag.uni-kl.de, ron.benyizhak@...ebreach.com
Subject: Re: Telnetd Vulnerability Report
On Sun, Mar 08, 2026 at 09:34:22AM +0200, Justin Swartz wrote:
> Based on the feedback provided, the third version of the patch set [1]:
> - Places the strings of the allowed environment variables array into
> the .rodata section.
Actually, the strings would be in .rodata (or after linking, in .text)
with your previous patch version as well. It's the array of pointers
that you're also moving to there now.
> - Discards the --accept-env feature [3], as an inetutils maintainer [2]
> is working on an implementation to extend the allowed environment
> using Gnulib instead.
It sounds like one of you will have to rebase this on the other's work.
> +extern int is_env_var_allowed (const char *var, const char *val);
You shouldn't need to have this one extern now - you can make it static.
> +#ifdef HAVE_PATHS_H
> +# include <paths.h>
> +#else
> +# ifndef _PATH_DEFPATH
> +# define _PATH_DEFPATH "/usr/bin:/bin"
> +# endif
> +#endif
You shouldn't need this anymore.
> +is_env_var_allowed (const char *var, const char *val)
> +{
> + const char * const *p;
This second const here looks wrong as you're changing the value of this
pointer. I suggested this syntax only for the array, where you used it
correctly.
> +void
> +set_env_var_if_allowed (const char *var, const char *val)
> +{
> + if (is_env_var_allowed (var, val))
> + {
> + if (val)
> + {
> + if (*val != 0)
> + setenv (var, val, 1);
> + }
> + else
> + {
> + unsetenv (var);
> + }
> + }
> +}
I doubt it's desired behavior to retain the previous value of the env
var if the new value is an empty string - or is it? If it is not, then
I suggest either dropping the "*val != 0" check or moving it into
"if (val && *val != 0)".
Alexander
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.
