From f3913b5b68347ce9a4cb17977df2c33f1e8f6000 Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Thu, 12 Jul 2012 15:10:03 -0700 Subject: [PATCH] FORTIFY_SOURCE: enhanced memcpy protections. Two changes: 1) Detect memory read overruns. For example: int main() { char buf[10]; memcpy(buf, "abcde", sizeof(buf)); sprintf("%s\n", buf); } because "abcde" is only 6 bytes, copying 10 bytes from it is a bug. This particular bug will be detected at compile time. Other similar bugs may be detected at runtime. 2) Detect overlapping buffers on memcpy() It is a bug to call memcpy() on buffers which overlap. For example, the following code is buggy: char buf3[0x800]; char *first_half = &buf3[0x400]; char *second_half = &buf3[1]; memset(buf3, 0, sizeof(buf3)); memcpy(first_half, second_half, 0x400); printf("1: %s\n", buf3); We now detect this at compile and run time. Change-Id: I092bd89f11f18e08e8a9dda0ca903aaea8e06d91 --- libc/include/string.h | 37 +++++++++++++++++++++++++++++++++++-- libc/string/__memcpy_chk.c | 27 ++++++++++++++++++++++----- libc/string/memmove.c | 1 + 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/libc/include/string.h b/libc/include/string.h index 32fd25f5b..269f7ac4e 100644 --- a/libc/include/string.h +++ b/libc/include/string.h @@ -87,9 +87,42 @@ extern size_t strxfrm(char *, const char *, size_t); #if defined(__BIONIC_FORTIFY_INLINE) +extern void __memcpy_dest_size_error() + __attribute__((__error__("memcpy called with size bigger than destination"))); +extern void __memcpy_src_size_error() + __attribute__((__error__("memcpy called with size bigger than source"))); +extern void __memcpy_overlap_error() + __attribute__((__error__("memcpy called with overlapping regions"))); +extern void *__memcpy_real(void *, const void *, size_t) + __asm__(__USER_LABEL_PREFIX__ "memcpy"); +extern void *__memcpy_chk2(void *, const void *, size_t, size_t, size_t); + + __BIONIC_FORTIFY_INLINE -void *memcpy (void *dest, const void *src, size_t len) { - return __builtin___memcpy_chk(dest, src, len, __builtin_object_size (dest, 0)); +void *memcpy (void *dest, const void *src, size_t copy_amount) { + char *d = (char *) dest; + const char *s = (const char *) src; + size_t s_len = __builtin_object_size(s, 0); + size_t d_len = __builtin_object_size(d, 0); + + if (__builtin_constant_p(copy_amount) && (copy_amount > d_len)) { + __memcpy_dest_size_error(); + } + + if (__builtin_constant_p(copy_amount) && (copy_amount > s_len)) { + __memcpy_src_size_error(); + } + + if (__builtin_constant_p(d - s) && __builtin_constant_p(copy_amount) + && (((size_t)(d - s) < copy_amount) || ((size_t)(s - d) < copy_amount))) { + __memcpy_overlap_error(); + } + + if (__builtin_constant_p(copy_amount) && __builtin_constant_p(d - s)) { + return __memcpy_real(dest, src, copy_amount); + } + + return __memcpy_chk2(dest, src, copy_amount, d_len, s_len); } __BIONIC_FORTIFY_INLINE diff --git a/libc/string/__memcpy_chk.c b/libc/string/__memcpy_chk.c index e79f6ac69..60fa42764 100644 --- a/libc/string/__memcpy_chk.c +++ b/libc/string/__memcpy_chk.c @@ -26,12 +26,13 @@ * SUCH DAMAGE. */ +#undef _FORTIFY_SOURCE #include #include #include /* - * Runtime implementation of __builtin____memcpy_chk. + * Runtime implementation of __memcpy_chk2. * * See * http://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html @@ -41,15 +42,31 @@ * This memcpy check is called if _FORTIFY_SOURCE is defined and * greater than 0. */ -void *__memcpy_chk (void *dest, const void *src, - size_t len, size_t dest_len) +void *__memcpy_chk2(void *dest, const void *src, + size_t copy_amount, size_t dest_len, size_t src_len) { - if (len > dest_len) { + char *d = (char *) dest; + const char *s = (const char *) src; + + if (__builtin_expect(copy_amount > dest_len, 0)) { __libc_android_log_print(ANDROID_LOG_FATAL, "libc", "*** memcpy buffer overflow detected ***\n"); __libc_android_log_event_uid(BIONIC_EVENT_MEMCPY_BUFFER_OVERFLOW); abort(); } - return memcpy(dest, src, len); + if (__builtin_expect(copy_amount > src_len, 0)) { + __libc_android_log_print(ANDROID_LOG_FATAL, "libc", + "*** memcpy read overflow detected ***\n"); + abort(); + } + + if (__builtin_expect(((d <= s) && ((size_t)(s - d) < copy_amount)) + || ((d >= s) && ((size_t)(d - s) < copy_amount)), 0)) { + __libc_android_log_print(ANDROID_LOG_FATAL, "libc", + "*** memcpy memory overlap detected ***\n"); + abort(); + } + + return memcpy(dest, src, copy_amount); } diff --git a/libc/string/memmove.c b/libc/string/memmove.c index fb1d9753e..a9fc1b53a 100644 --- a/libc/string/memmove.c +++ b/libc/string/memmove.c @@ -25,6 +25,7 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ +#undef _FORTIFY_SOURCE #include #include