On 2026-03-08 10:05, Solar Designer wrote:
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.
Thanks for clarifying that.
- 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.
I'm leaving this up to the inetutils maintainers to take of, as I won't
have the spare time to contribute towards this going forward.
+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.
You're right. I forgot to drop that when I got rid of exorcise_env().
+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.
That pointer isn't constant.
######################################################################
## ##
## A test with the pointer declaration used in the previous patch ##
## ##
######################################################################
$ cat > p1.c << "EOF"
#include <stdio.h>
static const char * const allowed_env_vars[] = {
"USER", "LOGNAME", "TERM", "LANG", "LC_*", NULL
};
int main(void)
{
const char **p;
for (p = allowed_env_vars; *p; p++)
puts(*p);
putchar('\n');
return 0;
}
EOF
$ cc -o p1 p1.c -Wall -Werror -Wextra -pedantic
p.c: In function ‘main’:
p.c:11:16: error: assignment discards ‘const’ qualifier from pointer
target type [-Werror=discarded-qualifiers]
11 | for (p = allowed_env_vars; *p; p++)
| ^
cc1: all warnings being treated as errors
######################################################################
## ##
## A test with the pointer declaration used in the current patch ##
## ##
######################################################################
$ (echo '9s/\*\*p/\* const \*p/g'; echo ',p'; echo 'wq') | ed -s p.c
#include <stdio.h>
static const char * const allowed_env_vars[] = {
"USER", "LOGNAME", "TERM", "LANG", "LC_*", NULL
};
int main(void)
{
const char * const *p;
for (p = allowed_env_vars; *p; p++)
puts(*p);
putchar('\n');
return 0;
}
$ cc -o p p.c -Wall -Werror -Wextra -pedantic && echo $?
0
######################################################################
## ##
## A test with the pointer declaration that would be constant ##
## ##
######################################################################
$ (echo '9s/p;/ const p;/g'; echo ',p'; echo 'wq') | ed -s p.c
#include <stdio.h>
static const char * const allowed_env_vars[] = {
"USER", "LOGNAME", "TERM", "LANG", "LC_*", NULL
};
int main(void)
{
const char * const * const p;
for (p = allowed_env_vars; *p; p++)
puts(*p);
putchar('\n');
return 0;
}
$ cc -o p p.c -Wall -Werror -Wextra -pedantic
p.c: In function ‘main’:
p.c:11:16: error: assignment of read-only variable ‘p’
11 | for (p = allowed_env_vars; *p; p++)
| ^
p.c:11:41: error: increment of read-only variable ‘p’
11 | for (p = allowed_env_vars; *p; p++)
| ^~
+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)".
My thinking was: if a whitelisted variable was defined but featured
an empty value, then it might be safer to use whatever value it was
already assigned in the environment instead.
But given that RFC 1572 explicitly supports a client sending a
defined variable with no value, then maybe it's best not to second
guess the client and just let them clear it.
