From 950a58e24d1019eb9d814dbb16f111a6b61e3f23 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Fri, 4 Apr 2014 14:38:18 -0700 Subject: [PATCH] Add stpcpy/stpncpy. Add tests for the above. Add the fortify implementations of __stpcpy_chk and __stpncpy_chk. Modify the strncpy test to cover more cases and use this template for stpncpy. Add all of the fortify test cases. Bug: 13746695 Change-Id: I8c0f0d4991a878b8e8734fff12c8b73b07fdd344 --- libc/Android.mk | 4 + libc/bionic/__stpcpy_chk.cpp | 55 ++++ libc/bionic/__stpncpy_chk.cpp | 92 +++++++ libc/bionic/__strcpy_chk.cpp | 4 +- libc/include/string.h | 44 +++- libc/private/libc_events.h | 2 + .../upstream-openbsd/lib/libc/string/stpcpy.c | 44 ++++ .../lib/libc/string/stpncpy.c | 56 +++++ tests/fortify_test.cpp | 120 ++++++++- tests/string_test.cpp | 235 +++++++++++------- 10 files changed, 557 insertions(+), 99 deletions(-) create mode 100644 libc/bionic/__stpcpy_chk.cpp create mode 100644 libc/bionic/__stpncpy_chk.cpp create mode 100644 libc/upstream-openbsd/lib/libc/string/stpcpy.c create mode 100644 libc/upstream-openbsd/lib/libc/string/stpncpy.c diff --git a/libc/Android.mk b/libc/Android.mk index 44bf3881e..3eaff2f60 100644 --- a/libc/Android.mk +++ b/libc/Android.mk @@ -96,6 +96,8 @@ libc_common_src_files += \ bionic/__memmove_chk.cpp \ bionic/__read_chk.cpp \ bionic/__recvfrom_chk.cpp \ + bionic/__stpcpy_chk.cpp \ + bionic/__stpncpy_chk.cpp \ bionic/__strchr_chk.cpp \ bionic/__strlcat_chk.cpp \ bionic/__strlcpy_chk.cpp \ @@ -383,6 +385,8 @@ libc_upstream_openbsd_src_files := \ upstream-openbsd/lib/libc/stdlib/strtoull.c \ upstream-openbsd/lib/libc/stdlib/strtoumax.c \ upstream-openbsd/lib/libc/stdlib/system.c \ + upstream-openbsd/lib/libc/string/stpcpy.c \ + upstream-openbsd/lib/libc/string/stpncpy.c \ upstream-openbsd/lib/libc/string/strcasecmp.c \ upstream-openbsd/lib/libc/string/strcspn.c \ upstream-openbsd/lib/libc/string/strdup.c \ diff --git a/libc/bionic/__stpcpy_chk.cpp b/libc/bionic/__stpcpy_chk.cpp new file mode 100644 index 000000000..3ce81eeba --- /dev/null +++ b/libc/bionic/__stpcpy_chk.cpp @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, + * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS + * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED + * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#undef _FORTIFY_SOURCE + +#include +#include +#include "private/libc_logging.h" + +/* + * Runtime implementation of __builtin____stpcpy_chk. + * + * See + * http://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html + * http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html + * for details. + * + * This stpcpy check is called if _FORTIFY_SOURCE is defined and + * greater than 0. + */ +extern "C" char* __stpcpy_chk(char* dest, const char* src, size_t dest_len) { + // TODO: optimize so we don't scan src twice. + size_t src_len = strlen(src) + 1; + if (__predict_false(src_len > dest_len)) { + __fortify_chk_fail("stpcpy: prevented write past end of buffer", + BIONIC_EVENT_STPCPY_BUFFER_OVERFLOW); + } + + return stpcpy(dest, src); +} diff --git a/libc/bionic/__stpncpy_chk.cpp b/libc/bionic/__stpncpy_chk.cpp new file mode 100644 index 000000000..6c9215c70 --- /dev/null +++ b/libc/bionic/__stpncpy_chk.cpp @@ -0,0 +1,92 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, + * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS + * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED + * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#undef _FORTIFY_SOURCE + +#include +#include +#include "private/libc_logging.h" + +/* + * Runtime implementation of __builtin____stpncpy_chk. + * + * See + * http://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html + * http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html + * for details. + * + * This stpncpy check is called if _FORTIFY_SOURCE is defined and + * greater than 0. + */ +extern "C" char* __stpncpy_chk(char* __restrict dest, const char* __restrict src, + size_t len, size_t dest_len) { + if (__predict_false(len > dest_len)) { + __fortify_chk_fail("stpncpy: prevented write past end of buffer", + BIONIC_EVENT_STPNCPY_BUFFER_OVERFLOW); + } + + return stpncpy(dest, src, len); +} + +/* + * __stpncpy_chk2 + * + * This is a variant of __stpncpy_chk, but it also checks to make + * sure we don't read beyond the end of "src". The code for this is + * based on the original version of stpncpy, but modified to check + * how much we read from "src" at the end of the copy operation. + */ +extern "C" char* __stpncpy_chk2(char* __restrict dst, const char* __restrict src, + size_t n, size_t dest_len, size_t src_len) +{ + if (__predict_false(n > dest_len)) { + __fortify_chk_fail("stpncpy: prevented write past end of buffer", + BIONIC_EVENT_STPNCPY_BUFFER_OVERFLOW); + } + if (n != 0) { + char* d = dst; + const char* s = src; + + do { + if ((*d++ = *s++) == 0) { + /* NUL pad the remaining n-1 bytes */ + while (--n != 0) { + *d++ = 0; + } + break; + } + } while (--n != 0); + + size_t s_copy_len = static_cast(s - src); + if (__predict_false(s_copy_len > src_len)) { + __fortify_chk_fail("stpncpy: prevented read past end of buffer", 0); + } + } + + return dst; +} diff --git a/libc/bionic/__strcpy_chk.cpp b/libc/bionic/__strcpy_chk.cpp index 1f6bc8085..ad3ef5024 100644 --- a/libc/bionic/__strcpy_chk.cpp +++ b/libc/bionic/__strcpy_chk.cpp @@ -26,6 +26,8 @@ * SUCH DAMAGE. */ +#undef _FORTIFY_SOURCE + #include #include #include "private/libc_logging.h" @@ -41,7 +43,7 @@ * This strcpy check is called if _FORTIFY_SOURCE is defined and * greater than 0. */ -extern "C" char* __strcpy_chk (char* dest, const char* src, size_t dest_len) { +extern "C" char* __strcpy_chk(char* dest, const char* src, size_t dest_len) { // TODO: optimize so we don't scan src twice. size_t src_len = strlen(src) + 1; if (__predict_false(src_len > dest_len)) { diff --git a/libc/include/string.h b/libc/include/string.h index ec998ed35..c9ae03bce 100644 --- a/libc/include/string.h +++ b/libc/include/string.h @@ -53,6 +53,7 @@ extern char* __strrchr_chk(const char *, int, size_t); 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* stpcpy(char* __restrict, const char* __restrict); extern char* strcpy(char* __restrict, const char* __restrict); extern char* strcat(char* __restrict, const char* __restrict); @@ -72,6 +73,7 @@ extern size_t strnlen(const char *, size_t) __purefunc; extern char* strncat(char* __restrict, const char* __restrict, size_t); extern char* strndup(const char *, size_t); extern int strncmp(const char *, const char *, size_t) __purefunc; +extern char* stpncpy(char* __restrict, const char* __restrict, size_t); extern char* strncpy(char* __restrict, const char* __restrict, size_t); extern size_t strlcat(char* __restrict, const char* __restrict, size_t); @@ -89,8 +91,8 @@ extern size_t strxfrm(char* __restrict, const char* __restrict, size_t); #if defined(__BIONIC_FORTIFY) -__errordecl(__memcpy_dest_size_error, "memcpy called with size bigger than destination"); -__errordecl(__memcpy_src_size_error, "memcpy called with size bigger than source"); +__errordecl(__memcpy_dest_size_error, "memcpy: prevented write past end of buffer"); +__errordecl(__memcpy_src_size_error, "memcpy: prevented read past end of buffer"); __BIONIC_FORTIFY_INLINE void* memcpy(void* __restrict dest, const void* __restrict src, size_t copy_amount) { @@ -115,12 +117,44 @@ void* memmove(void *dest, const void *src, size_t len) { return __builtin___memmove_chk(dest, src, len, __bos0(dest)); } +__BIONIC_FORTIFY_INLINE +char* stpcpy(char* __restrict dest, const char* __restrict src) { + return __builtin___stpcpy_chk(dest, src, __bos(dest)); +} + __BIONIC_FORTIFY_INLINE char* strcpy(char* __restrict dest, const char* __restrict src) { return __builtin___strcpy_chk(dest, src, __bos(dest)); } -__errordecl(__strncpy_error, "strncpy called with size bigger than buffer"); +__errordecl(__stpncpy_error, "stpncpy: prevented write past end of buffer"); +extern char* __stpncpy_chk2(char* __restrict, const char* __restrict, size_t, size_t, size_t); + +__BIONIC_FORTIFY_INLINE +char* stpncpy(char* __restrict dest, const char* __restrict src, size_t n) { + size_t bos_dest = __bos(dest); + size_t bos_src = __bos(src); + if (__builtin_constant_p(n) && (n > bos_dest)) { + __stpncpy_error(); + } + + if (bos_src == __BIONIC_FORTIFY_UNKNOWN_SIZE) { + return __builtin___stpncpy_chk(dest, src, n, bos_dest); + } + + if (__builtin_constant_p(n) && (n <= bos_src)) { + return __builtin___stpncpy_chk(dest, src, n, bos_dest); + } + + size_t slen = __builtin_strlen(src); + if (__builtin_constant_p(slen)) { + return __builtin___stpncpy_chk(dest, src, n, bos_dest); + } + + return __stpncpy_chk2(dest, src, n, bos_dest, bos_src); +} + +__errordecl(__strncpy_error, "strncpy: prevented write past end of buffer"); extern char* __strncpy_chk2(char* __restrict, const char* __restrict, size_t, size_t, size_t); __BIONIC_FORTIFY_INLINE @@ -164,7 +198,7 @@ void* memset(void *s, int c, size_t n) { extern size_t __strlcpy_real(char* __restrict, const char* __restrict, size_t) __asm__(__USER_LABEL_PREFIX__ "strlcpy"); -__errordecl(__strlcpy_error, "strlcpy called with size bigger than buffer"); +__errordecl(__strlcpy_error, "strlcpy: prevented write past end of buffer"); extern size_t __strlcpy_chk(char *, const char *, size_t, size_t); __BIONIC_FORTIFY_INLINE @@ -195,7 +229,7 @@ size_t strlcpy(char* __restrict dest, const char* __restrict src, size_t size) { 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"); +__errordecl(__strlcat_error, "strlcat: prevented write past end of buffer"); extern size_t __strlcat_chk(char* __restrict, const char* __restrict, size_t, size_t); diff --git a/libc/private/libc_events.h b/libc/private/libc_events.h index 5d20f4b81..f2b973d08 100644 --- a/libc/private/libc_events.h +++ b/libc/private/libc_events.h @@ -40,6 +40,8 @@ #define BIONIC_EVENT_STRNCPY_BUFFER_OVERFLOW 80120 #define BIONIC_EVENT_MEMSET_BUFFER_OVERFLOW 80125 #define BIONIC_EVENT_STRCPY_BUFFER_OVERFLOW 80130 +#define BIONIC_EVENT_STPCPY_BUFFER_OVERFLOW 80135 +#define BIONIC_EVENT_STPNCPY_BUFFER_OVERFLOW 80140 #define BIONIC_EVENT_RESOLVER_OLD_RESPONSE 80300 #define BIONIC_EVENT_RESOLVER_WRONG_SERVER 80305 diff --git a/libc/upstream-openbsd/lib/libc/string/stpcpy.c b/libc/upstream-openbsd/lib/libc/string/stpcpy.c new file mode 100644 index 000000000..d3d61e0f1 --- /dev/null +++ b/libc/upstream-openbsd/lib/libc/string/stpcpy.c @@ -0,0 +1,44 @@ +/* $OpenBSD: stpcpy.c,v 1.1 2012/01/17 02:48:01 guenther Exp $ */ + +/* + * Copyright (c) 1988 Regents of the University of California. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the University nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include + +#if defined(APIWARN) +__warn_references(stpcpy, + "warning: stpcpy() is dangerous GNU crap; don't use it"); +#endif + +char * +stpcpy(char *to, const char *from) +{ + for (; (*to = *from) != '\0'; ++from, ++to); + return(to); +} diff --git a/libc/upstream-openbsd/lib/libc/string/stpncpy.c b/libc/upstream-openbsd/lib/libc/string/stpncpy.c new file mode 100644 index 000000000..c7c2a57c4 --- /dev/null +++ b/libc/upstream-openbsd/lib/libc/string/stpncpy.c @@ -0,0 +1,56 @@ +/* $OpenBSD: stpncpy.c,v 1.2 2012/07/11 10:44:59 naddy Exp $ */ + +/*- + * Copyright (c) 1990 The Regents of the University of California. + * All rights reserved. + * + * This code is derived from software contributed to Berkeley by + * Chris Torek. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the University nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include + +char * +stpncpy(char *dst, const char *src, size_t n) +{ + if (n != 0) { + char *d = dst; + const char *s = src; + + dst = &dst[n]; + do { + if ((*d++ = *s++) == 0) { + dst = d - 1; + /* NUL pad the remaining n-1 bytes */ + while (--n != 0) + *d++ = 0; + break; + } + } while (--n != 0); + } + return (dst); +} diff --git a/tests/fortify_test.cpp b/tests/fortify_test.cpp index 67e197e6b..d80b2f72f 100644 --- a/tests/fortify_test.cpp +++ b/tests/fortify_test.cpp @@ -40,6 +40,31 @@ struct foo { char b[10]; }; +#ifndef __clang__ +// This test is disabled in clang because clang doesn't properly detect +// this buffer overflow. TODO: Fix clang. +TEST(DEATHTEST, stpncpy_fortified2) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + foo myfoo; + int copy_amt = atoi("11"); + ASSERT_EXIT(stpncpy(myfoo.a, "01234567890", copy_amt), + testing::KilledBySignal(SIGABRT), ""); +} +#endif + +#ifndef __clang__ +// This test is disabled in clang because clang doesn't properly detect +// this buffer overflow. TODO: Fix clang. +TEST(DEATHTEST, stpncpy2_fortified2) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + foo myfoo; + memset(&myfoo, 0, sizeof(myfoo)); + myfoo.one[0] = 'A'; // not null terminated string + ASSERT_EXIT(stpncpy(myfoo.b, myfoo.one, sizeof(myfoo.b)), + testing::KilledBySignal(SIGABRT), ""); +} +#endif + #ifndef __clang__ // This test is disabled in clang because clang doesn't properly detect // this buffer overflow. TODO: Fix clang. @@ -140,6 +165,24 @@ TEST(DEATHTEST, vsnprintf2_fortified2) { } #endif +#ifndef __clang__ +// zero sized target with "\0" source (should fail) +// This test is disabled in clang because clang doesn't properly detect +// this buffer overflow. TODO: Fix clang. +TEST(DEATHTEST, stpcpy_fortified2) { +#if defined(__BIONIC__) + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + foo myfoo; + char* src = strdup(""); + ASSERT_EXIT(stpcpy(myfoo.empty, src), + testing::KilledBySignal(SIGABRT), ""); + free(src); +#else // __BIONIC__ + GTEST_LOG_(INFO) << "This test does nothing.\n"; +#endif // __BIONIC__ +} +#endif + #ifndef __clang__ // zero sized target with "\0" source (should fail) // This test is disabled in clang because clang doesn't properly detect @@ -559,6 +602,23 @@ TEST(DEATHTEST, memcpy_fortified) { ASSERT_EXIT(memcpy(bufb, bufa, n), testing::KilledBySignal(SIGABRT), ""); } +TEST(DEATHTEST, stpncpy_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char bufa[15]; + char bufb[10]; + strcpy(bufa, "01234567890123"); + size_t n = strlen(bufa); + ASSERT_EXIT(stpncpy(bufb, bufa, n), testing::KilledBySignal(SIGABRT), ""); +} + +TEST(DEATHTEST, stpncpy2_fortified) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + char dest[11]; + char src[10]; + memcpy(src, "0123456789", sizeof(src)); // src is not null terminated + ASSERT_EXIT(stpncpy(dest, src, sizeof(dest)), testing::KilledBySignal(SIGABRT), ""); +} + TEST(DEATHTEST, strncpy_fortified) { ::testing::FLAGS_gtest_death_test_style = "threadsafe"; char bufa[15]; @@ -568,6 +628,7 @@ TEST(DEATHTEST, strncpy_fortified) { ASSERT_EXIT(strncpy(bufb, bufa, n), testing::KilledBySignal(SIGABRT), ""); } + TEST(DEATHTEST, strncpy2_fortified) { ::testing::FLAGS_gtest_death_test_style = "threadsafe"; char dest[11]; @@ -790,6 +851,45 @@ TEST(TEST_NAME, strcat2) { ASSERT_EQ('\0', buf[9]); } +TEST(TEST_NAME, stpncpy) { + char src[10]; + char dst[10]; + memcpy(src, "0123456789", sizeof(src)); // non null terminated string + stpncpy(dst, src, sizeof(dst)); + ASSERT_EQ('0', dst[0]); + ASSERT_EQ('1', dst[1]); + ASSERT_EQ('2', dst[2]); + ASSERT_EQ('3', dst[3]); + ASSERT_EQ('4', dst[4]); + ASSERT_EQ('5', dst[5]); + ASSERT_EQ('6', dst[6]); + ASSERT_EQ('7', dst[7]); + ASSERT_EQ('8', dst[8]); + ASSERT_EQ('9', dst[9]); +} + +TEST(TEST_NAME, stpncpy2) { + char src[10]; + char dst[15]; + memcpy(src, "012345678\0", sizeof(src)); + stpncpy(dst, src, sizeof(dst)); + ASSERT_EQ('0', dst[0]); + ASSERT_EQ('1', dst[1]); + ASSERT_EQ('2', dst[2]); + ASSERT_EQ('3', dst[3]); + ASSERT_EQ('4', dst[4]); + ASSERT_EQ('5', dst[5]); + ASSERT_EQ('6', dst[6]); + ASSERT_EQ('7', dst[7]); + ASSERT_EQ('8', dst[8]); + ASSERT_EQ('\0', dst[9]); + ASSERT_EQ('\0', dst[10]); + ASSERT_EQ('\0', dst[11]); + ASSERT_EQ('\0', dst[12]); + ASSERT_EQ('\0', dst[13]); + ASSERT_EQ('\0', dst[14]); +} + TEST(TEST_NAME, strncpy) { char src[10]; char dst[10]; @@ -848,22 +948,22 @@ TEST(TEST_NAME, strcat_chk_max_int_size) { ASSERT_EQ('\0', buf[9]); } +extern "C" char* __stpcpy_chk(char*, const char*, size_t); + +TEST(TEST_NAME, stpcpy_chk_max_int_size) { + char buf[10]; + char* res = __stpcpy_chk(buf, "012345678", (size_t)-1); + ASSERT_EQ(buf + strlen("012345678"), res); + ASSERT_STREQ("012345678", buf); +} + extern "C" char* __strcpy_chk(char*, const char*, size_t); TEST(TEST_NAME, strcpy_chk_max_int_size) { char buf[10]; char* res = __strcpy_chk(buf, "012345678", (size_t)-1); 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('5', buf[5]); - ASSERT_EQ('6', buf[6]); - ASSERT_EQ('7', buf[7]); - ASSERT_EQ('8', buf[8]); - ASSERT_EQ('\0', buf[9]); + ASSERT_STREQ("012345678", buf); } extern "C" void* __memcpy_chk(void*, const void*, size_t, size_t); diff --git a/tests/string_test.cpp b/tests/string_test.cpp index c35976a3e..14b284e2d 100644 --- a/tests/string_test.cpp +++ b/tests/string_test.cpp @@ -222,7 +222,7 @@ TEST(string, strcat) { TEST(string, strcpy2) { char buf[1]; char* orig = strdup(""); - strcpy(buf, orig); + ASSERT_EQ(buf, strcpy(buf, orig)); ASSERT_EQ('\0', buf[0]); free(orig); } @@ -232,13 +232,8 @@ TEST(string, strcpy3) { char buf[10]; char* orig = strdup("12345"); memset(buf, 'A', sizeof(buf)); - strcpy(buf, orig); - ASSERT_EQ('1', buf[0]); - ASSERT_EQ('2', buf[1]); - ASSERT_EQ('3', buf[2]); - ASSERT_EQ('4', buf[3]); - ASSERT_EQ('5', buf[4]); - ASSERT_EQ('\0', buf[5]); + ASSERT_EQ(buf, strcpy(buf, orig)); + ASSERT_STREQ("12345", buf); ASSERT_EQ('A', buf[6]); ASSERT_EQ('A', buf[7]); ASSERT_EQ('A', buf[8]); @@ -251,17 +246,41 @@ TEST(string, strcpy4) { char buf[10]; char* orig = strdup("123456789"); memset(buf, 'A', sizeof(buf)); - strcpy(buf, orig); - ASSERT_EQ('1', buf[0]); - ASSERT_EQ('2', buf[1]); - ASSERT_EQ('3', buf[2]); - ASSERT_EQ('4', buf[3]); - ASSERT_EQ('5', buf[4]); - ASSERT_EQ('6', buf[5]); - ASSERT_EQ('7', buf[6]); - ASSERT_EQ('8', buf[7]); - ASSERT_EQ('9', buf[8]); - ASSERT_EQ('\0', buf[9]); + ASSERT_EQ(buf, strcpy(buf, orig)); + ASSERT_STREQ("123456789", buf); + free(orig); +} + +// one byte target with "\0" source +TEST(string, stpcpy2) { + char buf[1]; + char* orig = strdup(""); + ASSERT_EQ(buf, stpcpy(buf, orig)); + ASSERT_EQ('\0', buf[0]); + free(orig); +} + +// multibyte target where we under fill target +TEST(string, stpcpy3) { + char buf[10]; + char* orig = strdup("12345"); + memset(buf, 'A', sizeof(buf)); + ASSERT_EQ(buf+strlen(orig), stpcpy(buf, orig)); + ASSERT_STREQ("12345", buf); + ASSERT_EQ('A', buf[6]); + ASSERT_EQ('A', buf[7]); + ASSERT_EQ('A', buf[8]); + ASSERT_EQ('A', buf[9]); + free(orig); +} + +// multibyte target where we fill target exactly +TEST(string, stpcpy4) { + char buf[10]; + char* orig = strdup("123456789"); + memset(buf, 'A', sizeof(buf)); + ASSERT_EQ(buf+strlen(orig), stpcpy(buf, orig)); + ASSERT_STREQ("123456789", buf); free(orig); } @@ -272,13 +291,7 @@ TEST(string, strcat2) { 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_STREQ("a01234", buf); ASSERT_EQ('A', buf[7]); ASSERT_EQ('A', buf[8]); ASSERT_EQ('A', buf[9]); @@ -291,16 +304,7 @@ TEST(string, strcat3) { 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]); + ASSERT_STREQ("a01234567", buf); } TEST(string, strncat2) { @@ -310,13 +314,7 @@ TEST(string, strncat2) { 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_STREQ("a01234", buf); ASSERT_EQ('A', buf[7]); ASSERT_EQ('A', buf[8]); ASSERT_EQ('A', buf[9]); @@ -329,13 +327,7 @@ TEST(string, strncat3) { 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_STREQ("a01234", buf); ASSERT_EQ('A', buf[7]); ASSERT_EQ('A', buf[8]); ASSERT_EQ('A', buf[9]); @@ -348,16 +340,7 @@ TEST(string, strncat4) { 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]); + ASSERT_STREQ("a01234567", buf); } TEST(string, strncat5) { @@ -367,16 +350,7 @@ TEST(string, strncat5) { 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]); + ASSERT_STREQ("a01234567", buf); } TEST(string, strchr_with_0) { @@ -456,6 +430,32 @@ TEST(string, strcmp) { } } +TEST(string, stpcpy) { + StringTestState state(SMALL); + for (size_t j = 0; j < POS_ITER; j++) { + state.NewIteration(); + + size_t pos = random() % state.MAX_LEN; + + memset(state.ptr1, '\2', pos); + state.ptr1[pos] = '\0'; + state.ptr1[state.MAX_LEN - 1] = '\0'; + + memcpy(state.ptr, state.ptr1, state.MAX_LEN); + + memset(state.ptr2, '\1', state.MAX_LEN); + state.ptr2[state.MAX_LEN - 1] = '\0'; + + memset(state.ptr + state.MAX_LEN, '\1', state.MAX_LEN); + memcpy(state.ptr + state.MAX_LEN, state.ptr1, pos + 1); + state.ptr[2 * state.MAX_LEN - 1] = '\0'; + + ASSERT_TRUE(stpcpy(state.ptr2, state.ptr1) == state.ptr2 + strlen(state.ptr1)); + ASSERT_FALSE((memcmp(state.ptr1, state.ptr, state.MAX_LEN)) != 0 || + (memcmp(state.ptr2, state.ptr + state.MAX_LEN, state.MAX_LEN) != 0)); + } +} + TEST(string, strcpy) { StringTestState state(SMALL); for (size_t j = 0; j < POS_ITER; j++) { @@ -482,7 +482,6 @@ TEST(string, strcpy) { } } - TEST(string, strlcat) { #if defined(__BIONIC__) StringTestState state(SMALL); @@ -614,30 +613,81 @@ TEST(string, strncmp) { } } +TEST(string, stpncpy) { + StringTestState state(SMALL); + for (size_t j = 0; j < ITER; j++) { + state.NewIteration(); + + // Choose a random value to fill the string, except \0 (string terminator), + // or \1 (guarantees it's different from anything in ptr2). + memset(state.ptr1, (random() % 254) + 2, state.MAX_LEN); + // Choose a random size for our src buffer. + size_t ptr1_len = random() % state.MAX_LEN; + state.ptr1[ptr1_len] = '\0'; + // Copy ptr1 into ptr, used to verify that ptr1 does not get modified. + memcpy(state.ptr, state.ptr1, state.MAX_LEN); + // Init ptr2 to a set value. + memset(state.ptr2, '\1', state.MAX_LEN); + + // Choose a random amount of data to copy. + size_t copy_len = random() % state.MAX_LEN; + + // Set the second half of ptr to the expected pattern in ptr2. + memset(state.ptr + state.MAX_LEN, '\1', state.MAX_LEN); + memcpy(state.ptr + state.MAX_LEN, state.ptr1, copy_len); + size_t expected_end; + if (copy_len > ptr1_len) { + memset(state.ptr + state.MAX_LEN + ptr1_len, '\0', copy_len - ptr1_len); + expected_end = ptr1_len; + } else { + expected_end = copy_len; + } + + ASSERT_EQ(state.ptr2 + expected_end, stpncpy(state.ptr2, state.ptr1, copy_len)); + + // Verify ptr1 was not modified. + ASSERT_EQ(0, memcmp(state.ptr1, state.ptr, state.MAX_LEN)); + // Verify ptr2 contains the expected data. + ASSERT_EQ(0, memcmp(state.ptr2, state.ptr + state.MAX_LEN, state.MAX_LEN)); + } +} + TEST(string, strncpy) { StringTestState state(SMALL); for (size_t j = 0; j < ITER; j++) { state.NewIteration(); - memset(state.ptr1, random() & 255, state.MAX_LEN); - state.ptr1[random () % state.MAX_LEN] = '\0'; + // Choose a random value to fill the string, except \0 (string terminator), + // or \1 (guarantees it's different from anything in ptr2). + memset(state.ptr1, (random() % 254) + 2, state.MAX_LEN); + // Choose a random size for our src buffer. + size_t ptr1_len = random() % state.MAX_LEN; + state.ptr1[ptr1_len] = '\0'; + // Copy ptr1 into ptr, used to verify that ptr1 does not get modified. memcpy(state.ptr, state.ptr1, state.MAX_LEN); - + // Init ptr2 to a set value. memset(state.ptr2, '\1', state.MAX_LEN); - size_t pos; - if (memchr(state.ptr1, 0, state.MAX_LEN)) { - pos = strlen(state.ptr1); + // Choose a random amount of data to copy. + size_t copy_len = random() % state.MAX_LEN; + + // Set the second half of ptr to the expected pattern in ptr2. + memset(state.ptr + state.MAX_LEN, '\1', state.MAX_LEN); + memcpy(state.ptr + state.MAX_LEN, state.ptr1, copy_len); + size_t expected_end; + if (copy_len > ptr1_len) { + memset(state.ptr + state.MAX_LEN + ptr1_len, '\0', copy_len - ptr1_len); + expected_end = ptr1_len; } else { - pos = state.MAX_LEN - 1; + expected_end = copy_len; } - memset(state.ptr + state.MAX_LEN, '\0', state.MAX_LEN); - memcpy(state.ptr + state.MAX_LEN, state.ptr1, pos + 1); + ASSERT_EQ(state.ptr2 + expected_end, stpncpy(state.ptr2, state.ptr1, copy_len)); - ASSERT_TRUE(strncpy(state.ptr2, state.ptr1, state.MAX_LEN) == state.ptr2); - ASSERT_FALSE(memcmp(state.ptr1, state.ptr, state.MAX_LEN) != 0 || - memcmp(state.ptr2, state.ptr + state.MAX_LEN, state.MAX_LEN) != 0); + // Verify ptr1 was not modified. + ASSERT_EQ(0, memcmp(state.ptr1, state.ptr, state.MAX_LEN)); + // Verify ptr2 contains the expected data. + ASSERT_EQ(0, memcmp(state.ptr2, state.ptr + state.MAX_LEN, state.MAX_LEN)); } } @@ -966,6 +1016,25 @@ TEST(string, strcpy_overread) { RunSrcDstBufferOverreadTest(DoStrcpyTest); } +static void DoStpcpyTest(uint8_t* src, uint8_t* dst, size_t len) { + if (len >= 1) { + memset(src, (32 + (len % 96)), len - 1); + src[len-1] = '\0'; + memset(dst, 0, len); + ASSERT_EQ(dst+len-1, reinterpret_cast(stpcpy(reinterpret_cast(dst), + reinterpret_cast(src)))); + ASSERT_TRUE(memcmp(src, dst, len) == 0); + } +} + +TEST(string, stpcpy_align) { + RunSrcDstBufferAlignTest(LARGE, DoStpcpyTest); +} + +TEST(string, stpcpy_overread) { + RunSrcDstBufferOverreadTest(DoStpcpyTest); +} + // Use our own incrementer to cut down on the total number of calls. static size_t LargeSetIncrement(size_t len) { if (len >= 4096) {