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 8191a0f08191a108x1 0000000000000000x2 0000000000000000x3 0000000000000020
x4 00000000ffffffa0x5 0000000000000010x6 0000000000000001x7 0000007f802c0018
x8 0000000000000000x9 000000000a7c5ac5x100000000000000000x110000000000000000
x12000000000000d841x130000000000000841x140000000000000041x150000007f8067bd9e
x160000005565984f08x170000007f80aeee48x1800000000ffffff91x190000007fd1de26c0
x208191a0f08191a108x218191a0f08191a0f0x220000000000000000x230000005565984000
x248191a0f08191a0f0x250000007fd1dea7b8x260000007f806690e0x270000007fd1de25d0
x28000000556596f000x290000007fd1de2550x300000005565961188
sp 0000007fd1de2550pc 0000007f80aeee58pstate 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://gitlab.com/exploit-database/exploitdb-bin-sploits/-/raw/main/bin-sploits/40354.zip