diff options
author | Treehugger Robot <treehugger-gerrit@google.com> | 2017-10-24 20:20:00 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2017-10-24 20:20:00 +0000 |
commit | b128c78aef290f0eb0a5fc989a53a39c89a131c9 (patch) | |
tree | 4f7c2e3c1e528c89c9e3eaaa80a5657b91850816 /libutils | |
parent | 0f2097c0f03760080322705c5ef8fcc836cc6419 (diff) | |
parent | bf3fff1a9ed39d005f36db43c9893697e0a006a3 (diff) | |
download | system_core-b128c78aef290f0eb0a5fc989a53a39c89a131c9.tar.gz system_core-b128c78aef290f0eb0a5fc989a53a39c89a131c9.tar.bz2 system_core-b128c78aef290f0eb0a5fc989a53a39c89a131c9.zip |
Merge "libutils: Fix bug in strstr16."
Diffstat (limited to 'libutils')
-rw-r--r-- | libutils/Unicode.cpp | 21 | ||||
-rw-r--r-- | libutils/tests/Unicode_test.cpp | 31 |
2 files changed, 40 insertions, 12 deletions
diff --git a/libutils/Unicode.cpp b/libutils/Unicode.cpp index 5fd915524..e7520a8e8 100644 --- a/libutils/Unicode.cpp +++ b/libutils/Unicode.cpp @@ -297,23 +297,22 @@ size_t strnlen16(const char16_t *s, size_t maxlen) char16_t* strstr16(const char16_t* src, const char16_t* target) { - const char16_t needle = *target++; - const size_t target_len = strlen16(target); - if (needle != '\0') { - do { + const char16_t needle = *target; + if (needle == '\0') return (char16_t*)src; + + const size_t target_len = strlen16(++target); + do { do { - if (*src == '\0') { - return nullptr; - } + if (*src == '\0') { + return nullptr; + } } while (*src++ != needle); - } while (strncmp16(src, target, target_len) != 0); - src--; - } + } while (strncmp16(src, target, target_len) != 0); + src--; return (char16_t*)src; } - int strzcmp16(const char16_t *s1, size_t n1, const char16_t *s2, size_t n2) { const char16_t* e1 = s1+n1; diff --git a/libutils/tests/Unicode_test.cpp b/libutils/tests/Unicode_test.cpp index d23e43a71..b92eef866 100644 --- a/libutils/tests/Unicode_test.cpp +++ b/libutils/tests/Unicode_test.cpp @@ -15,7 +15,11 @@ */ #define LOG_TAG "Unicode_test" -#include <utils/Log.h> + +#include <sys/mman.h> +#include <unistd.h> + +#include <log/log.h> #include <utils/Unicode.h> #include <gtest/gtest.h> @@ -119,6 +123,31 @@ TEST_F(UnicodeTest, strstr16EmptyTarget) { << "should return the original pointer"; } +TEST_F(UnicodeTest, strstr16EmptyTarget_bug) { + // In the original code when target is an empty string strlen16() would + // start reading the memory until a "terminating null" (that is, zero) + // character is found. This happens because "*target++" in the original + // code would increment the pointer beyond the actual string. + void* memptr; + const size_t alignment = sysconf(_SC_PAGESIZE); + const size_t size = 2 * alignment; + ASSERT_EQ(posix_memalign(&memptr, alignment, size), 0); + // Fill allocated memory. + memset(memptr, 'A', size); + // Create a pointer to an "empty" string on the first page. + char16_t* const emptyString = (char16_t* const)((char*)memptr + alignment - 4); + *emptyString = (char16_t)0; + // Protect the second page to show that strstr16() violates that. + ASSERT_EQ(mprotect((char*)memptr + alignment, alignment, PROT_NONE), 0); + // Test strstr16(): when bug is present a segmentation fault is raised. + ASSERT_EQ(strstr16((char16_t*)memptr, emptyString), (char16_t*)memptr) + << "should not read beyond the first char16_t."; + // Reset protection of the second page + ASSERT_EQ(mprotect((char*)memptr + alignment, alignment, PROT_READ | PROT_WRITE), 0); + // Free allocated memory. + free(memptr); +} + TEST_F(UnicodeTest, strstr16SameString) { const char16_t* result = strstr16(kSearchString, kSearchString); EXPECT_EQ(kSearchString, result) |