Hi again, Ich schrob: > The problem here is that the hardcoded "keep these at all costs" list > in debian/patches/env.c-safety.diff (the second hunk) prevents that > approach. > > I suggest just dropping that, particularly as all but XAPPLRESDIR, > XFILESEARCHPATH, XUSERFILESEARCHPATH are already listed in either > initial_checkenv_table or initial_keepenv_table. [...]
Two additions:
Bug #523882 would also be positively impacted by the suggested change,
as the behaviour would at least be explainable by the env_keep/env_check
mechanism. The manpage for -i should still be fixed to mention it, but
at least there won't be the totally undocumented hardcoded list anymore.
And since I was already playing with the code, I'm also attaching two
patches, one flat out dropping the list and one adding the 3 remaining
envvars to initial_keepenv_table. Please pick one and replace
env.c-safety.diff by it.
Cheers,
Jan
--
() ascii ribbon campaign - against html e-mail
/\ www.asciiribbon.org - against proprietary attachments
diff -ru sudo-orig//env.c sudo-1.7.4p4//env.c
--- sudo-orig//env.c 2010-10-01 05:21:10.000000000 +0200
+++ sudo-1.7.4p4//env.c 2010-10-01 07:26:28.000000000 +0200
@@ -116,6 +116,8 @@
static const char *initial_badenv_table[] = {
"IFS",
"CDPATH",
+ "SHELLOPTS",
+ "PS4",
"LOCALDOMAIN",
"RES_OPTIONS",
"HOSTALIASES",
diff -ru sudo-orig//sudo.pod sudo-1.7.4p4//sudo.pod
--- sudo-orig//sudo.pod 2010-10-01 05:21:10.000000000 +0200
+++ sudo-1.7.4p4//sudo.pod 2010-10-01 05:13:39.000000000 +0200
@@ -456,8 +456,8 @@
To prevent command spoofing, B<sudo> checks "." and "" (both denoting
current directory) last when searching for a command in the user's
PATH (if one or both are in the PATH). Note, however, that the
-actual C<PATH> environment variable is I<not> modified and is passed
-unchanged to the program that B<sudo> executes.
+C<PATH> environment variable is further modified in Debian because of
+the use of the I<SECURE_PATH> build option.
B<sudo> will check the ownership of its time stamp directory
(F<@timedir@> by default) and ignore the directory's contents if
diff -ru sudo-orig//sudoers.pod sudo-1.7.4p4//sudoers.pod
--- sudo-orig//sudoers.pod 2010-10-01 05:21:10.000000000 +0200
+++ sudo-1.7.4p4//sudoers.pod 2010-10-01 05:13:39.000000000 +0200
@@ -1309,6 +1309,9 @@
=item env_delete
+Not effective due to security issues: only variables listed in
+I<env_keep> or I<env_check> can be passed through B<sudo>!
+
Environment variables to be removed from the user's environment
when the I<env_reset> option is not in effect. The argument may
be a double-quoted, space-separated list or a single value without
@@ -1322,8 +1325,8 @@
=item env_keep
-Environment variables to be preserved in the user's environment
-when the I<env_reset> option is in effect. This allows fine-grained
+Environment variables to be preserved in the user's environment.
+This allows fine-grained
control over the environment B<sudo>-spawned processes will receive.
The argument may be a double-quoted, space-separated list or a
single value without double-quotes. The list can be replaced, added
diff -ru sudo-orig//env.c sudo-1.7.4p4//env.c
--- sudo-orig//env.c 2010-10-01 05:21:10.000000000 +0200
+++ sudo-1.7.4p4//env.c 2010-10-01 05:51:40.000000000 +0200
@@ -116,6 +116,8 @@
static const char *initial_badenv_table[] = {
"IFS",
"CDPATH",
+ "SHELLOPTS",
+ "PS4",
"LOCALDOMAIN",
"RES_OPTIONS",
"HOSTALIASES",
@@ -205,6 +207,9 @@
"TZ",
"XAUTHORITY",
"XAUTHORIZATION",
+ "XAPPLRESDIR",
+ "XFILESEARCHPATH",
+ "XUSERFILESEARCHPATH",
NULL
};
diff -ru sudo-orig//sudo.pod sudo-1.7.4p4//sudo.pod
--- sudo-orig//sudo.pod 2010-10-01 05:21:10.000000000 +0200
+++ sudo-1.7.4p4//sudo.pod 2010-10-01 05:13:39.000000000 +0200
@@ -456,8 +456,8 @@
To prevent command spoofing, B<sudo> checks "." and "" (both denoting
current directory) last when searching for a command in the user's
PATH (if one or both are in the PATH). Note, however, that the
-actual C<PATH> environment variable is I<not> modified and is passed
-unchanged to the program that B<sudo> executes.
+C<PATH> environment variable is further modified in Debian because of
+the use of the I<SECURE_PATH> build option.
B<sudo> will check the ownership of its time stamp directory
(F<@timedir@> by default) and ignore the directory's contents if
diff -ru sudo-orig//sudoers.pod sudo-1.7.4p4//sudoers.pod
--- sudo-orig//sudoers.pod 2010-10-01 05:21:10.000000000 +0200
+++ sudo-1.7.4p4//sudoers.pod 2010-10-01 05:13:39.000000000 +0200
@@ -1309,6 +1309,9 @@
=item env_delete
+Not effective due to security issues: only variables listed in
+I<env_keep> or I<env_check> can be passed through B<sudo>!
+
Environment variables to be removed from the user's environment
when the I<env_reset> option is not in effect. The argument may
be a double-quoted, space-separated list or a single value without
@@ -1322,8 +1325,8 @@
=item env_keep
-Environment variables to be preserved in the user's environment
-when the I<env_reset> option is in effect. This allows fine-grained
+Environment variables to be preserved in the user's environment.
+This allows fine-grained
control over the environment B<sudo>-spawned processes will receive.
The argument may be a double-quoted, space-separated list or a
single value without double-quotes. The list can be replaced, added
signature.asc
Description: Digital signature
