From 1801db3d3fe17df543e721b9fb355e5c882dc6cc Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Mon, 8 Jun 2015 18:04:00 -0700 Subject: [PATCH] Statically linked executables should honor AT_SECURE. Bug: http://b/19647373 Change-Id: I10e7682d9cec26a523f1a3597ca5326c3ca42ebe --- libc/bionic/libc_init_common.cpp | 194 +++++++++++++++++++++++++++++++ libc/bionic/libc_init_common.h | 4 + libc/bionic/libc_init_static.cpp | 1 + linker/Android.mk | 1 - linker/linker.cpp | 85 ++------------ linker/linker_environ.cpp | 191 ------------------------------ linker/linker_environ.h | 44 ------- 7 files changed, 209 insertions(+), 311 deletions(-) delete mode 100644 linker/linker_environ.cpp delete mode 100644 linker/linker_environ.h diff --git a/libc/bionic/libc_init_common.cpp b/libc/bionic/libc_init_common.cpp index 36dc0854b..9b23ece5a 100644 --- a/libc/bionic/libc_init_common.cpp +++ b/libc/bionic/libc_init_common.cpp @@ -30,10 +30,12 @@ #include #include +#include #include #include #include #include +#include #include #include #include @@ -120,6 +122,198 @@ void __libc_init_common(KernelArgumentBlock& args) { __libc_init_vdso(); } +__noreturn static void __early_abort(int line) { + // We can't write to stdout or stderr because we're aborting before we've checked that + // it's safe for us to use those file descriptors. We probably can't strace either, so + // we rely on the fact that if we dereference a low address, either debuggerd or the + // kernel's crash dump will show the fault address. + *reinterpret_cast(line) = 0; + _exit(EXIT_FAILURE); +} + +// Force any of the closed stdin, stdout and stderr to be associated with /dev/null. +static void __nullify_closed_stdio() { + int dev_null = TEMP_FAILURE_RETRY(open("/dev/null", O_RDWR)); + if (dev_null == -1) { + // init won't have /dev/null available, but SELinux provides an equivalent. + dev_null = TEMP_FAILURE_RETRY(open("/sys/fs/selinux/null", O_RDWR)); + } + if (dev_null == -1) { + __early_abort(__LINE__); + } + + // If any of the stdio file descriptors is valid and not associated + // with /dev/null, dup /dev/null to it. + for (int i = 0; i < 3; i++) { + // If it is /dev/null already, we are done. + if (i == dev_null) { + continue; + } + + // Is this fd already open? + int status = TEMP_FAILURE_RETRY(fcntl(i, F_GETFL)); + if (status != -1) { + continue; + } + + // The only error we allow is that the file descriptor does not + // exist, in which case we dup /dev/null to it. + if (errno == EBADF) { + // Try dupping /dev/null to this stdio file descriptor and + // repeat if there is a signal. Note that any errors in closing + // the stdio descriptor are lost. + status = TEMP_FAILURE_RETRY(dup2(dev_null, i)); + if (status == -1) { + __early_abort(__LINE__); + } + } else { + __early_abort(__LINE__); + } + } + + // If /dev/null is not one of the stdio file descriptors, close it. + if (dev_null > 2) { + if (close(dev_null) == -1) { + __early_abort(__LINE__); + } + } +} + +// Check if the environment variable definition at 'envstr' +// starts with '=', and if so return the address of the +// first character after the equal sign. Otherwise return null. +static const char* env_match(const char* envstr, const char* name) { + size_t i = 0; + + while (envstr[i] == name[i] && name[i] != '\0') { + ++i; + } + + if (name[i] == '\0' && envstr[i] == '=') { + return envstr + i + 1; + } + + return nullptr; +} + +static bool __is_valid_environment_variable(const char* name) { + // According to the kernel source, by default the kernel uses 32*PAGE_SIZE + // as the maximum size for an environment variable definition. + const int MAX_ENV_LEN = 32*4096; + + if (name == nullptr) { + return false; + } + + // Parse the string, looking for the first '=' there, and its size. + int pos = 0; + int first_equal_pos = -1; + while (pos < MAX_ENV_LEN) { + if (name[pos] == '\0') { + break; + } + if (name[pos] == '=' && first_equal_pos < 0) { + first_equal_pos = pos; + } + pos++; + } + + // Check that it's smaller than MAX_ENV_LEN (to detect non-zero terminated strings). + if (pos >= MAX_ENV_LEN) { + return false; + } + + // Check that it contains at least one equal sign that is not the first character + if (first_equal_pos < 1) { + return false; + } + + return true; +} + +static bool __is_unsafe_environment_variable(const char* name) { + // None of these should be allowed in setuid programs. + static const char* const UNSAFE_VARIABLE_NAMES[] = { + "GCONV_PATH", + "GETCONF_DIR", + "HOSTALIASES", + "JE_MALLOC_CONF", + "LD_AOUT_LIBRARY_PATH", + "LD_AOUT_PRELOAD", + "LD_AUDIT", + "LD_DEBUG", + "LD_DEBUG_OUTPUT", + "LD_DYNAMIC_WEAK", + "LD_LIBRARY_PATH", + "LD_ORIGIN_PATH", + "LD_PRELOAD", + "LD_PROFILE", + "LD_SHOW_AUXV", + "LD_USE_LOAD_BIAS", + "LOCALDOMAIN", + "LOCPATH", + "MALLOC_CHECK_", + "MALLOC_CONF", + "MALLOC_TRACE", + "NIS_PATH", + "NLSPATH", + "RESOLV_HOST_CONF", + "RES_OPTIONS", + "TMPDIR", + "TZDIR", + nullptr + }; + for (size_t i = 0; UNSAFE_VARIABLE_NAMES[i] != nullptr; ++i) { + if (env_match(name, UNSAFE_VARIABLE_NAMES[i]) != nullptr) { + return true; + } + } + return false; +} + +static void __sanitize_environment_variables(char** env) { + bool is_AT_SECURE = getauxval(AT_SECURE); + char** src = env; + char** dst = env; + for (; src[0] != nullptr; ++src) { + if (!__is_valid_environment_variable(src[0])) { + continue; + } + // Remove various unsafe environment variables if we're loading a setuid program. + if (is_AT_SECURE && __is_unsafe_environment_variable(src[0])) { + continue; + } + dst[0] = src[0]; + ++dst; + } + dst[0] = nullptr; +} + +void __libc_init_AT_SECURE(KernelArgumentBlock& args) { + __libc_auxv = args.auxv; + + // Check that the kernel provided a value for AT_SECURE. + bool found_AT_SECURE = false; + for (ElfW(auxv_t)* v = __libc_auxv; v->a_type != AT_NULL; ++v) { + if (v->a_type == AT_SECURE) { + found_AT_SECURE = true; + break; + } + } + if (!found_AT_SECURE) __early_abort(__LINE__); + + if (getauxval(AT_SECURE)) { + // If this is a setuid/setgid program, close the security hole described in + // ftp://ftp.freebsd.org/pub/FreeBSD/CERT/advisories/FreeBSD-SA-02:23.stdio.asc + __nullify_closed_stdio(); + + __sanitize_environment_variables(args.envp); + } + + // Now the environment has been sanitized, make it available. + environ = args.envp; +} + /* This function will be called during normal program termination * to run the destructors that are listed in the .fini_array section * of the executable, if any. diff --git a/libc/bionic/libc_init_common.h b/libc/bionic/libc_init_common.h index 3032f99c2..673ad5b04 100644 --- a/libc/bionic/libc_init_common.h +++ b/libc/bionic/libc_init_common.h @@ -49,8 +49,12 @@ __LIBC_HIDDEN__ void __libc_fini(void* finit_array); __END_DECLS #if defined(__cplusplus) + class KernelArgumentBlock; __LIBC_HIDDEN__ void __libc_init_common(KernelArgumentBlock& args); + +__LIBC_HIDDEN__ void __libc_init_AT_SECURE(KernelArgumentBlock& args); + #endif #endif diff --git a/libc/bionic/libc_init_static.cpp b/libc/bionic/libc_init_static.cpp index bc11f3d93..7794fbe76 100644 --- a/libc/bionic/libc_init_static.cpp +++ b/libc/bionic/libc_init_static.cpp @@ -91,6 +91,7 @@ __noreturn void __libc_init(void* raw_args, structors_array_t const * const structors) { KernelArgumentBlock args(raw_args); __libc_init_tls(args); + __libc_init_AT_SECURE(args); __libc_init_common(args); apply_gnu_relro(); diff --git a/linker/Android.mk b/linker/Android.mk index e0c79ed84..ff0e9885f 100644 --- a/linker/Android.mk +++ b/linker/Android.mk @@ -9,7 +9,6 @@ LOCAL_SRC_FILES:= \ linker_allocator.cpp \ linker_sdk_versions.cpp \ linker_block_allocator.cpp \ - linker_environ.cpp \ linker_libc_support.c \ linker_memory.cpp \ linker_phdr.cpp \ diff --git a/linker/linker.cpp b/linker/linker.cpp index b1bf09149..143320b26 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -54,14 +54,15 @@ #include "linker.h" #include "linker_block_allocator.h" #include "linker_debug.h" -#include "linker_environ.h" #include "linker_sleb128.h" #include "linker_phdr.h" #include "linker_relocs.h" #include "linker_reloc_iterators.h" #include "ziparchive/zip_archive.h" -// Override macros to use C++ style casts +extern void __libc_init_AT_SECURE(KernelArgumentBlock&); + +// Override macros to use C++ style casts. #undef ELF_ST_TYPE #define ELF_ST_TYPE(x) (static_cast(x) & 0xf) @@ -2405,66 +2406,6 @@ uint32_t soinfo::get_target_sdk_version() const { return local_group_root_->target_sdk_version_; } -/* Force any of the closed stdin, stdout and stderr to be associated with - /dev/null. */ -static int nullify_closed_stdio() { - int dev_null, i, status; - int return_value = 0; - - dev_null = TEMP_FAILURE_RETRY(open("/dev/null", O_RDWR)); - if (dev_null < 0) { - DL_ERR("cannot open /dev/null: %s", strerror(errno)); - return -1; - } - TRACE("[ Opened /dev/null file-descriptor=%d]", dev_null); - - /* If any of the stdio file descriptors is valid and not associated - with /dev/null, dup /dev/null to it. */ - for (i = 0; i < 3; i++) { - /* If it is /dev/null already, we are done. */ - if (i == dev_null) { - continue; - } - - TRACE("[ Nullifying stdio file descriptor %d]", i); - status = TEMP_FAILURE_RETRY(fcntl(i, F_GETFL)); - - /* If file is opened, we are good. */ - if (status != -1) { - continue; - } - - /* The only error we allow is that the file descriptor does not - exist, in which case we dup /dev/null to it. */ - if (errno != EBADF) { - DL_ERR("fcntl failed: %s", strerror(errno)); - return_value = -1; - continue; - } - - /* Try dupping /dev/null to this stdio file descriptor and - repeat if there is a signal. Note that any errors in closing - the stdio descriptor are lost. */ - status = TEMP_FAILURE_RETRY(dup2(dev_null, i)); - if (status < 0) { - DL_ERR("dup2 failed: %s", strerror(errno)); - return_value = -1; - continue; - } - } - - /* If /dev/null is not one of the stdio file descriptors, close it. */ - if (dev_null > 2) { - TRACE("[ Closing /dev/null file-descriptor=%d]", dev_null); - if (close(dev_null) == -1) { - DL_ERR("close failed: %s", strerror(errno)); - return_value = -1; - } - } - - return return_value; -} - bool soinfo::prelink_image() { /* Extract dynamic section */ ElfW(Word) dynamic_flags = 0; @@ -3107,33 +3048,27 @@ static ElfW(Addr) __linker_init_post_relocation(KernelArgumentBlock& args, ElfW( gettimeofday(&t0, 0); #endif - // Initialize environment functions, and get to the ELF aux vectors table. - linker_env_init(args); + // Sanitize the environment. + __libc_init_AT_SECURE(args); // Initialize system properties __system_properties_init(); // may use 'environ' - // If this is a setuid/setgid program, close the security hole described in - // ftp://ftp.freebsd.org/pub/FreeBSD/CERT/advisories/FreeBSD-SA-02:23.stdio.asc - if (get_AT_SECURE()) { - nullify_closed_stdio(); - } - debuggerd_init(); // Get a few environment variables. - const char* LD_DEBUG = linker_env_get("LD_DEBUG"); + const char* LD_DEBUG = getenv("LD_DEBUG"); if (LD_DEBUG != nullptr) { g_ld_debug_verbosity = atoi(LD_DEBUG); } - // Normally, these are cleaned by linker_env_init, but the test + // These should have been sanitized by __libc_init_AT_SECURE, but the test // doesn't cost us anything. const char* ldpath_env = nullptr; const char* ldpreload_env = nullptr; - if (!get_AT_SECURE()) { - ldpath_env = linker_env_get("LD_LIBRARY_PATH"); - ldpreload_env = linker_env_get("LD_PRELOAD"); + if (!getauxval(AT_SECURE)) { + ldpath_env = getenv("LD_LIBRARY_PATH"); + ldpreload_env = getenv("LD_PRELOAD"); } #if !defined(__LP64__) diff --git a/linker/linker_environ.cpp b/linker/linker_environ.cpp deleted file mode 100644 index 3c466a2f0..000000000 --- a/linker/linker_environ.cpp +++ /dev/null @@ -1,191 +0,0 @@ -/* - * Copyright (C) 2010 The Android Open Source Project - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in - * the documentation and/or other materials provided with the - * distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS - * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE - * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, - * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS - * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED - * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, - * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT - * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ - -#include "linker_environ.h" - -#include -#include -#include -#include -#include - -#include "private/KernelArgumentBlock.h" - -static bool _AT_SECURE_value = true; - -bool get_AT_SECURE() { - return _AT_SECURE_value; -} - -static void __init_AT_SECURE(KernelArgumentBlock& args) { - // Check auxv for AT_SECURE first to see if program is setuid, setgid, - // has file caps, or caused a SELinux/AppArmor domain transition. - bool kernel_supplied_AT_SECURE; - _AT_SECURE_value = args.getauxval(AT_SECURE, &kernel_supplied_AT_SECURE); - - // We don't support ancient kernels. - if (!kernel_supplied_AT_SECURE) { - const char* msg = "FATAL: kernel did not supply AT_SECURE\n"; - write(2, msg, strlen(msg)); - exit(EXIT_FAILURE); - } -} - -// Check if the environment variable definition at 'envstr' -// starts with '=', and if so return the address of the -// first character after the equal sign. Otherwise return null. -static const char* env_match(const char* envstr, const char* name) { - size_t i = 0; - - while (envstr[i] == name[i] && name[i] != '\0') { - ++i; - } - - if (name[i] == '\0' && envstr[i] == '=') { - return envstr + i + 1; - } - - return nullptr; -} - -static bool __is_valid_environment_variable(const char* name) { - // According to its sources, the kernel uses 32*PAGE_SIZE by default - // as the maximum size for an env. variable definition. - const int MAX_ENV_LEN = 32*4096; - - if (name == nullptr) { - return false; - } - - // Parse the string, looking for the first '=' there, and its size. - int pos = 0; - int first_equal_pos = -1; - while (pos < MAX_ENV_LEN) { - if (name[pos] == '\0') { - break; - } - if (name[pos] == '=' && first_equal_pos < 0) { - first_equal_pos = pos; - } - pos++; - } - - // Check that it's smaller than MAX_ENV_LEN (to detect non-zero terminated strings). - if (pos >= MAX_ENV_LEN) { - return false; - } - - // Check that it contains at least one equal sign that is not the first character - if (first_equal_pos < 1) { - return false; - } - - return true; -} - -static bool __is_unsafe_environment_variable(const char* name) { - // None of these should be allowed in setuid programs. - static const char* const UNSAFE_VARIABLE_NAMES[] = { - "GCONV_PATH", - "GETCONF_DIR", - "HOSTALIASES", - "JE_MALLOC_CONF", - "LD_AOUT_LIBRARY_PATH", - "LD_AOUT_PRELOAD", - "LD_AUDIT", - "LD_DEBUG", - "LD_DEBUG_OUTPUT", - "LD_DYNAMIC_WEAK", - "LD_LIBRARY_PATH", - "LD_ORIGIN_PATH", - "LD_PRELOAD", - "LD_PROFILE", - "LD_SHOW_AUXV", - "LD_USE_LOAD_BIAS", - "LOCALDOMAIN", - "LOCPATH", - "MALLOC_CHECK_", - "MALLOC_CONF", - "MALLOC_TRACE", - "NIS_PATH", - "NLSPATH", - "RESOLV_HOST_CONF", - "RES_OPTIONS", - "TMPDIR", - "TZDIR", - nullptr - }; - for (size_t i = 0; UNSAFE_VARIABLE_NAMES[i] != nullptr; ++i) { - if (env_match(name, UNSAFE_VARIABLE_NAMES[i]) != nullptr) { - return true; - } - } - return false; -} - -static void __sanitize_environment_variables() { - char** src = environ; - char** dst = environ; - for (; src[0] != nullptr; ++src) { - if (!__is_valid_environment_variable(src[0])) { - continue; - } - // Remove various unsafe environment variables if we're loading a setuid program. - if (get_AT_SECURE() && __is_unsafe_environment_variable(src[0])) { - continue; - } - dst[0] = src[0]; - ++dst; - } - dst[0] = nullptr; -} - -void linker_env_init(KernelArgumentBlock& args) { - // Store environment pointer - can't be null. - environ = args.envp; - - __init_AT_SECURE(args); - __sanitize_environment_variables(); -} - -const char* linker_env_get(const char* name) { - if (name == nullptr || name[0] == '\0') { - return nullptr; - } - - for (char** p = environ; p[0] != nullptr; ++p) { - const char* val = env_match(p[0], name); - if (val != nullptr) { - if (val[0] == '\0') { - return nullptr; // Return null for empty strings. - } - return val; - } - } - return nullptr; -} diff --git a/linker/linker_environ.h b/linker/linker_environ.h deleted file mode 100644 index 0f6ac084d..000000000 --- a/linker/linker_environ.h +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright (C) 2010 The Android Open Source Project - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in - * the documentation and/or other materials provided with the - * distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS - * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE - * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, - * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS - * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED - * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, - * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT - * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ - -#ifndef LINKER_ENVIRON_H -#define LINKER_ENVIRON_H - -class KernelArgumentBlock; - -// Call this function before any of the other functions in this header file. -extern void linker_env_init(KernelArgumentBlock& args); - -// Returns the value of environment variable 'name' if defined and not -// empty, or null otherwise. -extern const char* linker_env_get(const char* name); - -// Returns the value of this program's AT_SECURE variable. -extern bool get_AT_SECURE(); - -#endif // LINKER_ENVIRON_H