Re: Telnetd Vulnerability Report

[Solar Designer <solar () openwall com>]

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