diff --git a/libc/bionic/__strcat_chk.cpp b/libc/bionic/__strcat_chk.cpp index fb46e0d7d..e0b3259bf 100644 --- a/libc/bionic/__strcat_chk.cpp +++ b/libc/bionic/__strcat_chk.cpp @@ -29,7 +29,6 @@ #include #include #include "libc_logging.h" -#include /* * Runtime implementation of __builtin____strcat_chk. @@ -42,22 +41,24 @@ * This strcat check is called if _FORTIFY_SOURCE is defined and * greater than 0. */ -extern "C" char *__strcat_chk (char *dest, const char *src, size_t dest_buf_size) { - // TODO: optimize so we don't scan src/dest twice. - size_t src_len = strlen(src); - size_t dest_len = strlen(dest); - size_t sum; +extern "C" char* __strcat_chk( + char* __restrict dest, + const char* __restrict src, + size_t dest_buf_size) +{ + char* save = dest; + size_t dest_len = __strlen_chk(dest, dest_buf_size); - // sum = src_len + dest_len + 1 (with overflow protection) - if (!safe_add3(&sum, src_len, dest_len, 1U)) { - __fortify_chk_fail("strcat integer overflow", - BIONIC_EVENT_STRCAT_INTEGER_OVERFLOW); + dest += dest_len; + dest_buf_size -= dest_len; + + while ((*dest++ = *src++) != '\0') { + dest_buf_size--; + if (__predict_false(dest_buf_size == 0)) { + __fortify_chk_fail("strcat buffer overflow", + BIONIC_EVENT_STRCAT_BUFFER_OVERFLOW); + } } - if (sum > dest_buf_size) { - __fortify_chk_fail("strcat buffer overflow", - BIONIC_EVENT_STRCAT_BUFFER_OVERFLOW); - } - - return strcat(dest, src); + return save; } diff --git a/libc/bionic/__strncat_chk.cpp b/libc/bionic/__strncat_chk.cpp index ab2854175..f54d838cd 100644 --- a/libc/bionic/__strncat_chk.cpp +++ b/libc/bionic/__strncat_chk.cpp @@ -29,7 +29,6 @@ #include #include #include "libc_logging.h" -#include /* * Runtime implementation of __builtin____strncat_chk. @@ -42,27 +41,33 @@ * This strncat check is called if _FORTIFY_SOURCE is defined and * greater than 0. */ -extern "C" char *__strncat_chk (char *dest, const char *src, - size_t len, size_t dest_buf_size) +extern "C" char *__strncat_chk( + char* __restrict dest, + const char* __restrict src, + size_t len, size_t dest_buf_size) { - // TODO: optimize so we don't scan src/dest twice. - size_t dest_len = strlen(dest); - size_t src_len = strlen(src); - if (src_len > len) { - src_len = len; + if (len == 0) { + return dest; } - size_t sum; - // sum = src_len + dest_len + 1 (with overflow protection) - if (!safe_add3(&sum, src_len, dest_len, 1U)) { - __fortify_chk_fail("strncat integer overflow", - BIONIC_EVENT_STRNCAT_INTEGER_OVERFLOW); + size_t dest_len = __strlen_chk(dest, dest_buf_size); + char *d = dest + dest_len; + dest_buf_size -= dest_len; + + while (*src != '\0') { + *d++ = *src++; + len--; dest_buf_size--; + + if (__predict_false(dest_buf_size == 0)) { + __fortify_chk_fail("strncat buffer overflow", + BIONIC_EVENT_STRNCAT_BUFFER_OVERFLOW); + } + + if (len == 0) { + break; + } } - if (sum > dest_buf_size) { - __fortify_chk_fail("strncat buffer overflow", - BIONIC_EVENT_STRNCAT_BUFFER_OVERFLOW); - } - - return strncat(dest, src, len); + *d = '\0'; + return dest; } diff --git a/libc/include/string.h b/libc/include/string.h index f8c573cc3..1691b16c0 100644 --- a/libc/include/string.h +++ b/libc/include/string.h @@ -49,6 +49,7 @@ extern char* strchr(const char *, int) __purefunc; extern char* strrchr(const char *, int) __purefunc; extern size_t strlen(const char *) __purefunc; +extern size_t __strlen_chk(const char *, size_t); extern int strcmp(const char *, const char *) __purefunc; extern char* strcpy(char* __restrict, const char* __restrict); extern char* strcat(char* __restrict, const char* __restrict); @@ -207,8 +208,6 @@ size_t strlcat(char* __restrict dest, const char* __restrict src, size_t size) { return __strlcat_chk(dest, src, size, bos); } -extern size_t __strlen_chk(const char *, size_t); - __BIONIC_FORTIFY_INLINE size_t strlen(const char *s) { size_t bos = __bos(s); diff --git a/libc/private/libc_logging.h b/libc/private/libc_logging.h index c6e176597..e62ddf20b 100644 --- a/libc/private/libc_logging.h +++ b/libc/private/libc_logging.h @@ -45,9 +45,6 @@ enum { BIONIC_EVENT_MEMSET_BUFFER_OVERFLOW = 80125, BIONIC_EVENT_STRCPY_BUFFER_OVERFLOW = 80130, - BIONIC_EVENT_STRCAT_INTEGER_OVERFLOW = 80200, - BIONIC_EVENT_STRNCAT_INTEGER_OVERFLOW = 80205, - BIONIC_EVENT_RESOLVER_OLD_RESPONSE = 80300, BIONIC_EVENT_RESOLVER_WRONG_SERVER = 80305, BIONIC_EVENT_RESOLVER_WRONG_QUERY = 80310, diff --git a/tests/fortify1_test.cpp b/tests/fortify1_test.cpp index 70d458a08..a9d8afd64 100644 --- a/tests/fortify1_test.cpp +++ b/tests/fortify1_test.cpp @@ -76,3 +76,166 @@ TEST(Fortify1_DeathTest, strncat2_fortified) { size_t n = atoi("10"); // avoid compiler optimizations ASSERT_EXIT(strncat(buf, "0123456789", n), testing::KilledBySignal(SIGSEGV), ""); } + +TEST(Fortify1_DeathTest, strcat_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char src[11]; + strcpy(src, "0123456789"); + char buf[10]; + buf[0] = '\0'; + ASSERT_EXIT(strcat(buf, src), testing::KilledBySignal(SIGSEGV), ""); +} + +extern "C" char* __strncat_chk(char*, const char*, size_t, size_t); +extern "C" char* __strcat_chk(char*, const char*, size_t); + +TEST(Fortify1, strncat) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = __strncat_chk(buf, "01234", sizeof(buf) - strlen(buf) - 1, sizeof(buf)); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('\0', buf[6]); + ASSERT_EQ('A', buf[7]); + ASSERT_EQ('A', buf[8]); + ASSERT_EQ('A', buf[9]); +} + +TEST(Fortify1, strncat2) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = __strncat_chk(buf, "0123456789", 5, sizeof(buf)); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('\0', buf[6]); + ASSERT_EQ('A', buf[7]); + ASSERT_EQ('A', buf[8]); + ASSERT_EQ('A', buf[9]); +} + +TEST(Fortify1, strncat3) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = '\0'; + char* res = __strncat_chk(buf, "0123456789", 5, sizeof(buf)); + ASSERT_EQ(buf, res); + ASSERT_EQ('0', buf[0]); + ASSERT_EQ('1', buf[1]); + ASSERT_EQ('2', buf[2]); + ASSERT_EQ('3', buf[3]); + ASSERT_EQ('4', buf[4]); + ASSERT_EQ('\0', buf[5]); + ASSERT_EQ('A', buf[6]); + ASSERT_EQ('A', buf[7]); + ASSERT_EQ('A', buf[8]); + ASSERT_EQ('A', buf[9]); +} + +TEST(Fortify1, strncat4) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[9] = '\0'; + char* res = __strncat_chk(buf, "", 5, sizeof(buf)); + ASSERT_EQ(buf, res); + ASSERT_EQ('A', buf[0]); + ASSERT_EQ('A', buf[1]); + ASSERT_EQ('A', buf[2]); + ASSERT_EQ('A', buf[3]); + ASSERT_EQ('A', buf[4]); + ASSERT_EQ('A', buf[5]); + ASSERT_EQ('A', buf[6]); + ASSERT_EQ('A', buf[7]); + ASSERT_EQ('A', buf[8]); + ASSERT_EQ('\0', buf[9]); +} + +TEST(Fortify1, strncat5) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = __strncat_chk(buf, "01234567", 8, sizeof(buf)); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('5', buf[6]); + ASSERT_EQ('6', buf[7]); + ASSERT_EQ('7', buf[8]); + ASSERT_EQ('\0', buf[9]); +} + +TEST(Fortify1, strncat6) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = __strncat_chk(buf, "01234567", 9, sizeof(buf)); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('5', buf[6]); + ASSERT_EQ('6', buf[7]); + ASSERT_EQ('7', buf[8]); + ASSERT_EQ('\0', buf[9]); +} + + +TEST(Fortify1, strcat) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = __strcat_chk(buf, "01234", sizeof(buf)); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('\0', buf[6]); + ASSERT_EQ('A', buf[7]); + ASSERT_EQ('A', buf[8]); + ASSERT_EQ('A', buf[9]); +} + +TEST(Fortify1, strcat2) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = __strcat_chk(buf, "01234567", sizeof(buf)); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('5', buf[6]); + ASSERT_EQ('6', buf[7]); + ASSERT_EQ('7', buf[8]); + ASSERT_EQ('\0', buf[9]); +} diff --git a/tests/fortify2_test.cpp b/tests/fortify2_test.cpp index c937e91e7..eee5770dd 100644 --- a/tests/fortify2_test.cpp +++ b/tests/fortify2_test.cpp @@ -80,6 +80,32 @@ TEST(Fortify2_DeathTest, strncat2_fortified2) { ASSERT_EXIT(strncat(myfoo.a, "0123456789", n), testing::KilledBySignal(SIGSEGV), ""); } +TEST(Fortify2_DeathTest, strncat3_fortified2) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + foo myfoo; + memcpy(myfoo.a, "0123456789", sizeof(myfoo.a)); // unterminated string + myfoo.b[0] = '\0'; + size_t n = atoi("10"); // avoid compiler optimizations + ASSERT_EXIT(strncat(myfoo.b, myfoo.a, n), testing::KilledBySignal(SIGSEGV), ""); +} + +TEST(Fortify2_DeathTest, strcat_fortified2) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char src[11]; + strcpy(src, "0123456789"); + foo myfoo; + myfoo.a[0] = '\0'; + ASSERT_EXIT(strcat(myfoo.a, src), testing::KilledBySignal(SIGSEGV), ""); +} + +TEST(Fortify2_DeathTest, strcat2_fortified2) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + foo myfoo; + memcpy(myfoo.a, "0123456789", sizeof(myfoo.a)); // unterminated string + myfoo.b[0] = '\0'; + ASSERT_EXIT(strcat(myfoo.b, myfoo.a), testing::KilledBySignal(SIGSEGV), ""); +} + /***********************************************************/ /* TESTS BELOW HERE DUPLICATE TESTS FROM fortify1_test.cpp */ /***********************************************************/ @@ -138,3 +164,12 @@ TEST(Fortify2_DeathTest, strncat2_fortified) { size_t n = atoi("10"); // avoid compiler optimizations ASSERT_EXIT(strncat(buf, "0123456789", n), testing::KilledBySignal(SIGSEGV), ""); } + +TEST(Fortify2_DeathTest, strcat_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char src[11]; + strcpy(src, "0123456789"); + char buf[10]; + buf[0] = '\0'; + ASSERT_EXIT(strcat(buf, src), testing::KilledBySignal(SIGSEGV), ""); +} diff --git a/tests/string_test.cpp b/tests/string_test.cpp index 63bfadb45..a0924dcb7 100644 --- a/tests/string_test.cpp +++ b/tests/string_test.cpp @@ -209,6 +209,120 @@ TEST(string, strcat) { } } +TEST(string, strcat2) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = strcat(buf, "01234"); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('\0', buf[6]); + ASSERT_EQ('A', buf[7]); + ASSERT_EQ('A', buf[8]); + ASSERT_EQ('A', buf[9]); +} + +TEST(string, strcat3) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = strcat(buf, "01234567"); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('5', buf[6]); + ASSERT_EQ('6', buf[7]); + ASSERT_EQ('7', buf[8]); + ASSERT_EQ('\0', buf[9]); +} + +TEST(string, strncat2) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = strncat(buf, "01234", sizeof(buf) - strlen(buf) - 1); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('\0', buf[6]); + ASSERT_EQ('A', buf[7]); + ASSERT_EQ('A', buf[8]); + ASSERT_EQ('A', buf[9]); +} + +TEST(string, strncat3) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = strncat(buf, "0123456789", 5); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('\0', buf[6]); + ASSERT_EQ('A', buf[7]); + ASSERT_EQ('A', buf[8]); + ASSERT_EQ('A', buf[9]); +} + +TEST(string, strncat4) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = strncat(buf, "01234567", 8); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('5', buf[6]); + ASSERT_EQ('6', buf[7]); + ASSERT_EQ('7', buf[8]); + ASSERT_EQ('\0', buf[9]); +} + +TEST(string, strncat5) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = strncat(buf, "01234567", 9); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('5', buf[6]); + ASSERT_EQ('6', buf[7]); + ASSERT_EQ('7', buf[8]); + ASSERT_EQ('\0', buf[9]); +} + TEST(string, strchr_with_0) { char buf[10]; const char* s = "01234";