libutils: Fix bug in strstr16.

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 may happen because "*target++", at line 300,
would increment the pointer beyond the actual string.

Signed-off-by: Branislav Rankov <branislav.rankov@arm.com>
Signed-off-by: Tamas Petz <tamas.petz@arm.com>
Test: libutils_tests --gtest_filter=UnicodeTest.strstr16*
Change-Id: I213ffe061057c7fa8f34b68881e106a709557dcd
This commit is contained in:
Branislav Rankov 2017-10-12 15:08:42 +02:00 committed by Mark Salyzyn
parent 1c65e77e6d
commit bf3fff1a9e
2 changed files with 40 additions and 12 deletions

View File

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

View File

@ -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)