From: Todd C. Miller Date: Fri, 6 Feb 2015 18:01:05 +0000 (-0700) Subject: Sanity check the TZ environment variable by special casing it in X-Git-Tag: SUDO_1_8_12^2~5 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c3c28773f58cf1ea7aa3c4cb8c6eb2072b42b4c7;p=sudo Sanity check the TZ environment variable by special casing it in env_check. The --with-tzdir configure option can be used to specify the zoneinfo directory if configure doesn't find it. --- diff --git a/INSTALL b/INSTALL index e19b08e1f..a13fdd504 100644 --- a/INSTALL +++ b/INSTALL @@ -145,6 +145,16 @@ Directory and file names: /var/db, /var/lib, /var/adm, /usr/adm This directory should not be cleared when the system boots. + --with-tzdir=DIR + Set the directory to the system's time zone data files. This + is only used when sanitizing the TZ environment variable to + allow for fully-qualified paths in TZ. + By default, configure will look for an existing "zoneinfo" + directory in the following locations: + /usr/share /usr/share/lib /usr/lib /etc + If no zoneinfo directory is found, the TZ variable may not + contain a fully-qualified path. + Compilation options: --disable-hardening Disable the use of compiler/linker exploit mitigation options diff --git a/NEWS b/NEWS index 2e74a9533..19633a835 100644 --- a/NEWS +++ b/NEWS @@ -69,6 +69,9 @@ What's new in Sudo 1.8.12 * Fixed two potential crashes when sudo is run with very low resource limits. + * The TZ environment variable is now checked for safety instead + of simply being copied to the environment of the command. + What's new in Sudo 1.8.11p2 * Fixed a bug where dynamic shared objects loaded from a plugin diff --git a/configure b/configure index 5ac327f81..d3666f667 100755 --- a/configure +++ b/configure @@ -889,6 +889,7 @@ with_timedir with_rundir with_vardir with_iologdir +with_tzdir with_sendmail with_sudoers_mode with_sudoers_uid @@ -1686,6 +1687,7 @@ Optional Packages: --with-rundir=DIR path to the sudo time stamp parent dir --with-vardir=DIR path to the sudo var dir --with-iologdir=DIR directory to store sudo I/O log files in + --with-tzdir=DIR path to the time zone data directory --with-sendmail set path to sendmail --without-sendmail do not send mail at all --with-sudoers-mode mode of sudoers file (defaults to 0440) @@ -4828,6 +4830,16 @@ fi +# Check whether --with-tzdir was given. +if test "${with_tzdir+set}" = set; then : + withval=$with_tzdir; case $with_tzdir in + yes) as_fn_error $? "\"must give --with-tzdir an argument.\"" "$LINENO" 5 + ;; +esac +fi + + + # Check whether --with-sendmail was given. if test "${with_sendmail+set}" = set; then : withval=$with_sendmail; case $with_sendmail in @@ -23367,6 +23379,27 @@ EOF { $as_echo "$as_me:${as_lineno-$LINENO}: result: $iolog_dir" >&5 $as_echo "$iolog_dir" >&6; } +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking time zone data directory" >&5 +$as_echo_n "checking time zone data directory... " >&6; } +tzdir="$with_tzdir" +if test -z "$tzdir"; then + tzdir=no + for d in /usr/share /usr/share/lib /usr/lib /etc; do + if test -d "$d/zoneinfo"; then + tzdir="$d/zoneinfo" + break + fi + done +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $tzdir" >&5 +$as_echo "$tzdir" >&6; } +if test "${tzdir}" != "no"; then + cat >>confdefs.h < dnl dnl XXX - should cache values in all cases!!! @@ -82,6 +82,26 @@ else fi ])dnl +dnl +dnl Detect time zone file directory, if any. +dnl +AC_DEFUN([SUDO_TZDIR], [AC_MSG_CHECKING(time zone data directory) +tzdir="$with_tzdir" +if test -z "$tzdir"; then + tzdir=no + for d in /usr/share /usr/share/lib /usr/lib /etc; do + if test -d "$d/zoneinfo"; then + tzdir="$d/zoneinfo" + break + fi + done +fi +AC_MSG_RESULT([$tzdir]) +if test "${tzdir}" != "no"; then + SUDO_DEFINE_UNQUOTED(_PATH_ZONEINFO, "$tzdir") +fi +])dnl + dnl dnl Parent directory for time stamp dir. dnl diff --git a/pathnames.h.in b/pathnames.h.in index e831eee65..003f4f40a 100644 --- a/pathnames.h.in +++ b/pathnames.h.in @@ -179,6 +179,10 @@ # undef _PATH_NETSVC_CONF #endif /* _PATH_NETSVC_CONF */ +#ifndef _PATH_ZONEINFO +# undef _PATH_ZONEINFO +#endif /* _PATH_ZONEINFO */ + /* On AIX, _PATH_BSHELL in paths.h is /usr/bin/bsh but we want /usr/bin/sh */ #ifndef _PATH_SUDO_BSHELL # if defined(_AIX) && defined(HAVE_PATHS_H) diff --git a/plugins/sudoers/env.c b/plugins/sudoers/env.c index 000461435..d2f844f3e 100644 --- a/plugins/sudoers/env.c +++ b/plugins/sudoers/env.c @@ -187,6 +187,7 @@ static const char *initial_checkenv_table[] = { "LC_*", "LINGUAS", "TERM", + "TZ", NULL }; @@ -202,7 +203,6 @@ static const char *initial_keepenv_table[] = { "PATH", "PS1", "PS2", - "TZ", "XAUTHORITY", "XAUTHORIZATION", NULL @@ -587,6 +587,54 @@ matches_env_delete(const char *var) debug_return_bool(matches_env_list(var, &def_env_delete, &full_match)); } +/* + * Sanity-check the TZ environment variable. + * On many systems it is possible to set this to a pathname. + */ +static bool +tz_is_sane(const char *tzval) +{ + const char *cp; + char lastch; + debug_decl(tz_is_sane, SUDOERS_DEBUG_ENV) + + /* tzcode treats a value beginning with a ':' as a path. */ + if (tzval[0] == ':') + tzval++; + + /* Reject fully-qualified TZ that doesn't being with the zoneinfo dir. */ + if (tzval[0] == '/') { +#ifdef _PATH_ZONEINFO + if (strncmp(tzval, _PATH_ZONEINFO, sizeof(_PATH_ZONEINFO) - 1) != 0 || + tzval[sizeof(_PATH_ZONEINFO) - 1] != '/') + debug_return_bool(false); +#else + /* Assume the worst. */ + debug_return_bool(false); +#endif + } + + /* + * Make sure TZ only contains printable non-space characters + * and does not contain a '..' path element. + */ + lastch = '/'; + for (cp = tzval; *cp != '\0'; cp++) { + if (isspace((unsigned char)*cp) || !isprint((unsigned char)*cp)) + debug_return_bool(false); + if (lastch == '/' && cp[0] == '.' && cp[1] == '.' && + (cp[2] == '/' || cp[2] == '\0')) + debug_return_bool(false); + lastch = *cp; + } + + /* Reject extra long TZ values (even if not a path). */ + if ((size_t)(cp - tzval) >= PATH_MAX) + debug_return_bool(false); + + debug_return_bool(true); +} + /* * Apply the env_check list. * Returns true if the variable is allowed, false if denied @@ -600,9 +648,14 @@ matches_env_check(const char *var, bool *full_match) /* 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, "/%"); + if (strncmp(var, "TZ=", 3) == 0) { + /* Special case for TZ */ + keepit = tz_is_sane(var + 3); + } else { + const char *val = strchr(var, '='); + if (val != NULL) + keepit = !strpbrk(++val, "/%"); + } } debug_return_bool(keepit); }