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

[usr1224]

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.

[clover2123]

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

[usr1224]

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

Done. Sorry for my mistake