From: Todd C. Miller Date: Sat, 3 Jun 2017 14:43:32 +0000 (-0600) Subject: Instead of hard-coding a check for bash functions in env_should_delete(), X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e1e2162dcf921b0cd1756b88bcdefda5f9aea490;p=sudo Instead of hard-coding a check for bash functions in env_should_delete(), use a "*=()* " pattern in initial_badenv_table[] to match them instead. This allows the user to remove the check via env_delete. --- diff --git a/doc/sudoers.cat b/doc/sudoers.cat index 990b33e7d..e6ddd2db9 100644 --- a/doc/sudoers.cat +++ b/doc/sudoers.cat @@ -130,16 +130,16 @@ DDEESSCCRRIIPPTTIIOONN _e_n_v___c_h_e_c_k and _e_n_v___k_e_e_p options. This is effectively a whitelist for environment variables. Environment variables with a value beginning with () are removed unless both the name and value parts are matched by - _e_n_v___k_e_e_p or _e_n_v___c_h_e_c_k, as they will be interpreted as functions by older - versions of the bbaasshh shell. Prior to version 1.8.11, such variables were - always removed. + _e_n_v___k_e_e_p or _e_n_v___c_h_e_c_k, as they may be interpreted as functions by the + bbaasshh shell. Prior to version 1.8.11, such variables were always removed. If, however, the _e_n_v___r_e_s_e_t option is disabled, any variables not explicitly denied by the _e_n_v___c_h_e_c_k and _e_n_v___d_e_l_e_t_e options are inherited from the invoking process. In this case, _e_n_v___c_h_e_c_k and _e_n_v___d_e_l_e_t_e behave - like a blacklist. Environment variables with a value beginning with () - are always removed, even if they do not match one of the blacklists. - Since it is not possible to blacklist all potentially dangerous + like a blacklist. Prior to version 1.8.21, environment variables with a + value beginning with () were always removed. Beginning with version + 1.8.21, a pattern in _e_n_v___d_e_l_e_t_e is used to match bbaasshh shell functions + instead. Since it is not possible to blacklist all potentially dangerous environment variables, use of the default _e_n_v___r_e_s_e_t behavior is encouraged. @@ -149,13 +149,13 @@ DDEESSCCRRIIPPTTIIOONN By default, environment variables are matched by name. However, if the pattern includes an equal sign (`='), both the variables name and value - must match. For example, an old-style (pre-shellshock) bbaasshh shell - function could be matched as follows: + must match. For example, a bbaasshh shell function could be matched as + follows: - env_keep += "my_func=()*" + env_keep += "BASH_FUNC_my_func%%=()*" - Without the "=()*" suffix, this would not match, as old-style bbaasshh shell - functions are not preserved by default. + Without the "=()*" suffix, this would not match, as bbaasshh shell functions + are not preserved by default. The complete list of environment variables that ssuuddoo allows or denies is contained in the output of "sudo -V" when run as root. Please note that @@ -2817,4 +2817,4 @@ DDIISSCCLLAAIIMMEERR file distributed with ssuuddoo or https://www.sudo.ws/license.html for complete details. -Sudo 1.8.21 June 2, 2017 Sudo 1.8.21 +Sudo 1.8.21 June 3, 2017 Sudo 1.8.21 diff --git a/doc/sudoers.man.in b/doc/sudoers.man.in index 3f097f2ee..76d740163 100644 --- a/doc/sudoers.man.in +++ b/doc/sudoers.man.in @@ -21,7 +21,7 @@ .\" Agency (DARPA) and Air Force Research Laboratory, Air Force .\" Materiel Command, USAF, under agreement number F39502-99-1-0512. .\" -.TH "SUDOERS" "5" "June 2, 2017" "Sudo @PACKAGE_VERSION@" "File Formats Manual" +.TH "SUDOERS" "5" "June 3, 2017" "Sudo @PACKAGE_VERSION@" "File Formats Manual" .nh .if n .ad l .SH "NAME" @@ -326,7 +326,7 @@ are removed unless both the name and value parts are matched by \fIenv_keep\fR or \fIenv_check\fR, -as they will be interpreted as functions by older versions of the +as they may be interpreted as functions by the \fBbash\fR shell. Prior to version 1.8.11, such variables were always removed. @@ -345,9 +345,14 @@ In this case, and \fIenv_delete\fR behave like a blacklist. -Environment variables with a value beginning with +Prior to version 1.8.21, environment variables with a value beginning with \fR()\fR -are always removed, even if they do not match one of the blacklists. +were always removed. +Beginning with version 1.8.21, a pattern in +\fIenv_delete\fR +is used to match +\fBbash\fR +shell functions instead. Since it is not possible to blacklist all potentially dangerous environment variables, use of the default @@ -368,19 +373,19 @@ By default, environment variables are matched by name. However, if the pattern includes an equal sign (\(oq=\&\(cq), both the variables name and value must match. -For example, an old-style (pre-shellshock) +For example, a \fBbash\fR shell function could be matched as follows: .nf .sp .RS 4n -env_keep += "my_func=()*" +env_keep += "BASH_FUNC_my_func%%=()*" .RE .fi .PP Without the \(Lq\fR=()*\fR\(Rq -suffix, this would not match, as old-style +suffix, this would not match, as \fBbash\fR shell functions are not preserved by default. .PP diff --git a/doc/sudoers.mdoc.in b/doc/sudoers.mdoc.in index d3245e2eb..c47ce4d9e 100644 --- a/doc/sudoers.mdoc.in +++ b/doc/sudoers.mdoc.in @@ -19,7 +19,7 @@ .\" Agency (DARPA) and Air Force Research Laboratory, Air Force .\" Materiel Command, USAF, under agreement number F39502-99-1-0512. .\" -.Dd June 2, 2017 +.Dd June 3, 2017 .Dt SUDOERS @mansectform@ .Os Sudo @PACKAGE_VERSION@ .Sh NAME @@ -315,7 +315,7 @@ are removed unless both the name and value parts are matched by .Em env_keep or .Em env_check , -as they will be interpreted as functions by older versions of the +as they may be interpreted as functions by the .Sy bash shell. Prior to version 1.8.11, such variables were always removed. @@ -334,9 +334,14 @@ In this case, and .Em env_delete behave like a blacklist. -Environment variables with a value beginning with +Prior to version 1.8.21, environment variables with a value beginning with .Li () -are always removed, even if they do not match one of the blacklists. +were always removed. +Beginning with version 1.8.21, a pattern in +.Em env_delete +is used to match +.Sy bash +shell functions instead. Since it is not possible to blacklist all potentially dangerous environment variables, use of the default @@ -357,16 +362,16 @@ By default, environment variables are matched by name. However, if the pattern includes an equal sign .Pq Ql =\& , both the variables name and value must match. -For example, an old-style (pre-shellshock) +For example, a .Sy bash shell function could be matched as follows: .Bd -literal -offset 4n -env_keep += "my_func=()*" +env_keep += "BASH_FUNC_my_func%%=()*" .Ed .Pp Without the .Dq Li =()* -suffix, this would not match, as old-style +suffix, this would not match, as .Sy bash shell functions are not preserved by default. .Pp diff --git a/plugins/sudoers/env.c b/plugins/sudoers/env.c index 547639e06..c23706660 100644 --- a/plugins/sudoers/env.c +++ b/plugins/sudoers/env.c @@ -164,8 +164,7 @@ static const char *initial_badenv_table[] = { "PYTHONUSERBASE", /* python, per user site-packages directory */ "RUBYLIB", /* ruby, library load path */ "RUBYOPT", /* ruby, extra command line options */ - "BASH_FUNC_*", /* new-style bash functions */ - "__BASH_FUNC<*", /* new-style bash functions (Apple) */ + "*=()*", /* bash functions */ NULL }; @@ -692,24 +691,14 @@ matches_env_keep(const char *var, bool *full_match) static bool env_should_delete(const char *var) { - const char *cp; int delete_it; bool full_match = false; debug_decl(env_should_delete, SUDOERS_DEBUG_ENV); - /* Skip variables with values beginning with () (bash functions) */ - if ((cp = strchr(var, '=')) != NULL) { - if (strncmp(cp, "=() ", 4) == 0) { - delete_it = true; - goto done; - } - } - delete_it = matches_env_delete(var); if (!delete_it) delete_it = matches_env_check(var, &full_match) == false; -done: sudo_debug_printf(SUDO_DEBUG_INFO, "delete %s: %s", var, delete_it ? "YES" : "NO"); debug_return_bool(delete_it);