From patchwork Sun Jan 29 17:11:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 19053 Received: (qmail 113716 invoked by alias); 29 Jan 2017 17:11:45 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 113603 invoked by uid 89); 29 Jan 2017 17:11:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_NEUTRAL autolearn=no version=3.3.2 spammy=H*F:U*siddhesh, HX-Envelope-From:sk:siddhes, H*f:sk:1485709, 3125 X-HELO: homiemail-a118.g.dreamhost.com From: Siddhesh Poyarekar To: libc-alpha@sourceware.org Cc: fweimer@redhat.com Subject: [PATCH 2/2] Erase GLIBC_TUNABLES for setxid processes when tunables is disabled Date: Sun, 29 Jan 2017 22:41:10 +0530 Message-Id: <1485709870-25804-3-git-send-email-siddhesh@sourceware.org> In-Reply-To: <1485709870-25804-1-git-send-email-siddhesh@sourceware.org> References: <1485709870-25804-1-git-send-email-siddhesh@sourceware.org> A setxid program that uses a glibc with tunables disabled may pass on GLIBC_TUNABLES as is to its child processes. If the child process ends up using a different glibc that has tunables enabled, it will end up getting access to unsafe tunables. To fix this, remove GLIBC_TUNABLES from the environment for setxid process. In addition to this, I'll post a patch for earlier releases (2.24 and older) to add GLIBC_TUNABLES to unsecure_envvars (is unsecure even a word?) so that they too don't end up passing on unsafe tunables. * elf/dl-tunables.c (GLIBC_TUNABLES): Define as TUNABLES_VALSTRING_ENVVAR. (__tunables_init): Call tunables_delete_env. (is_name): Move definition... * elf/dl-tunables.h: ... here. (TUNABLES_VALSTRING_ENVVAR): New macro. (tunables_delete_env): New function. [!HAVE_TUNABLES](__tunables_init): Call it. --- elf/dl-tunables.c | 40 +++++----------------------------------- elf/dl-tunables.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 37 deletions(-) diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index c3d4c24..23208b5 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -31,25 +31,9 @@ #include "dl-tunables.h" #if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring -# define GLIBC_TUNABLES "GLIBC_TUNABLES" +# define GLIBC_TUNABLES TUNABLES_VALSTRING_ENVVAR #endif -/* Compare environment or tunable names, bounded by the name hardcoded in - glibc. */ -static bool -is_name (const char *orig, const char *envname) -{ - for (;*orig != '\0' && *envname != '\0'; envname++, orig++) - if (*orig != *envname) - break; - - /* The ENVNAME is immediately followed by a value. */ - if (*orig == '\0' && *envname == '=') - return true; - else - return false; -} - static char ** get_next_env (char **envp, char **name, size_t *namelen, char **val, char ***prev_envp) @@ -405,24 +389,10 @@ __tunables_init (char **envp) { if (cur->security_level == TUNABLE_SECLEVEL_SXID_ERASE) { - /* Erase the environment variable. */ - char **ep = prev_envp; - - while (*ep != NULL) - { - if (is_name (name, *ep)) - { - char **dp = ep; - - do - dp[0] = dp[1]; - while (*dp++); - } - else - ++ep; - } - /* Reset the iterator so that we read the environment again - from the point we erased. */ + /* Erase the environment variable and then reset the + iterator so that we read the environment again from + the point we erased. */ + tunables_delete_env (name, prev_envp); envp = prev_envp; } diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h index f33adfb..589e22f 100644 --- a/elf/dl-tunables.h +++ b/elf/dl-tunables.h @@ -21,12 +21,57 @@ #ifndef _TUNABLES_H_ #define _TUNABLES_H_ +#define TUNABLES_VALSTRING_ENVVAR "GLIBC_TUNABLES" + +/* Compare environment or tunable names, bounded by the name hardcoded in + glibc. */ +static inline bool +__always_inline +is_name (const char *orig, const char *envname) +{ + for (;*orig != '\0' && *envname != '\0'; envname++, orig++) + if (*orig != *envname) + break; + + /* The ENVNAME is immediately followed by a value. */ + if (*orig == '\0' && *envname == '=') + return true; + else + return false; +} + +static inline void +__always_inline +tunables_delete_env (const char *name, char **envp) +{ + if (envp == NULL) + return; + + while (*envp != NULL) + { + if (is_name (name, *envp)) + { + char **dp = envp; + + do + dp[0] = dp[1]; + while (*dp++); + } + else + ++envp; + } +} + #if !HAVE_TUNABLES static inline void __always_inline -__tunables_init (char **unused __attribute__ ((unused))) +__tunables_init (char **envp) { - /* This is optimized out if tunables are not enabled. */ + /* If tunables is not enabled, we want to make sure that we don't pass on + insecure tunables to child processes of setxid processes, so just drop + GLIBC_TUNABLES from the environment. */ + if (__libc_enable_secure) + tunables_delete_env (TUNABLES_VALSTRING_ENVVAR, envp); } #else