isa/sudo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add explicit support for matching the full environment string

Browse Source 
(name=value).  Bash functions may now be preserved for full matches,
but not for name-only matches.
This commit is contained in:
Todd C. Miller
2014-08-06 16:45:57 -06:00
parent 84fa5a505c
commit df0fd41530
5 changed files with 149 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
NEWS
View File

@@ -32,6 +32,11 @@ What's new in Sudo 1.8.11
* Sudo and its associated programs now link against a shared version
of libsudo_util.
* It is now possible to match an environment variable's value as
well as its name using env_keep and env_check. This can be used
to preserve bash functions which would otherwise be removed from
the environment.
What's new in Sudo 1.8.10p3?
* Fixed expansion of %p in the prompt for "sudo -l" when rootpw,

28
doc/sudoers.cat
View File

@@ -122,19 +122,31 @@ DDEESSCCRRIIPPTTIIOONN
PATH, HOME, MAIL, SHELL, LOGNAME, USER, USERNAME and SUDO_* variables in
addition to variables from the invoking process permitted by the
_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. 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 could be interpreted as bbaasshh functions.
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. 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
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
environment variables, use of the default _e_n_v___r_e_s_e_t behavior is
encouraged.
In all cases, environment variables with a value beginning with () are
removed as they could be interpreted as bbaasshh functions. The list of
environment variables that ssuuddoo allows or denies is contained in the
output of ``sudo -V'' when run as root.
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, a bbaasshh function could be matched as follows:
env_keep += "my_func=()*"
Without the ``=()*'' suffix, this would not match, as bbaasshh functions are
not preserved by default.
The list of environment variables that ssuuddoo allows or denies is contained
in the output of ``sudo -V'' when run as root.
Note that the dynamic linker on most operating systems will remove
variables that can control dynamic linking from the environment of setuid
@@ -2331,4 +2343,4 @@ DDIISSCCLLAAIIMMEERR
file distributed with ssuuddoo or http://www.sudo.ws/sudo/license.html for
complete details.
Sudo 1.8.11 July 16, 2014 Sudo 1.8.11
Sudo 1.8.11b1 July 16, 2014 Sudo 1.8.11b1

36
doc/sudoers.man.in
View File

