Escargot::readUTF8Sequence(char const*&, bool&, int&) Heap overflow

Question

Escargot::readUTF8Sequence(char const*&, bool&, int&) Heap overflow

Opened this issue 5 months ago · 2 comments

Credit : Sunghoon Jang, Jeonil Ji.

Escargot

OS: Ubuntu 18.04

Describe the bug
Heap Buffer overflow
The patch for CVE-2024-32669 in commit bbc9fe8 introduced a new buffer overflow vulnerability. The patch for CVE-2024-32669 was incorrect.

Test case
Test code to reproduce the behavior:
readUTF8Sequence.txt

Rename before use(readUTF8Sequence.txt -> readUTF8Sequence.js)

Backtrace

==2480407==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61200000017a at pc 0x000000d0c088 bp 0x7ffe07e73160 sp 0x7ffe07e73158
READ of size 1 at 0x61200000017a thread T0 
    #0 0xd0c087 in Escargot::readUTF8Sequence(char const*&, bool&, int&) /home/hoonsecc/escargot/src/runtime/String.cpp:189:60
    #1 0xd0c087 in Escargot::utf8StringToUTF16StringNonGC[abi:cxx11](char const*, unsigned long) /home/hoonsecc/escargot/src/runtime/String.cpp:235:23
    #2 0xd15bd0 in Escargot::String::fromUTF8ToCompressibleString(Escargot::VMInstance*, char const*, unsigned long, bool) /home/hoonsecc/escargot/src/runtime/String.cpp:750:18
    #3 0xd8de5e in builtinHelperFileRead(Escargot::OptionalRef<Escargot::ExecutionStateRef>, char const*, char const*) /home/hoonsecc/escargot/src/shell/Shell.cpp:234:27
    #4 0xd9241a in main::$_2::operator()(Escargot::ExecutionStateRef*, char*) const /home/hoonsecc/escargot/src/shell/Shell.cpp:1140:41
    #5 0xd9241a in main::$_2::__invoke(Escargot::ExecutionStateRef*, char*) /home/hoonsecc/escargot/src/shell/Shell.cpp:1139:58
    #6 0x4da633 in Escargot::Evaluator::executeFunction(Escargot::ContextRef*, Escargot::ValueRef* (*)(Escargot::ExecutionStateRef*, void*, void*), void*, void*)::$_4::operator()(Escargot::ExecutionState&, void*) const /home/hoonsecc/escargot/src/api/EscargotPublic.cpp:1094:23
    #7 0x4da633 in Escargot::Evaluator::executeFunction(Escargot::ContextRef*, Escargot::ValueRef* (*)(Escargot::ExecutionStateRef*, void*, void*), void*, void*)::$_4::__invoke(Escargot::ExecutionState&, void*) /home/hoonsecc/escargot/src/api/EscargotPublic.cpp:1091:26
    #8 0xca60e6 in Escargot::SandBox::run(Escargot::Value (*)(Escargot::ExecutionState&, void*), void*) /home/hoonsecc/escargot/src/runtime/SandBox.cpp:111:25
    #9 0x4d29ab in Escargot::Evaluator::executeFunction(Escargot::ContextRef*, Escargot::ValueRef* (*)(Escargot::ExecutionStateRef*, void*, void*), void*, void*) /home/hoonsecc/escargot/src/api/EscargotPublic.cpp:1091:22
    #10 0xd861de in Escargot::Evaluator::EvaluatorResult Escargot::Evaluator::executeImpl<Escargot::ContextRef, char*>(Escargot::ContextRef*, Escargot::ValueRef* (*)(Escargot::ExecutionStateRef*, char*), char*) /home/hoonsecc/escargot/src/api/EscargotPublic.h:606:16
    #11 0xd861de in Escargot::Evaluator::EvaluatorResult Escargot::Evaluator::execute<char*, main::$_2>(Escargot::ContextRef*, main::$_2&&, char*) /home/hoonsecc/escargot/src/api/EscargotPublic.h:585:16
    #12 0xd861de in main /home/hoonsecc/escargot/src/shell/Shell.cpp:1139:30
    #13 0x7f242114d082 in __libc_start_main /build/glibc-LcI20x/glibc-2.31/csu/../csu/libc-start.c:308:16
    #14 0x42685d in _start (/home/hoonsecc/escargot/escargot+0x42685d)

0x61200000017a is located 0 bytes to the right of 314-byte region [0x612000000040,0x61200000017a)
allocated by thread T0 here:
    #0 0x4cc90d in operator new(unsigned long) (/home/hoonsecc/escargot/escargot+0x4cc90d)
    #1 0x7f24215ca38d in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_mutate(unsigned long, unsigned long, char const*, unsigned long) (/lib/x86_64-linux-gnu/libstdc++.so.6+0x14338d)

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/hoonsecc/escargot/src/runtime/String.cpp:189:60 in Escargot::readUTF8Sequence(char const*&, bool&, int&)
Shadow bytes around the buggy address:
  0x0c247fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff8000: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fff8010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c247fff8020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00[02]
  0x0c247fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c247fff8070: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==2480407==ABORTING

backtrace

backtrace#0 : readUTF8Sequence, String.cpp

*(sequence + 2) causes a heap buffer overflow READ.
The sequence pointer is pointing to the end of a variable, and accessing ptr + 2 causes the overflow by reading the value from that location.
The sequence pointer is passed as an argument when the readUTF8Sequence() function is called. Let's check where the readUTF8Sequence() function is invoked.

backtrace#1 : utf8StringToUTF16StringNonGC, String.cpp

As the function name suggests, it converts a UTF-8 string into a UTF-16 string. The function reads the UTF-8 character through readUTF8Sequence(), stores it in ch, and adds it to str after conversion.
source is fetched from the buf argument of the utf8StringToUTF16StringNonGC() function.
The source value changes through source -= (charlen - 1); in the while loop, and although it seemed like a heap overflow was happening, this didn't make sense as the pointer was decrementing.
Let's recheck backtrace#0.

backtrace#0 : readUTF8Sequence(), String.cpp

In this part, I observed that valid is set to 0, and sequence++ is performed. If valid is false, the utf8StringToUTF16StringNonGC() function adds 0xfffd to the str string and continues the while loop.
This process repeats, increasing the value of sequence, and eventually, when it approaches the end of the string, it attempts to access sequence + 3, leading to a heap overflow. Let's debug to confirm this.

The first and second lines of code set the valid variable to false.

I confirmed that the sequence value increments, and -1 is returned.

Initial state of the sequence variable in the while loop.

As the loop progresses, the value increases by 1 at a time.

This is the situation just before the ASan report is triggered. An access to the address 0x604000000030 occurs beyond the bounds of sequence, causing the heap overflow report.

The crash happens at *(sequence + 3), but accessing seq+2 is allowed to signify the end of the string.

UTF16StringDataNonGCStd(), String.cpp

The bug occurs because, although the while loop checks to escape when source surpasses buf + buf_length, there is an internal part accessing source + 3.

Credit : Sunghoon Jang, Jeonil Ji.

Answer 1 · 2024-09-13T07:15:28.000Z

Thanks for reporting
But would you please rewrite this issue in English to share it with others?

Answer 2 · 2024-09-13T10:50:29.000Z

Thanks for reporting But would you please rewrite this issue in English to share it with others?

Done. Sorry for my mistake