diff options
author | Christopher Ferris <cferris@google.com> | 2015-09-23 22:09:09 -0700 |
---|---|---|
committer | Christopher Ferris <cferris@google.com> | 2015-09-24 14:17:36 -0700 |
commit | fdfcfce7c6392d32f95a9f776ecd13da205b906c (patch) | |
tree | 2195834870f5c0ecda99c8f6354f974211eb6d66 /tests | |
parent | 1399759118f50071d499f88b18828e36aa2bad00 (diff) | |
download | android_bionic-fdfcfce7c6392d32f95a9f776ecd13da205b906c.tar.gz android_bionic-fdfcfce7c6392d32f95a9f776ecd13da205b906c.tar.bz2 android_bionic-fdfcfce7c6392d32f95a9f776ecd13da205b906c.zip |
Fix over read in strcpy/stpcpy/strcat.
This bug will happen when these circumstances are met:
- Destination address & 0x7 == 1, strlen of src is 11, 12, 13.
- Destination address & 0x7 == 2, strlen of src is 10, 11, 12.
- Destination address & 0x7 == 3, strlen of src is 9, 10, 11.
- Destination address & 0x7 == 4, strlen of src is 8, 9, 10.
In these cases, the dest alignment code does a ldr which reads 4 bytes,
and it will read past the end of the source. In most cases, this is
probably benign, but if this crosses into a new page it could cause a
crash.
Fix the labels in the cortex-a9 strcat.
Modify the overread test to vary the dst alignment to expost this bug.
Also, shrink the strcat/strlcat overread cases since the dst alignment
variation increases the runtime too much.
Bug: 24345899
Change-Id: Ib34a559bfcebd89861985b29cae6c1e47b5b5855
Diffstat (limited to 'tests')
-rw-r--r-- | tests/buffer_tests.cpp | 16 | ||||
-rw-r--r-- | tests/string_test.cpp | 20 |
2 files changed, 27 insertions, 9 deletions
diff --git a/tests/buffer_tests.cpp b/tests/buffer_tests.cpp index 7d830bb16..a5a0c2a16 100644 --- a/tests/buffer_tests.cpp +++ b/tests/buffer_tests.cpp @@ -381,15 +381,19 @@ void RunSrcDstBufferOverreadTest(void (*test_func)(uint8_t*, uint8_t*, size_t)) // Make the second page unreadable and unwritable. ASSERT_TRUE(mprotect(&memory[pagesize], pagesize, PROT_NONE) == 0); - uint8_t* dst = new uint8_t[pagesize]; - for (size_t i = 0; i < pagesize; i++) { - uint8_t* src = &memory[pagesize-i]; - - test_func(src, dst, i); + uint8_t* dst_buffer = new uint8_t[2*pagesize]; + // Change the dst alignment as we change the source. + for (size_t i = 0; i < 16; i++) { + uint8_t* dst = &dst_buffer[i]; + for (size_t j = 0; j < pagesize; j++) { + uint8_t* src = &memory[pagesize-j]; + + test_func(src, dst, j); + } } ASSERT_TRUE(mprotect(&memory[pagesize], pagesize, PROT_READ | PROT_WRITE) == 0); free(memory); - delete[] dst; + delete[] dst_buffer; } void RunCmpBufferOverreadTest( diff --git a/tests/string_test.cpp b/tests/string_test.cpp index 80190d7d7..842e1c7ee 100644 --- a/tests/string_test.cpp +++ b/tests/string_test.cpp @@ -1174,7 +1174,7 @@ static size_t LargeSetIncrement(size_t len) { return 1; } -#define STRCAT_DST_LEN 128 +#define STRCAT_DST_LEN 64 static void DoStrcatTest(uint8_t* src, uint8_t* dst, size_t len) { if (len >= 1) { @@ -1189,7 +1189,7 @@ static void DoStrcatTest(uint8_t* src, uint8_t* dst, size_t len) { int value2 = 32 + (value + 2) % 96; memset(cmp_buf, value2, sizeof(cmp_buf)); - for (size_t i = 1; i <= STRCAT_DST_LEN; i++) { + for (size_t i = 1; i <= STRCAT_DST_LEN;) { memset(dst, value2, i-1); memset(dst+i-1, 0, len-i); src[len-i] = '\0'; @@ -1197,6 +1197,13 @@ static void DoStrcatTest(uint8_t* src, uint8_t* dst, size_t len) { reinterpret_cast<char*>(src)))); ASSERT_TRUE(memcmp(dst, cmp_buf, i-1) == 0); ASSERT_TRUE(memcmp(src, dst+i-1, len-i+1) == 0); + // This is an expensive loop, so don't loop through every value, + // get to a certain size and then start doubling. + if (i < 16) { + i++; + } else { + i <<= 1; + } } } else { dst[0] = '\0'; @@ -1229,7 +1236,7 @@ static void DoStrlcatTest(uint8_t* src, uint8_t* dst, size_t len) { int value2 = 32 + (value + 2) % 96; memset(cmp_buf, value2, sizeof(cmp_buf)); - for (size_t i = 1; i <= STRCAT_DST_LEN; i++) { + for (size_t i = 1; i <= STRCAT_DST_LEN;) { memset(dst, value2, i-1); memset(dst+i-1, 0, len-i); src[len-i] = '\0'; @@ -1237,6 +1244,13 @@ static void DoStrlcatTest(uint8_t* src, uint8_t* dst, size_t len) { reinterpret_cast<char*>(src), len)); ASSERT_TRUE(memcmp(dst, cmp_buf, i-1) == 0); ASSERT_TRUE(memcmp(src, dst+i-1, len-i+1) == 0); + // This is an expensive loop, so don't loop through every value, + // get to a certain size and then start doubling. + if (i < 16) { + i++; + } else { + i <<= 1; + } } } else { dst[0] = '\0'; |