@@ -304,6 +304,16 @@ and
options.
This is effectively a whitelist
for environment variables.
Environment variables with a value beginning with
\fR()\fR
are removed unless both the name and value parts are matched by
\fIenv_keep\fR
or
\fIenv_check\fR,
as they could be interpreted as
\fBbash\fR
functions.
Prior to version 1.8.11, such variables were always removed.
.PP
If, however, the
\fIenv_reset\fR
@@ -319,17 +329,35 @@ In this case,
and
\fIenv_delete\fR
behave like a blacklist.
Environment variables with a value beginning with
\fR()\fR
are always removed, even if they do not match one of the blacklists.
Since it is not possible
to blacklist all potentially dangerous environment variables, use
of the default
\fIenv_reset\fR
behavior is encouraged.
.PP
In all cases, environment variables with a value beginning with
\fR()\fR
are removed as they could be interpreted as
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, a
\fBbash\fR
functions.
function could be matched as follows:
.nf
.sp
.RS 4n
env_keep += "my_func=()*"
.RE
.fi
.PP
Without the
\(lq\fR=()*\fR\(rq
suffix, this would not match, as
\fBbash\fR
functions are not preserved by default.
.PP
The list of environment variables that
\fBsudo\fR
allows or denies is

33
doc/sudoers.mdoc.in
View File

@@ -293,6 +293,16 @@ and
options.
This is effectively a whitelist
for environment variables.
Environment variables with a value beginning with
.Li ()
are removed unless both the name and value parts are matched by
.Em env_keep
or
.Em env_check ,
as they could be interpreted as
.Sy bash
functions.
Prior to version 1.8.11, such variables were always removed.
.Pp
If, however, the
.Em env_reset
@@ -308,17 +318,32 @@ In this case,
and
.Em env_delete
behave like a blacklist.
Environment variables with a value beginning with
.Li ()
are always removed, even if they do not match one of the blacklists.
Since it is not possible
to blacklist all potentially dangerous environment variables, use
of the default
.Em env_reset
behavior is encouraged.
.Pp
In all cases, environment variables with a value beginning with
.Li ()
are removed as they could be interpreted as
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, a
.Sy bash
functions.
function could be matched as follows:
.Bd -literal -offset 4n
env_keep += "my_func=()*"
.Ed
.Pp
Without the
.Dq Li =()*
suffix, this would not match, as
.Sy bash
functions are not preserved by default.
.Pp
The list of environment variables that
.Nm sudo
allows or denies is

126
plugins/sudoers/env.c
View File

@@ -536,29 +536,32 @@ sudo_getenv(const char *name)
}
/*
* Check the env_delete blacklist.
* Check for var against patterns in the specified environment list.
* Returns true if the variable was found, else false.
*/
static bool
matches_env_delete(const char *var)
matches_env_list(const char *var, struct list_members *list, bool *full_match)
{
struct list_member *cur;
size_t len;
bool iswild;
bool match = false;
debug_decl(matches_env_delete, SUDO_DEBUG_ENV)
debug_decl(matches_env_list, SUDO_DEBUG_ENV)
/* Skip anything listed in env_delete. */
SLIST_FOREACH(cur, &def_env_delete, entries) {
len = strlen(cur->value);
/* Deal with '*' wildcard */
SLIST_FOREACH(cur, list, entries) {
size_t sep_pos, len = strlen(cur->value);
bool iswild = false;
/* Locate position of the '=' separator in var=value. */
sep_pos = strcspn(var, "=");
/* Deal with '*' wildcard at the end of the pattern. */
if (cur->value[len - 1] == '*') {
len--;
iswild = true;
} else
iswild = false;
}
if (strncmp(cur->value, var, len) == 0 &&
(iswild || var[len] == '=')) {
(iswild || len == sep_pos || var[len] == '\0')) {
/* If we matched past the '=', count as a full match. */
*full_match = len > sep_pos + 1;
match = true;
break;
}
@@ -566,33 +569,36 @@ matches_env_delete(const char *var)
debug_return_bool(match);
}
/*
* Check the env_delete blacklist.
* Returns true if the variable was found, else false.
*/
static bool
matches_env_delete(const char *var)
{
bool full_match; /* unused */
debug_decl(matches_env_delete, SUDO_DEBUG_ENV)
/* Skip anything listed in env_delete. */
debug_return_bool(matches_env_list(var, &def_env_delete, &full_match));
}
/*
* Apply the env_check list.
* Returns true if the variable is allowed, false if denied
* or -1 if no match.
*/
static int
matches_env_check(const char *var)
matches_env_check(const char *var, bool *full_match)
{
struct list_member *cur;
size_t len;
bool iswild;
int keepit = -1;
debug_decl(matches_env_check, SUDO_DEBUG_ENV)
SLIST_FOREACH(cur, &def_env_check, entries) {
len = strlen(cur->value);
/* Deal with '*' wildcard */
if (cur->value[len - 1] == '*') {
len--;
iswild = true;
} else
iswild = false;
if (strncmp(cur->value, var, len) == 0 &&
(iswild || var[len] == '=')) {
keepit = !strpbrk(var, "/%");
break;
}
/* Skip anything listed in env_check that includes '/' or '%'. */
if (matches_env_list(var, &def_env_check, full_match)) {
const char *val = strchr(var, '=');
if (val != NULL)
keepit = !strpbrk(++val, "/%");
}
debug_return_bool(keepit);
}
@@ -602,34 +608,17 @@ matches_env_check(const char *var)
* Returns true if the variable is allowed else false.
*/
static bool
matches_env_keep(const char *var)
matches_env_keep(const char *var, bool *full_match)
{
struct list_member *cur;
size_t len;
bool iswild, keepit = false;
bool keepit = false;
debug_decl(matches_env_keep, SUDO_DEBUG_ENV)
/* Preserve SHELL variable for "sudo -s". */
if (ISSET(sudo_mode, MODE_SHELL) && strncmp(var, "SHELL=", 6) == 0) {
keepit = true;
goto done;
}
SLIST_FOREACH(cur, &def_env_keep, entries) {
len = strlen(cur->value);
/* Deal with '*' wildcard */
if (cur->value[len - 1] == '*') {
len--;
iswild = true;
} else
iswild = false;
if (strncmp(cur->value, var, len) == 0 &&
(iswild || var[len] == '=')) {
} else if (matches_env_list(var, &def_env_keep, full_match)) {
keepit = true;
break;
}
}
done:
debug_return_bool(keepit);
}
@@ -640,13 +629,24 @@ done:
static bool
env_should_delete(const char *var)
{
const char *cp;
int delete_it;
bool full_match = false;
debug_decl(env_should_delete, SUDO_DEBUG_ENV);
/* Skip variables with values beginning with () (bash functions) */
if ((cp = strchr(var, '=')) != NULL) {
if (strncmp(cp, "=() ", 3) == 0) {
delete_it = true;
goto done;
}
}
delete_it = matches_env_delete(var);
if (!delete_it)
delete_it = matches_env_check(var) == false;
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);
@@ -660,12 +660,21 @@ static bool
env_should_keep(const char *var)
{
int keepit;
bool full_match = false;
const char *cp;
debug_decl(env_should_keep, SUDO_DEBUG_ENV)
keepit = matches_env_check(var);
keepit = matches_env_check(var, &full_match);
if (keepit == -1)
keepit = matches_env_keep(var);
keepit = matches_env_keep(var, &full_match);
/* Skip bash functions unless we matched on the value as well as name. */
if (keepit && !full_match) {
if ((cp = strchr(var, '=')) != NULL) {
if (strncmp(cp, "=() ", 3) == 0)
keepit = false;
}
}
sudo_debug_printf(SUDO_DEBUG_INFO, "keep %s: %s",
var, keepit ? "YES" : "NO");
debug_return_bool(keepit == true);
@@ -685,6 +694,7 @@ env_merge(char * const envp[])
debug_decl(env_merge, SUDO_DEBUG_ENV)
for (ep = envp; *ep != NULL; ep++) {
/* XXX - avoid checking value here too */
if (sudo_putenv(*ep, true, !env_should_keep(*ep)) == -1) {
/* XXX cannot undo on failure */
rval = false;
@@ -800,12 +810,6 @@ rebuild_env(void)
for (ep = old_envp; *ep; ep++) {
bool keepit;
/* Skip variables with values beginning with () (bash functions) */
if ((cp = strchr(*ep, '=')) != NULL) {
if (strncmp(cp, "=() ", 3) == 0)
continue;
}
/*
* Look up the variable in the env_check and env_keep lists.
*/
@@ -880,12 +884,6 @@ rebuild_env(void)
* env_check.
*/
for (ep = old_envp; *ep; ep++) {
/* Skip variables with values beginning with () (bash functions) */
if ((cp = strchr(*ep, '=')) != NULL) {
if (strncmp(cp, "=() ", 3) == 0)
continue;
}
/* Add variable unless it matches a black list. */
if (!env_should_delete(*ep)) {
if (strncmp(*ep, "SUDO_PS1=", 9) == 0)