GetObject: object-key conversion causes an out-of-bounds fast-array read

[str8outtaheap]

get-object-fast-array-reentrancy-oob-read-poc.js

GetObject: object-key conversion causes an out-of-bounds fast-array read

Summary

The GetObject bytecode checks that an array uses fast storage before converting an attacker-controlled property key into an array index. The key's toString() method can switch the array to slow storage and enlarge its logical length. The interpreter then indexes the one-element placeholder used for slow arrays as if it were the old element buffer.

The attached PoC triggers an out-of-bounds read in the interpreter. The project's ASan build reports a SEGV in EncodedValue::toValue<true>().

Affected Version

Escargot v4.3.0 at commit 1f3079ea4b9688fa5edc777e6aacd180436eba3e, tested on Ubuntu 24.04 aarch64 in both the default release ESCARGOT_USE_32BIT_IN_64BIT layout and the project-supported ASan layout.

Steps to Reproduce

cmake -S . -B build-asan -G Ninja \
  -DCMAKE_POLICY_VERSION_MINIMUM=3.5 \
  -DESCARGOT_ARCH=aarch64 \
  -DESCARGOT_MODE=release \
  -DESCARGOT_OUTPUT=shell \
  -DESCARGOT_LIBICU_SUPPORT=ON \
  -DESCARGOT_THREADING=ON \
  -DESCARGOT_ASAN=ON

cmake --build build-asan --parallel

./build-asan/escargot \
  poc/get-object-fast-array-reentrancy-oob-read-poc.js

Expected Result

Property 4294967294 is absent after key conversion, so the expression should return undefined.

Actual Result

AddressSanitizer:DEADLYSIGNAL
=================================================================
==1==ERROR: AddressSanitizer: SEGV on unknown address 0xaab2b380f2f0 (pc 0xaaaab2c10130 bp 0xffffdb534fa0 sp 0xffffdb534fa0 T0)
==1==The signal is caused by a READ memory access.
    #0 0xaaaab2c10130 in Escargot::Value Escargot::EncodedValue::toValue<true>() const /src/src/runtime/EncodedValue.h:259
    #1 0xaaaab2c10130 in Escargot::Interpreter::interpret(Escargot::ExecutionState*, Escargot::ByteCodeBlock*, unsigned long, Escargot::Value*) /src/src/interpreter/ByteCodeInterpreter.cpp:586
    #2 0xaaaab2d821c4 in Escargot::Script::execute(Escargot::ExecutionState&, bool, bool) /src/src/parser/Script.cpp:500
    #3 0xaaaab29ca860 in Escargot::ScriptRef::execute(Escargot::ExecutionStateRef*) /src/src/api/EscargotPublic.cpp:5067
    #4 0xaaaab29c35bc in operator() /src/src/api/EscargotPublic.cpp:1350
    #5 0xaaaab29c35bc in _FUN /src/src/api/EscargotPublic.cpp:1351
    #6 0xaaaab30bb51c in Escargot::SandBox::run(Escargot::Value (*)(Escargot::ExecutionState&, void*), void*) /src/src/runtime/SandBox.cpp:111
    #7 0xaaaab29c801c in Escargot::Evaluator::executeFunction(Escargot::ContextRef*, Escargot::ValueRef* (*)(Escargot::ExecutionStateRef*, void*, void*), void*, void*) /src/src/api/EscargotPublic.cpp:1352
    #8 0xaaaab31980a8 in Escargot::Evaluator::EvaluatorResult Escargot::Evaluator::executeImpl<Escargot::ContextRef, Escargot::ScriptRef*>(Escargot::ContextRef*, Escargot::ValueRef* (*)(Escargot::ExecutionStateRef*, Escargot::ScriptRef*), Escargot::ScriptRef*) /src/src/api/EscargotPublic.h:672
    #9 0xaaaab31980a8 in execute<Escargot::ScriptRef*, evalScript(Escargot::ContextRef*, Escargot::StringRef*, Escargot::StringRef*, bool, bool)::<lambda(Escargot::ExecutionStateRef*, Escargot::ScriptRef*)> > /src/src/api/EscargotPublic.h:645
    #10 0xaaaab31980a8 in evalScript /src/src/shell/Shell.cpp:947
    #11 0xaaaab29bf56c in main /src/src/shell/Shell.cpp:1319
    #12 0xffffb6f584c0 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #13 0xffffb6f58594 in __libc_start_main_impl ../csu/libc-start.c:360
    #14 0xaaaab29c07ac in _start (/work/builds/asan/escargot+0x1107ac) (BuildId: 3d038057658ef0c271a54b5dc1229c33a17746e0)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /src/src/runtime/EncodedValue.h:259 in Escargot::Value Escargot::EncodedValue::toValue<true>() const
==1==ABORTING

Root Cause

Interpreter::interpret records the array's fast-mode state before tryToUseAsIndexProperty runs attacker JavaScript. It does not recheck the storage mode before reading the element directly:

Value Interpreter::interpret(
    ExecutionState* state,
    ByteCodeBlock* byteCodeBlock,
    size_t programCounter,
    Value* registerFile)
{
    // ...
    ArrayObject* arr = reinterpret_cast<ArrayObject*>(obj);
    if (LIKELY(arr->isFastModeArray())) {
        // Attacker-controlled object-key conversion runs here.
        uint32_t idx = property.tryToUseAsIndexProperty(*state);
        if (LIKELY(idx < arr->arrayLength(*state))) {
            // Uses the backing pointer after conversion changed it.
            registerFile[code->m_storeRegisterIndex] =
                arr->m_fastModeData[idx].toValue<true>();
        }
    }
    // ...
}

The callback enters ArrayObject::convertIntoNonFastMode, which replaces the element pointer with a one-element placeholder:

void ArrayObject::convertIntoNonFastMode(ExecutionState& state)
{
    // ...
    TightVectorWithNoSize<
        ObjectPropertyValue,
        CustomAllocator<ObjectPropertyValue>> tempFastModeData(
            std::move(m_fastModeData));
    m_fastModeData.reset(&ArrayObject::DummyArrayElement);
    // ...
}

The callback also sets the logical length to 0xffffffff, so the bounds check accepts an index that is outside the placeholder's one-element storage.

Suggested Fix

Do not use the fast-array shortcut for an object key that can run JavaScript during conversion. Alternatively, recheck fast mode after conversion and fall back to ordinary property lookup if the storage mode changed. The fix for issue #1597 applies the same object-key restriction in SetObjectOperation.