generic-library/bionic
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix over read in strcpy/stpcpy/strcat.

Browse Source 
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
This commit is contained in:
Christopher Ferris
2015-09-23 22:09:09 -07:00
parent 1399759118
commit fdfcfce7c6
6 changed files with 201 additions and 155 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
tests/buffer_tests.cpp
View File

@@ -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];
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, i);
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(

20
tests/string_test.cpp
View File

@@ -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';