New issue
Advanced search Search tips

Issue 840 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:



Sign in to add a comment

Android - libutils UTF16 to UTF8 conversion heap-buffer-overflow

Project Member Reported by markbrand@google.com, Jun 9 2016

Issue description

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

This bug is subject to a 90 day disclosure deadline. If 90 days elapse
without a broadly available patch, then the bug report will automatically
become visible to the public.

 
keystore_utf16_to_utf8.c
13.7 KB View Download
Project Member

Comment 2 by markbrand@google.com, Jun 9 2016

And related issue https://code.google.com/p/android/issues/detail?id=212626 reported without deadline since there's not an obvious path to exploitation.
Project Member

Comment 3 by markbrand@google.com, Jun 14 2016

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;
    }
}
generic_poc.c
13.2 KB View Download
generic_poc
3.0 MB View Download
Project Member

Comment 4 by markbrand@google.com, Jul 28 2016

Updating bug with additional conversations had with Android team (15/06/07); re. confirming remote exploitability via stagefright. 

PoC MP4 file attached here.
poc.mp4
6.1 KB View Download
Project Member

Comment 5 by markbrand@google.com, Sep 7 2016

Labels: -Severity-High CVE-2016-3861 Severity-HIgh Fixed-2016-Sep-6
Status: Fixed (was: New)
Fixed in https://source.android.com/security/bulletin/2016-09-01.html
Project Member

Comment 6 by markbrand@google.com, Sep 7 2016

Attaching the code for the blog post.
poc.tar.gz
30.0 KB Download
Project Member

Comment 7 by hawkes@google.com, Sep 7 2016

Labels: -Restrict-View-Commit
Project Member

Comment 8 by hawkes@google.com, Sep 7 2016

Looks like jduck spotted the underlying src increment issue last year: https://gist.github.com/jduck/6c217beed59d23381385e770c4e1f37f
hello ,how can I build this poc use aosp source code
Project Member

Comment 10 by markbrand@google.com, Sep 27 2016

The file generic_poc.c just needs to be compiled as a static executable; you can just use the standard NDK toolchain. It might also need the binder.c/binder.h from service_manager, I don't remember, but I often use those. Compile command would look something like

/android/ndk/toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/bin/aarch64-linux-android-gcc --sysroot=/android/ndk/platforms/android-24/arch-arm64 -std=c99 -fPIE -static binder.c generic_poc.c -o generic_poc

The blog post poc is python, and it shouldn't need to be compiled.

Sign in to add a comment