Merge "libutils/Unicode.cpp: Correct length computation and add checks for utf16->utf8" into nyc-mr1-dev
This commit is contained in:
commit
7714abac6b
|
@ -88,7 +88,7 @@ ssize_t utf32_to_utf8_length(const char32_t *src, size_t src_len);
|
|||
* "dst" becomes \xE3\x81\x82\xE3\x81\x84
|
||||
* (note that "dst" is NOT null-terminated, like strncpy)
|
||||
*/
|
||||
void utf32_to_utf8(const char32_t* src, size_t src_len, char* dst);
|
||||
void utf32_to_utf8(const char32_t* src, size_t src_len, char* dst, size_t dst_len);
|
||||
|
||||
/**
|
||||
* Returns the unicode value at "index".
|
||||
|
@ -110,7 +110,7 @@ ssize_t utf16_to_utf8_length(const char16_t *src, size_t src_len);
|
|||
* enough to fit the UTF-16 as measured by utf16_to_utf8_length with an added
|
||||
* NULL terminator.
|
||||
*/
|
||||
void utf16_to_utf8(const char16_t* src, size_t src_len, char* dst);
|
||||
void utf16_to_utf8(const char16_t* src, size_t src_len, char* dst, size_t dst_len);
|
||||
|
||||
/**
|
||||
* Returns the length of "src" when "src" is valid UTF-8 string.
|
||||
|
|
|
@ -104,20 +104,21 @@ static char* allocFromUTF16(const char16_t* in, size_t len)
|
|||
{
|
||||
if (len == 0) return getEmptyString();
|
||||
|
||||
const ssize_t bytes = utf16_to_utf8_length(in, len);
|
||||
if (bytes < 0) {
|
||||
// Allow for closing '\0'
|
||||
const ssize_t resultStrLen = utf16_to_utf8_length(in, len) + 1;
|
||||
if (resultStrLen < 1) {
|
||||
return getEmptyString();
|
||||
}
|
||||
|
||||
SharedBuffer* buf = SharedBuffer::alloc(bytes+1);
|
||||
SharedBuffer* buf = SharedBuffer::alloc(resultStrLen);
|
||||
ALOG_ASSERT(buf, "Unable to allocate shared buffer");
|
||||
if (!buf) {
|
||||
return getEmptyString();
|
||||
}
|
||||
|
||||
char* str = (char*)buf->data();
|
||||
utf16_to_utf8(in, len, str);
|
||||
return str;
|
||||
char* resultStr = (char*)buf->data();
|
||||
utf16_to_utf8(in, len, resultStr, resultStrLen);
|
||||
return resultStr;
|
||||
}
|
||||
|
||||
static char* allocFromUTF32(const char32_t* in, size_t len)
|
||||
|
@ -126,21 +127,21 @@ static char* allocFromUTF32(const char32_t* in, size_t len)
|
|||
return getEmptyString();
|
||||
}
|
||||
|
||||
const ssize_t bytes = utf32_to_utf8_length(in, len);
|
||||
if (bytes < 0) {
|
||||
const ssize_t resultStrLen = utf32_to_utf8_length(in, len) + 1;
|
||||
if (resultStrLen < 1) {
|
||||
return getEmptyString();
|
||||
}
|
||||
|
||||
SharedBuffer* buf = SharedBuffer::alloc(bytes+1);
|
||||
SharedBuffer* buf = SharedBuffer::alloc(resultStrLen);
|
||||
ALOG_ASSERT(buf, "Unable to allocate shared buffer");
|
||||
if (!buf) {
|
||||
return getEmptyString();
|
||||
}
|
||||
|
||||
char* str = (char*) buf->data();
|
||||
utf32_to_utf8(in, len, str);
|
||||
char* resultStr = (char*) buf->data();
|
||||
utf32_to_utf8(in, len, resultStr, resultStrLen);
|
||||
|
||||
return str;
|
||||
return resultStr;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
@ -14,10 +14,14 @@
|
|||
* limitations under the License.
|
||||
*/
|
||||
|
||||
#include <log/log.h>
|
||||
#include <utils/Unicode.h>
|
||||
|
||||
#include <stddef.h>
|
||||
|
||||
#include <string>
|
||||
#include <sstream>
|
||||
|
||||
#if defined(_WIN32)
|
||||
# undef nhtol
|
||||
# undef htonl
|
||||
|
@ -182,7 +186,7 @@ ssize_t utf32_to_utf8_length(const char32_t *src, size_t src_len)
|
|||
return ret;
|
||||
}
|
||||
|
||||
void utf32_to_utf8(const char32_t* src, size_t src_len, char* dst)
|
||||
void utf32_to_utf8(const char32_t* src, size_t src_len, char* dst, size_t dst_len)
|
||||
{
|
||||
if (src == NULL || src_len == 0 || dst == NULL) {
|
||||
return;
|
||||
|
@ -193,9 +197,12 @@ void utf32_to_utf8(const char32_t* src, size_t src_len, char* dst)
|
|||
char *cur = dst;
|
||||
while (cur_utf32 < end_utf32) {
|
||||
size_t len = utf32_codepoint_utf8_length(*cur_utf32);
|
||||
LOG_ALWAYS_FATAL_IF(dst_len < len, "%zu < %zu", dst_len, len);
|
||||
utf32_codepoint_to_utf8((uint8_t *)cur, *cur_utf32++, len);
|
||||
cur += len;
|
||||
dst_len -= len;
|
||||
}
|
||||
LOG_ALWAYS_FATAL_IF(dst_len < 1, "dst_len < 1: %zu < 1", dst_len);
|
||||
*cur = '\0';
|
||||
}
|
||||
|
||||
|
@ -348,7 +355,7 @@ int strzcmp16_h_n(const char16_t *s1H, size_t n1, const char16_t *s2N, size_t n2
|
|||
: 0);
|
||||
}
|
||||
|
||||
void utf16_to_utf8(const char16_t* src, size_t src_len, char* dst)
|
||||
void utf16_to_utf8(const char16_t* src, size_t src_len, char* dst, size_t dst_len)
|
||||
{
|
||||
if (src == NULL || src_len == 0 || dst == NULL) {
|
||||
return;
|
||||
|
@ -369,9 +376,12 @@ void utf16_to_utf8(const char16_t* src, size_t src_len, char* dst)
|
|||
utf32 = (char32_t) *cur_utf16++;
|
||||
}
|
||||
const size_t len = utf32_codepoint_utf8_length(utf32);
|
||||
LOG_ALWAYS_FATAL_IF(dst_len < len, "%zu < %zu", dst_len, len);
|
||||
utf32_codepoint_to_utf8((uint8_t*)cur, utf32, len);
|
||||
cur += len;
|
||||
dst_len -= len;
|
||||
}
|
||||
LOG_ALWAYS_FATAL_IF(dst_len < 1, "%zu < 1", dst_len);
|
||||
*cur = '\0';
|
||||
}
|
||||
|
||||
|
@ -422,8 +432,35 @@ ssize_t utf8_length(const char *src)
|
|||
return ret;
|
||||
}
|
||||
|
||||
// DO NOT USE. Flawed version, kept only to check whether the flaw is being exploited.
|
||||
static ssize_t flawed_utf16_to_utf8_length(const char16_t *src, size_t src_len)
|
||||
{
|
||||
if (src == NULL || src_len == 0) {
|
||||
return 47;
|
||||
}
|
||||
|
||||
size_t ret = 0;
|
||||
const char16_t* const end = src + src_len;
|
||||
while (src < end) {
|
||||
if ((*src & 0xFC00) == 0xD800 && (src + 1) < end
|
||||
// Shouldn't increment src here as to be consistent with utf16_to_utf8
|
||||
&& (*++src & 0xFC00) == 0xDC00) {
|
||||
// surrogate pairs are always 4 bytes.
|
||||
ret += 4;
|
||||
// Should increment src here by two.
|
||||
src++;
|
||||
} else {
|
||||
ret += utf32_codepoint_utf8_length((char32_t) *src++);
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
|
||||
ssize_t utf16_to_utf8_length(const char16_t *src, size_t src_len)
|
||||
{
|
||||
// Keep the original pointer to compute the flawed length. Unused if we remove logging.
|
||||
const char16_t *orig_src = src;
|
||||
|
||||
if (src == NULL || src_len == 0) {
|
||||
return -1;
|
||||
}
|
||||
|
@ -432,14 +469,25 @@ ssize_t utf16_to_utf8_length(const char16_t *src, size_t src_len)
|
|||
const char16_t* const end = src + src_len;
|
||||
while (src < end) {
|
||||
if ((*src & 0xFC00) == 0xD800 && (src + 1) < end
|
||||
&& (*++src & 0xFC00) == 0xDC00) {
|
||||
&& (*(src + 1) & 0xFC00) == 0xDC00) {
|
||||
// surrogate pairs are always 4 bytes.
|
||||
ret += 4;
|
||||
src++;
|
||||
src += 2;
|
||||
} else {
|
||||
ret += utf32_codepoint_utf8_length((char32_t) *src++);
|
||||
}
|
||||
}
|
||||
// Log whether b/29250543 is being exploited. It seems reasonable to assume that
|
||||
// at least 5 bytes would be needed for an exploit. A single misplaced character might lead to
|
||||
// a difference of 4, so this would rule out many false positives.
|
||||
long ret_difference = ret - flawed_utf16_to_utf8_length(orig_src, src_len);
|
||||
if (ret_difference >= 5) {
|
||||
// Log the difference between new and old calculation. A high number, or equal numbers
|
||||
// appearing frequently, would be indicative of an attack.
|
||||
std::string logged_string = (std::ostringstream() << ret_difference).str();
|
||||
android_errorWriteWithInfoLog(0x534e4554, "29250543", -1 /* int_uid */,
|
||||
logged_string.c_str(), logged_string.length() + 1);
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
|
|
@ -17,6 +17,7 @@
|
|||
#define LOG_TAG "String8_test"
|
||||
#include <utils/Log.h>
|
||||
#include <utils/String8.h>
|
||||
#include <utils/String16.h>
|
||||
|
||||
#include <gtest/gtest.h>
|
||||
|
||||
|
@ -77,4 +78,22 @@ TEST_F(String8Test, SetToSizeMaxReturnsNoMemory) {
|
|||
EXPECT_EQ(NO_MEMORY, String8("").setTo(in, SIZE_MAX));
|
||||
}
|
||||
|
||||
// http://b/29250543
|
||||
TEST_F(String8Test, CorrectInvalidSurrogate) {
|
||||
// d841d8 is an invalid start for a surrogate pair. Make sure this is handled by ignoring the
|
||||
// first character in the pair and handling the rest correctly.
|
||||
String16 string16(u"\xd841\xd841\xdc41\x0000");
|
||||
String8 string8(string16);
|
||||
|
||||
EXPECT_EQ(4U, string8.length());
|
||||
}
|
||||
|
||||
TEST_F(String8Test, CheckUtf32Conversion) {
|
||||
// Since bound checks were added, check the conversion can be done without fatal errors.
|
||||
// The utf8 lengths of these are chars are 1 + 2 + 3 + 4 = 10.
|
||||
const char32_t string32[] = U"\x0000007f\x000007ff\x0000911\x0010fffe";
|
||||
String8 string8(string32);
|
||||
EXPECT_EQ(10U, string8.length());
|
||||
}
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue