From fed2659869ec41a93f655be8058568ddab419e01 Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Sat, 18 Jul 2015 13:55:51 -0400 Subject: [PATCH] add fortified implementations of fread/fwrite A __size_mul_overflow utility is used to take advantage of the checked overflow intrinsics in Clang and GCC (>= 5). The fallback for older compilers is the optimized but less than ideal overflow checking pattern used in OpenBSD. Change-Id: Ibb0d4fd9b5acb67983e6a9f46844c2fd444f7e69 --- libc/Android.mk | 2 + libc/bionic/__fread_chk.cpp | 47 ++++++++++++++++++++++ libc/bionic/__fwrite_chk.cpp | 47 ++++++++++++++++++++++ libc/include/stdio.h | 62 ++++++++++++++++++++++++++++++ libc/include/sys/cdefs.h | 15 ++++++++ libc/libc.map | 2 + tests/fortify_compilation_test.cpp | 32 +++++++++++++++ tests/fortify_test.cpp | 16 ++++++++ 8 files changed, 223 insertions(+) create mode 100644 libc/bionic/__fread_chk.cpp create mode 100644 libc/bionic/__fwrite_chk.cpp diff --git a/libc/Android.mk b/libc/Android.mk index de0bb4174..5e09cf950 100644 --- a/libc/Android.mk +++ b/libc/Android.mk @@ -70,6 +70,8 @@ libc_common_src_files := \ libc_common_src_files += \ bionic/__FD_chk.cpp \ bionic/__fgets_chk.cpp \ + bionic/__fread_chk.cpp \ + bionic/__fwrite_chk.cpp \ bionic/__memchr_chk.cpp \ bionic/__memmove_chk.cpp \ bionic/__memrchr_chk.cpp \ diff --git a/libc/bionic/__fread_chk.cpp b/libc/bionic/__fread_chk.cpp new file mode 100644 index 000000000..afc8d90ce --- /dev/null +++ b/libc/bionic/__fread_chk.cpp @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2015 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. + */ + +#undef _FORTIFY_SOURCE +#include +#include +#include "private/libc_logging.h" + +extern "C" size_t __fread_chk(void * __restrict buf, size_t size, size_t count, + FILE * __restrict stream, size_t buf_size) { + size_t total; + if (__predict_false(__size_mul_overflow(size, count, &total))) { + // overflow: trigger the error path in fread + return fread(buf, size, count, stream); + } + + if (__predict_false(total > buf_size)) { + __fortify_chk_fail("fread: prevented write past end of buffer", 0); + } + + return fread(buf, size, count, stream); +} diff --git a/libc/bionic/__fwrite_chk.cpp b/libc/bionic/__fwrite_chk.cpp new file mode 100644 index 000000000..2af13d653 --- /dev/null +++ b/libc/bionic/__fwrite_chk.cpp @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2015 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. + */ + +#undef _FORTIFY_SOURCE +#include +#include +#include "private/libc_logging.h" + +extern "C" size_t __fwrite_chk(const void * __restrict buf, size_t size, size_t count, + FILE * __restrict stream, size_t buf_size) { + size_t total; + if (__predict_false(__size_mul_overflow(size, count, &total))) { + // overflow: trigger the error path in fwrite + return fwrite(buf, size, count, stream); + } + + if (__predict_false(total > buf_size)) { + __fortify_chk_fail("fwrite: prevented read past end of buffer", 0); + } + + return fwrite(buf, size, count, stream); +} diff --git a/libc/include/stdio.h b/libc/include/stdio.h index f5ed65278..c4bfb4fdf 100644 --- a/libc/include/stdio.h +++ b/libc/include/stdio.h @@ -382,6 +382,16 @@ extern char* __fgets_real(char*, int, FILE*) __RENAME(fgets); __errordecl(__fgets_too_big_error, "fgets called with size bigger than buffer"); __errordecl(__fgets_too_small_error, "fgets called with size less than zero"); +extern size_t __fread_chk(void * __restrict, size_t, size_t, FILE * __restrict, size_t); +extern size_t __fread_real(void * __restrict, size_t, size_t, FILE * __restrict) __RENAME(fread); +__errordecl(__fread_too_big_error, "fread called with size * count bigger than buffer"); +__errordecl(__fread_overflow, "fread called with overflowing size * count"); + +extern size_t __fwrite_chk(const void * __restrict, size_t, size_t, FILE * __restrict, size_t); +extern size_t __fwrite_real(const void * __restrict, size_t, size_t, FILE * __restrict) __RENAME(fwrite); +__errordecl(__fwrite_too_big_error, "fwrite called with size * count bigger than buffer"); +__errordecl(__fwrite_overflow, "fwrite called with overflowing size * count"); + #if defined(__BIONIC_FORTIFY) __BIONIC_FORTIFY_INLINE @@ -428,6 +438,58 @@ int sprintf(char *dest, const char *format, ...) } #endif +__BIONIC_FORTIFY_INLINE +size_t fread(void * __restrict buf, size_t size, size_t count, FILE * __restrict stream) { + size_t bos = __bos0(buf); + +#if !defined(__clang__) + if (bos == __BIONIC_FORTIFY_UNKNOWN_SIZE) { + return __fread_real(buf, size, count, stream); + } + + if (__builtin_constant_p(size) && __builtin_constant_p(count)) { + size_t total; + if (__size_mul_overflow(size, count, &total)) { + __fread_overflow(); + } + + if (total > bos) { + __fread_too_big_error(); + } + + return __fread_real(buf, size, count, stream); + } +#endif + + return __fread_chk(buf, size, count, stream, bos); +} + +__BIONIC_FORTIFY_INLINE +size_t fwrite(const void * __restrict buf, size_t size, size_t count, FILE * __restrict stream) { + size_t bos = __bos0(buf); + +#if !defined(__clang__) + if (bos == __BIONIC_FORTIFY_UNKNOWN_SIZE) { + return __fwrite_real(buf, size, count, stream); + } + + if (__builtin_constant_p(size) && __builtin_constant_p(count)) { + size_t total; + if (__size_mul_overflow(size, count, &total)) { + __fwrite_overflow(); + } + + if (total > bos) { + __fwrite_too_big_error(); + } + + return __fwrite_real(buf, size, count, stream); + } +#endif + + return __fwrite_chk(buf, size, count, stream, bos); +} + #if !defined(__clang__) __BIONIC_FORTIFY_INLINE diff --git a/libc/include/sys/cdefs.h b/libc/include/sys/cdefs.h index 7df8b6078..8a11bc62a 100644 --- a/libc/include/sys/cdefs.h +++ b/libc/include/sys/cdefs.h @@ -578,4 +578,19 @@ #define _BIONIC_NOT_BEFORE_21(x) #endif /* __ANDROID_API__ >= 21 */ +#if __has_builtin(__builtin_umul_overflow) || __GNUC__ >= 5 +#if __LP64__ +#define __size_mul_overflow(a, b, result) __builtin_umull_overflow(a, b, result) +#else +#define __size_mul_overflow(a, b, result) __builtin_umul_overflow(a, b, result) +#endif +#else +static __inline__ __always_inline int __size_mul_overflow(__SIZE_TYPE__ a, __SIZE_TYPE__ b, + __SIZE_TYPE__ *result) { + *result = a * b; + static const __SIZE_TYPE__ mul_no_overflow = 1UL << (sizeof(__SIZE_TYPE__) * 4); + return (a >= mul_no_overflow || b >= mul_no_overflow) && a > 0 && (__SIZE_TYPE__)-1 / a < b; +} +#endif + #endif /* !_SYS_CDEFS_H_ */ diff --git a/libc/libc.map b/libc/libc.map index ffbd29ccf..8c3ac9e72 100644 --- a/libc/libc.map +++ b/libc/libc.map @@ -1334,6 +1334,8 @@ LIBC { LIBC_N { global: + __fread_chk; + __fwrite_chk; getgrgid_r; getgrnam_r; } LIBC; diff --git a/tests/fortify_compilation_test.cpp b/tests/fortify_compilation_test.cpp index 537b341b9..166e8d9f9 100644 --- a/tests/fortify_compilation_test.cpp +++ b/tests/fortify_compilation_test.cpp @@ -230,3 +230,35 @@ void test_ppoll() { // clang should emit a warning, but doesn't ppoll(fds, 2, &timeout, NULL); } + +void test_fread_overflow() { + char buf[4]; + // NOLINTNEXTLINE(whitespace/line_length) + // GCC: error: call to '__fread_overflow' declared with attribute error: fread called with overflowing size * count + // clang should emit a warning, but doesn't + fread(buf, 2, (size_t)-1, stdin); +} + +void test_fread_too_big() { + char buf[4]; + // NOLINTNEXTLINE(whitespace/line_length) + // GCC: error: call to '__fread_too_big_error' declared with attribute error: fread called with size * count bigger than buffer + // clang should emit a warning, but doesn't + fread(buf, 1, 5, stdin); +} + +void test_fwrite_overflow() { + char buf[4]; + // NOLINTNEXTLINE(whitespace/line_length) + // GCC: error: call to '__fwrite_overflow' declared with attribute error: fwrite called with overflowing size * count + // clang should emit a warning, but doesn't + fwrite(buf, 2, (size_t)-1, stdout); +} + +void test_fwrite_too_big() { + char buf[4] = {0}; + // NOLINTNEXTLINE(whitespace/line_length) + // GCC: error: call to '__fwrite_too_big_error' declared with attribute error: fwrite called with size * count bigger than buffer + // clang should emit a warning, but doesn't + fwrite(buf, 1, 5, stdout); +} diff --git a/tests/fortify_test.cpp b/tests/fortify_test.cpp index 4faccb49f..664e057a6 100644 --- a/tests/fortify_test.cpp +++ b/tests/fortify_test.cpp @@ -647,6 +647,22 @@ TEST_F(DEATHTEST, read_fortified) { close(fd); } +TEST_F(DEATHTEST, fread_fortified) { + char buf[1]; + size_t ct = atoi("2"); // prevent optimizations + FILE* fp = fopen("/dev/null", "r"); + ASSERT_FORTIFY(fread(buf, 1, ct, fp)); + fclose(fp); +} + +TEST_F(DEATHTEST, fwrite_fortified) { + char buf[1] = {0}; + size_t ct = atoi("2"); // prevent optimizations + FILE* fp = fopen("/dev/null", "w"); + ASSERT_FORTIFY(fwrite(buf, 1, ct, fp)); + fclose(fp); +} + TEST_F(DEATHTEST, readlink_fortified) { char buf[1]; size_t ct = atoi("2"); // prevent optimizations