Source: https://bugs.chromium.org/p/project-zero/issues/detail?id=840
There's an inconsistency between the way that the two functions in libutils/Unicode.cpp handle invalid surrogate pairs in UTF16, resulting in a mismatch between the size calculated by utf16_to_utf8_length and the number of bytes written by utf16_to_utf8.
This results in a heap-buffer-overflow; one route to this code is the String8 constructor initialising a String8 from a String16. This can be reached via binder calls to the core system service "android.security.keystore" from a normal app context without any additional permissions. There are probably other routes to reach this code with attacker controlled data.
ssize_t utf16_to_utf8_length(const char16_t *src, size_t src_len)
{
if (src == NULL || src_len == 0) {
return -1;
}
size_t ret = 0;
const char16_t* const end = src + src_len;
while (src < end) {
if ((*src & 0xFC00) == 0xD800 && (src + 1) < end
&& (*++src & 0xFC00) == 0xDC00) { // <---- increment src here even if condition is false
// surrogate pairs are always 4 bytes.
ret += 4;
src++;
} else {
ret += utf32_codepoint_utf8_length((char32_t) *src++); // <---- increment src again
}
}
return ret;
}
void utf16_to_utf8(const char16_t* src, size_t src_len, char* dst)
{
if (src == NULL || src_len == 0 || dst == NULL) {
return;
}
const char16_t* cur_utf16 = src;
const char16_t* const end_utf16 = src + src_len;
char *cur = dst;
while (cur_utf16 < end_utf16) {
char32_t utf32;
// surrogate pairs
if((*cur_utf16 & 0xFC00) == 0xD800 && (cur_utf16 + 1) < end_utf16
&& (*(cur_utf16 + 1) & 0xFC00) == 0xDC00) { // <---- no increment if condition is false
utf32 = (*cur_utf16++ - 0xD800) << 10;
utf32 |= *cur_utf16++ - 0xDC00;
utf32 += 0x10000;
} else {
utf32 = (char32_t) *cur_utf16++; // <---- increment src
}
const size_t len = utf32_codepoint_utf8_length(utf32);
utf32_codepoint_to_utf8((uint8_t*)cur, utf32, len);
cur += len;
}
*cur = '\0';
}
An example character sequence would be the following:
\x41\xd8 \x41\xd8 \x41\xdc \x00\x00
This will be processed by utf16_to_utf8_len like this:
first loop iteration:
\x41\xd8 \x41\xd8 \x41\xdc \x00\x00
^
invalid surrogate; skip at (*++src & 0xfc00 == 0xdc00)
\x41\xd8 \x41\xd8 \x41\xdc \x00\x00
^
invalid surrogate; emit length 0 at (utf32_codepoint_utf8_length(*src++))
second loop iteration:
\x41\xd8 \x41\xd8 \x41\xdc \x00\x00
^
invalid surrogate; emit length 0 at (utf32_codepoint_utf8_length(*src++))
And will be processed by utf16_to_utf8 like this:
first loop iteration:
\x41\xd8 \x41\xd8 \x41\xdc \x00\x00
^
invalid surrogate; write 0 length character to output
second loop iteration
\x41\xd8 \x41\xd8 \x41\xdc \x00\x00
^
valid surrogate pair 0xd841 0xdc41; emit length 4 character to output
We can then construct a crash PoC using this sequence for the String16 passed to the keystore method 'getKeyCharacteristics' that will perform the String8(String16&) constructor on attacker supplied input; and provide a massive input string. The crash PoC should write 0x20000 * 2/3 bytes into a 2 byte heap allocation. It has been tested on a recent nexus5x userdebug build; resulting in the following crash (the object backing an android::vectorImpl has been corrupted by the overwrite, and "\xf0\xa0\x91\x81" is the utf8 encoding for the utf16 "\x41\xd8 \x41\xdc"):
pid: 16669, tid: 16669, name: keystore >>> /system/bin/keystore <<<
signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x91a0f08191a110
x0 8191a0f08191a108 x1 0000000000000000 x2 0000000000000000 x3 0000000000000020
x4 00000000ffffffa0 x5 0000000000000010 x6 0000000000000001 x7 0000007f802c0018
x8 0000000000000000 x9 000000000a7c5ac5 x10 0000000000000000 x11 0000000000000000
x12 000000000000d841 x13 0000000000000841 x14 0000000000000041 x15 0000007f8067bd9e
x16 0000005565984f08 x17 0000007f80aeee48 x18 00000000ffffff91 x19 0000007fd1de26c0
x20 8191a0f08191a108 x21 8191a0f08191a0f0 x22 0000000000000000 x23 0000005565984000
x24 8191a0f08191a0f0 x25 0000007fd1dea7b8 x26 0000007f806690e0 x27 0000007fd1de25d0
x28 000000556596f000 x29 0000007fd1de2550 x30 0000005565961188
sp 0000007fd1de2550 pc 0000007f80aeee58 pstate 0000000060000000
backtrace:
#00 pc 0000000000016e58 /system/lib64/libutils.so (_ZN7android10VectorImpl13editArrayImplEv+16)
#01 pc 000000000000a184 /system/bin/keystore
#02 pc 00000000000112d0 /system/bin/keystore
#03 pc 000000000000b7f4 /system/lib64/libkeystore_binder.so (_ZN7android17BnKeystoreService10onTransactEjRKNS_6ParcelEPS1_j+1560)
#04 pc 0000000000024c9c /system/lib64/libbinder.so (_ZN7android7BBinder8transactEjRKNS_6ParcelEPS1_j+168)
#05 pc 000000000002dd98 /system/lib64/libbinder.so (_ZN7android14IPCThreadState14executeCommandEi+1240)
#06 pc 000000000002de4c /system/lib64/libbinder.so (_ZN7android14IPCThreadState20getAndExecuteCommandEv+140)
#07 pc 000000000002def4 /system/lib64/libbinder.so (_ZN7android14IPCThreadState14joinThreadPoolEb+76)
#08 pc 0000000000007a04 /system/bin/keystore (main+1940)
#09 pc 000000000001bc98 /system/lib64/libc.so (__libc_init+100)
#10 pc 0000000000007c20 /system/bin/keystore
######################################################
Actually you can compromise many native system services using this bug (ie those not implemented in Java); because of the interface token checking code in Parcel.cpp. See attached for another PoC that takes as a first command line argument the name of the service to crash. On my nexus 5x with very unscientific testing, this includes the following services:
- phone, iphonesubinfo, isub (com.android.phone)
- telecom, voiceinteraction, backup, audio, location, notification, connectivity, wifi, network_management, statusbar, device_policy, mount, input_method, window, content, account, telephony.registry, user, package, batterystats (system_server)
- media.audio_policy, media.audio_flinger (mediaserver)
- drm.drmManager (drmserver)
- android.security.keystore (keystore)
- SurfaceFlinger (surfaceflinger)
bool Parcel::enforceInterface(const String16& interface,
IPCThreadState* threadState) const
{
int32_t strictPolicy = readInt32();
if (threadState == NULL) {
threadState = IPCThreadState::self();
}
if ((threadState->getLastTransactionBinderFlags() &
IBinder::FLAG_ONEWAY) != 0) {
// For one-way calls, the callee is running entirely
// disconnected from the caller, so disable StrictMode entirely.
// Not only does disk/network usage not impact the caller, but
// there's no way to commuicate back any violations anyway.
threadState->setStrictModePolicy(0);
} else {
threadState->setStrictModePolicy(strictPolicy);
}
const String16 str(readString16());
if (str == interface) {
return true;
} else {
ALOGW("**** enforceInterface() expected '%s' but read '%s'",
String8(interface).string(), String8(str).string());
return false;
}
}
Proofs of Concept:
https://github.com/offensive-security/exploit-database-bin-sploits/raw/master/sploits/40354.zip
