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