From 1aae9bd170883805f2e7975cd3dbd2502b083cc1 Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Mon, 29 Apr 2013 14:07:06 -0700 Subject: [PATCH] strncpy: implement _FORTIFY_SOURCE=2 Add support for fortify source level 2 to strncpy. This will enable detection of more areas where strncpy is used inappropriately. For example, this would have detected bug 8727221. Move the fortify_source tests out of string_test.cpp, and put it into fortify1_test.cpp. Create a new fortify2_test.cpp file, which copies all the tests in fortify1_test.cpp, and adds fortify_source level 2 specific tests. Change-Id: Ica0fba531cc7d0609e4f23b8176739b13f7f7a83 --- libc/include/string.h | 9 +++++- libc/include/sys/cdefs.h | 6 ++++ tests/Android.mk | 2 ++ tests/fortify1_test.cpp | 54 ++++++++++++++++++++++++++++++++ tests/fortify2_test.cpp | 67 ++++++++++++++++++++++++++++++++++++++++ tests/string_test.cpp | 33 -------------------- 6 files changed, 137 insertions(+), 34 deletions(-) create mode 100644 tests/fortify1_test.cpp create mode 100644 tests/fortify2_test.cpp diff --git a/libc/include/string.h b/libc/include/string.h index 56d89a4b8..02d815143 100644 --- a/libc/include/string.h +++ b/libc/include/string.h @@ -119,9 +119,16 @@ char *strcpy(char *dest, const char *src) { return __builtin___strcpy_chk(dest, src, __builtin_object_size (dest, 0)); } +extern void __strncpy_error() + __attribute__((__error__("strncpy called with size bigger than buffer"))); + __BIONIC_FORTIFY_INLINE char *strncpy(char *dest, const char *src, size_t n) { - return __builtin___strncpy_chk(dest, src, n, __builtin_object_size (dest, 0)); + size_t bos = __bos(dest); + if (__builtin_constant_p(n) && (n > bos)) { + __strncpy_error(); + } + return __builtin___strncpy_chk(dest, src, n, bos); } __BIONIC_FORTIFY_INLINE diff --git a/libc/include/sys/cdefs.h b/libc/include/sys/cdefs.h index 1976d6ac2..1288b28a2 100644 --- a/libc/include/sys/cdefs.h +++ b/libc/include/sys/cdefs.h @@ -517,6 +517,12 @@ #if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0 && defined(__OPTIMIZE__) && __OPTIMIZE__ > 0 && !defined(__clang__) #define __BIONIC_FORTIFY 1 +#if _FORTIFY_SOURCE == 2 +#define __bos(s) __builtin_object_size((s), 1); +#else +#define __bos(s) __builtin_object_size((s), 0); +#endif + #define __BIONIC_FORTIFY_INLINE \ extern inline \ __attribute__ ((always_inline)) \ diff --git a/tests/Android.mk b/tests/Android.mk index dee5e3311..45cb462ce 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -60,6 +60,8 @@ test_src_files = \ dirent_test.cpp \ eventfd_test.cpp \ fenv_test.cpp \ + fortify1_test.cpp \ + fortify2_test.cpp \ getauxval_test.cpp \ getcwd_test.cpp \ libc_logging_test.cpp \ diff --git a/tests/fortify1_test.cpp b/tests/fortify1_test.cpp new file mode 100644 index 000000000..0cbb8cf67 --- /dev/null +++ b/tests/fortify1_test.cpp @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#undef _FORTIFY_SOURCE +#define _FORTIFY_SOURCE 1 + +#include +#include + +#if __BIONIC__ +// We have to say "DeathTest" here so gtest knows to run this test (which exits) +// in its own process. +TEST(Fortify1_DeathTest, strcpy_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char buf[10]; + char *orig = strdup("0123456789"); + ASSERT_EXIT(strcpy(buf, orig), testing::KilledBySignal(SIGSEGV), ""); + free(orig); +} + +TEST(Fortify1_DeathTest, strlen_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char buf[10]; + memcpy(buf, "0123456789", sizeof(buf)); + ASSERT_EXIT(printf("%d", strlen(buf)), testing::KilledBySignal(SIGSEGV), ""); +} + +TEST(Fortify1_DeathTest, strchr_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char buf[10]; + memcpy(buf, "0123456789", sizeof(buf)); + ASSERT_EXIT(printf("%s", strchr(buf, 'a')), testing::KilledBySignal(SIGSEGV), ""); +} + +TEST(Fortify1_DeathTest, strrchr_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char buf[10]; + memcpy(buf, "0123456789", sizeof(buf)); + ASSERT_EXIT(printf("%s", strrchr(buf, 'a')), testing::KilledBySignal(SIGSEGV), ""); +} +#endif diff --git a/tests/fortify2_test.cpp b/tests/fortify2_test.cpp new file mode 100644 index 000000000..9bedbe504 --- /dev/null +++ b/tests/fortify2_test.cpp @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#undef _FORTIFY_SOURCE +#define _FORTIFY_SOURCE 2 + +#include +#include + +struct foo { + char a[10]; + char b[10]; +}; + +// We have to say "DeathTest" here so gtest knows to run this test (which exits) +// in its own process. +TEST(Fortify2_DeathTest, strncpy_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + foo myfoo; + int copy_amt = atoi("11"); + ASSERT_EXIT(strncpy(myfoo.a, "01234567890", copy_amt), + testing::KilledBySignal(SIGSEGV), ""); +} + +#if __BIONIC__ +TEST(Fortify2_DeathTest, strcpy_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char buf[10]; + char *orig = strdup("0123456789"); + ASSERT_EXIT(strcpy(buf, orig), testing::KilledBySignal(SIGSEGV), ""); + free(orig); +} + +TEST(Fortify2_DeathTest, strlen_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char buf[10]; + memcpy(buf, "0123456789", sizeof(buf)); + ASSERT_EXIT(printf("%d", strlen(buf)), testing::KilledBySignal(SIGSEGV), ""); +} + +TEST(Fortify2_DeathTest, strchr_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char buf[10]; + memcpy(buf, "0123456789", sizeof(buf)); + ASSERT_EXIT(printf("%s", strchr(buf, 'a')), testing::KilledBySignal(SIGSEGV), ""); +} + +TEST(Fortify2_DeathTest, strrchr_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char buf[10]; + memcpy(buf, "0123456789", sizeof(buf)); + ASSERT_EXIT(printf("%s", strrchr(buf, 'a')), testing::KilledBySignal(SIGSEGV), ""); +} +#endif diff --git a/tests/string_test.cpp b/tests/string_test.cpp index 1720058e6..eb10c161b 100644 --- a/tests/string_test.cpp +++ b/tests/string_test.cpp @@ -306,39 +306,6 @@ TEST(string, strcpy) { } -#if __BIONIC__ -// We have to say "DeathTest" here so gtest knows to run this test (which exits) -// in its own process. -TEST(string_DeathTest, strcpy_fortified) { - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - char buf[10]; - char *orig = strdup("0123456789"); - ASSERT_EXIT(strcpy(buf, orig), testing::KilledBySignal(SIGSEGV), ""); - free(orig); -} - -TEST(string_DeathTest, strlen_fortified) { - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - char buf[10]; - memcpy(buf, "0123456789", sizeof(buf)); - ASSERT_EXIT(printf("%d", strlen(buf)), testing::KilledBySignal(SIGSEGV), ""); -} - -TEST(string_DeathTest, strchr_fortified) { - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - char buf[10]; - memcpy(buf, "0123456789", sizeof(buf)); - ASSERT_EXIT(printf("%s", strchr(buf, 'a')), testing::KilledBySignal(SIGSEGV), ""); -} - -TEST(string_DeathTest, strrchr_fortified) { - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - char buf[10]; - memcpy(buf, "0123456789", sizeof(buf)); - ASSERT_EXIT(printf("%s", strrchr(buf, 'a')), testing::KilledBySignal(SIGSEGV), ""); -} -#endif - #if __BIONIC__ TEST(string, strlcat) { StringTestState state(SMALL);