From dcab1b2c76a498c56bc00024613386de8b4b2aae Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Thu, 10 Jan 2013 17:12:29 -0800 Subject: [PATCH] Add stack canaries / strcpy tests. Add a test to ensure that stack canaries are working correctly. Since stack canaries aren't normally generated on non-string functions, we have to enable stack-protector-all. Add a test to ensure that an out of bounds strcpy generates a runtime failure. Change-Id: Id0d3e59fc4b9602da019e4d35c5c653e1a57fae4 --- tests/Android.mk | 2 +- tests/stack_protector_test.cpp | 20 ++++++++++++++++++++ tests/string_test.cpp | 13 +++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/Android.mk b/tests/Android.mk index a50232e94..635c0214c 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -47,7 +47,7 @@ include $(BUILD_EXECUTABLE) # ----------------------------------------------------------------------------- test_c_flags = \ - -fstack-protector \ + -fstack-protector-all \ -g \ -Wall -Wextra \ -Werror \ diff --git a/tests/stack_protector_test.cpp b/tests/stack_protector_test.cpp index 9d86506ac..9cf3c38ab 100644 --- a/tests/stack_protector_test.cpp +++ b/tests/stack_protector_test.cpp @@ -114,4 +114,24 @@ TEST(stack_protector, global_guard) { ASSERT_NE(0U, reinterpret_cast(__stack_chk_guard)); } +/* + * When this function returns, the stack canary will be inconsistent + * with the previous value, which will generate a call to __stack_chk_fail(), + * eventually resulting in a SIGABRT. + * + * This must be marked with "__attribute__ ((noinline))", to ensure the + * compiler generates the proper stack guards around this function. + */ +__attribute__ ((noinline)) +static void do_modify_stack_chk_guard() { + __stack_chk_guard = (void *) 0x12345678; +} + +// We have to say "DeathTest" here so gtest knows to run this test (which exits) +// in its own process. +TEST(stack_protector_DeathTest, modify_stack_protector) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + ASSERT_EXIT(do_modify_stack_chk_guard(), testing::KilledBySignal(SIGABRT), ""); +} + #endif diff --git a/tests/string_test.cpp b/tests/string_test.cpp index d55771cf6..3f7d50019 100644 --- a/tests/string_test.cpp +++ b/tests/string_test.cpp @@ -305,6 +305,19 @@ 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); +} +#endif + #if __BIONIC__ TEST(string, strlcat) { StringTestState state(SMALL);