From a6cde392765eb955cb4be5faa6ee62dcf77e8aa5 Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Sat, 29 Jun 2013 08:15:25 -0700 Subject: [PATCH] More FORTIFY_SOURCE functions under clang * bzero * umask * strlcat Change-Id: I65065208e0b8b37e10f6a266d5305de8fa9e59fc --- libc/include/string.h | 4 ++-- libc/include/strings.h | 4 ++-- libc/include/sys/stat.h | 6 ++++-- tests/fortify_test.cpp | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/libc/include/string.h b/libc/include/string.h index 2b7d47c40..7801ee987 100644 --- a/libc/include/string.h +++ b/libc/include/string.h @@ -175,7 +175,6 @@ size_t strlcpy(char* __restrict dest, const char* __restrict src, size_t size) { return __strlcpy_chk(dest, src, size, bos); } -#if !defined(__clang__) extern size_t __strlcat_real(char* __restrict, const char* __restrict, size_t) __asm__(__USER_LABEL_PREFIX__ "strlcat"); __errordecl(__strlcat_error, "strlcat called with size bigger than buffer"); @@ -186,6 +185,7 @@ __BIONIC_FORTIFY_INLINE size_t strlcat(char* __restrict dest, const char* __restrict src, size_t size) { size_t bos = __bos(dest); +#if !defined(__clang__) // Compiler doesn't know destination size. Don't call __strlcat_chk if (bos == __BIONIC_FORTIFY_UNKNOWN_SIZE) { return __strlcat_real(dest, src, size); @@ -202,10 +202,10 @@ size_t strlcat(char* __restrict dest, const char* __restrict src, size_t size) { if (__builtin_constant_p(size) && (size > bos)) { __strlcat_error(); } +#endif /* !defined(__clang__) */ return __strlcat_chk(dest, src, size, bos); } -#endif /* !defined(__clang__) */ __BIONIC_FORTIFY_INLINE size_t strlen(const char *s) { diff --git a/libc/include/strings.h b/libc/include/strings.h index faa12a47c..e72798b66 100644 --- a/libc/include/strings.h +++ b/libc/include/strings.h @@ -50,12 +50,12 @@ char *index(const char *, int); int strcasecmp(const char *, const char *); int strncasecmp(const char *, const char *, size_t); -#if defined(__BIONIC_FORTIFY) && !defined(__clang__) +#if defined(__BIONIC_FORTIFY) __BIONIC_FORTIFY_INLINE void bzero (void *s, size_t n) { __builtin___memset_chk(s, '\0', n, __builtin_object_size (s, 0)); } -#endif /* defined(__BIONIC_FORTIFY) && !defined(__clang__) */ +#endif /* defined(__BIONIC_FORTIFY) */ __END_DECLS diff --git a/libc/include/sys/stat.h b/libc/include/sys/stat.h index c9ad23d95..c7715a3ce 100644 --- a/libc/include/sys/stat.h +++ b/libc/include/sys/stat.h @@ -129,7 +129,7 @@ extern int lstat(const char *, struct stat *); extern int mknod(const char *, mode_t, dev_t); extern mode_t umask(mode_t); -#if defined(__BIONIC_FORTIFY) && !defined(__clang__) +#if defined(__BIONIC_FORTIFY) extern mode_t __umask_chk(mode_t); extern mode_t __umask_real(mode_t) @@ -138,15 +138,17 @@ __errordecl(__umask_invalid_mode, "umask called with invalid mode"); __BIONIC_FORTIFY_INLINE mode_t umask(mode_t mode) { +#if !defined(__clang__) if (__builtin_constant_p(mode)) { if ((mode & 0777) != mode) { __umask_invalid_mode(); } return __umask_real(mode); } +#endif return __umask_chk(mode); } -#endif /* defined(__BIONIC_FORTIFY) && !defined(__clang__) */ +#endif /* defined(__BIONIC_FORTIFY) */ #define stat64 stat diff --git a/tests/fortify_test.cpp b/tests/fortify_test.cpp index 68b5517d6..d8f0e7619 100644 --- a/tests/fortify_test.cpp +++ b/tests/fortify_test.cpp @@ -17,6 +17,8 @@ #include #include #include +#include +#include // We have to say "DeathTest" here so gtest knows to run this test (which exits) // in its own process. Unfortunately, the C preprocessor doesn't give us an @@ -204,6 +206,20 @@ TEST(DEATHTEST, strlcpy_fortified2) { } #endif +#ifndef __clang__ +// This test is disabled in clang because clang doesn't properly detect +// this buffer overflow. TODO: Fix clang. +TEST(DEATHTEST, strlcat_fortified2) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + foo myfoo; + strcpy(myfoo.a, "01"); + myfoo.one[0] = '\0'; + size_t n = strlen(myfoo.a); + ASSERT_EXIT(strlcat(myfoo.one, myfoo.a, n), + testing::KilledBySignal(SIGABRT), ""); +} +#endif + #endif /* __BIONIC__ */ #ifndef __clang__ @@ -268,6 +284,14 @@ TEST(DEATHTEST, snprintf_fortified2) { ASSERT_EXIT(snprintf(myfoo.b, n, "a%s", myfoo.a), testing::KilledBySignal(SIGABRT), ""); } +TEST(DEATHTEST, bzero_fortified2) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + foo myfoo; + memcpy(myfoo.b, "0123456789", sizeof(myfoo.b)); + size_t n = atoi("11"); + ASSERT_EXIT(bzero(myfoo.b, n), testing::KilledBySignal(SIGABRT), ""); +} + #endif /* defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE=2 */ #if __BIONIC__ @@ -337,6 +361,16 @@ TEST(DEATHTEST, strlcpy_fortified) { ASSERT_EXIT(strlcpy(bufb, bufa, n), testing::KilledBySignal(SIGABRT), ""); } +TEST(DEATHTEST, strlcat_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char bufa[15]; + char bufb[10]; + bufb[0] = '\0'; + strcpy(bufa, "01234567890123"); + size_t n = strlen(bufa); + ASSERT_EXIT(strlcat(bufb, bufa, n), testing::KilledBySignal(SIGABRT), ""); +} + #endif TEST(DEATHTEST, sprintf_fortified) { @@ -456,6 +490,20 @@ TEST(DEATHTEST, snprintf_fortified) { ASSERT_EXIT(snprintf(bufb, n, "%s", bufa), testing::KilledBySignal(SIGABRT), ""); } +TEST(DEATHTEST, bzero_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char buf[10]; + memcpy(buf, "0123456789", sizeof(buf)); + size_t n = atoi("11"); + ASSERT_EXIT(bzero(buf, n), testing::KilledBySignal(SIGABRT), ""); +} + +TEST(DEATHTEST, umask_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + mode_t mask = atoi("1023"); // 01777 in octal + ASSERT_EXIT(umask(mask), testing::KilledBySignal(SIGABRT), ""); +} + extern "C" char* __strncat_chk(char*, const char*, size_t, size_t); extern "C" char* __strcat_chk(char*, const char*, size_t); -- 2.11.0