1988883 - AssertedCast error: Cannot cast 131070 from uint64_t to uint16_t: out of range with too many function arguments

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Shaheen Fazim]

Attachment: poc.js

Step to reproduce:
Run: ./objdir-debug/dist/bin/js poc.js
Version: JavaScript-C144.0a1 c0d953e8a00329ea7f2d98ec9ae71ff35f5f12b8

Crash:

AssertedCast error: Cannot cast 131070 from uint64_t to uint16_t: out of range
[31453] Hit MOZ_CRASH() at /home/user/firefox/objdir-noopt/dist/include/mozilla/Casting.h:256
#01: ???[../firefox/objdir-noopt/dist/bin/js +0x2f1a7d7]
#02: ???[../firefox/objdir-noopt/dist/bin/js +0x2e340c1]
#03: ???[../firefox/objdir-noopt/dist/bin/js +0x2e3406e]
#04: ???[../firefox/objdir-noopt/dist/bin/js +0x2e33f3d]
#05: ???[../firefox/objdir-noopt/dist/bin/js +0x2e31bf3]
...
#35: ??? (???:???)
Segmentation fault (core dumped)

(gdb) bt
#0  MOZ_CrashSequence (aAddress=0x0, aLine=256)
    at /home/user/firefox/objdir-noopt/dist/include/mozilla/Assertions.h:248
#1  0x000055555846e7f1 in mozilla::AssertedCast<unsigned short, unsigned long> (aFrom=131070)
    at /home/user/firefox/objdir-noopt/dist/include/mozilla/Casting.h:256
#2  0x00005555583880c1 in js::frontend::detail::InitializeIndexedBindings<js::FunctionScope::SlotInfo, unsigned short> (slotInfo=..., start=0x55555a850500, cursor=0x55555a8d04f8,
    field=&js::FunctionScope::SlotInfo::varStart, bindings=...)
    at /home/user/firefox/js/src/frontend/Parser.cpp:1048
#3  0x000055555838806e in js::frontend::detail::InitializeIndexedBindings<js::FunctionScope::SlotInfo, unsigned short, unsigned short js::FunctionScope::SlotInfo::*, mozilla::Vector<js::AbstractBindingName<js::frontend::TaggedParserAtomIndex>, 6ul, js::TempAllocPolicy>&> (slotInfo=..., start=0x55555a850500, cursor=0x55555a8904fc,
    field=&js::FunctionScope::SlotInfo::nonPositionalFormalStart, bindings=..., step=..., step=...)
    at /home/user/firefox/js/src/frontend/Parser.cpp:1053
#4  0x0000555558387f3d in js::frontend::InitializeBindingData<js::ParserScopeData<js::FunctionScope::SlotInfo>, unsigned short js::FunctionScope::SlotInfo::*, mozilla::Vector<js::AbstractBindingName<js::frontend::TaggedParserAtomIndex>, 6ul, js::TempAllocPolicy>&, unsigned short js::FunctionScope::SlotInfo::*, mozilla::Vector<js::AbstractBindingName<js::frontend::TaggedParserAtomIndex>, 6ul, js::TempAllocPolicy>&> (data=0x55555a8504f0,
    count=131070, firstBindings=..., step=..., step=..., step=..., step=...)
    at /home/user/firefox/js/src/frontend/Parser.cpp:1081
#5  0x0000555558385bf3 in js::frontend::NewFunctionScopeData (fc=0x7fffffff8758, scope=...,
    hasParameterExprs=false, alloc=..., pc=0x7fffffff7e20)
    at /home/user/firefox/js/src/frontend/Parser.cpp:1374
#6  0x0000555558385491 in js::frontend::ParserBase::newFunctionScopeData (this=0x7fffffff9138, scope=...,
    hasParameterExprs=false) at /home/user/firefox/js/src/frontend/Parser.cpp:1409
#7  0x0000555558382a1a in js::frontend::PerHandlerParser<js::frontend::FullParseHandler>::finishFunction (
    this=0x7fffffff9138, isStandaloneFunction=true) at /home/user/firefox/js/src/frontend/Parser.cpp:2166
#8  0x00005555583fee40 in js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::functionFormalParametersAndBody (this=0x7fffffff9138, inHandling=js::frontend::InAllowed,
    yieldHandling=js::frontend::YieldIsName, funNode=0x7fffffff7df0,
--Type <RET> for more, q to quit, c to continue without paging--
    kind=js::frontend::FunctionSyntaxKind::Expression, parameterListEnd=..., isStandaloneFunction=true)
    at /home/user/firefox/js/src/frontend/Parser.cpp:3677
#9  0x000055555844d397 in js::frontend::Parser<js::frontend::FullParseHandler, char16_t>::standaloneFunction (
    this=0x7fffffff9138, parameterListEnd=..., syntaxKind=js::frontend::FunctionSyntaxKind::Expression,
    generatorKind=js::GeneratorKind::NotGenerator, asyncKind=js::FunctionAsyncKind::SyncFunction,
    inheritedDirectives=..., newDirectives=0x7fffffff7fde)
    at /home/user/firefox/js/src/frontend/Parser.cpp:2399
#10 0x00005555584c63e0 in StandaloneFunctionCompiler<char16_t>::parse (this=0x7fffffff8840, cx=0x555559e9a090,
    syntaxKind=js::frontend::FunctionSyntaxKind::Expression, generatorKind=js::GeneratorKind::NotGenerator,
    asyncKind=js::FunctionAsyncKind::SyncFunction, parameterListEnd=...)
    at /home/user/firefox/js/src/frontend/BytecodeCompiler.cpp:1083
#11 0x00005555584c4f49 in StandaloneFunctionCompiler<char16_t>::compile (this=0x7fffffff8840,
    cx=0x555559e9a090, syntaxKind=js::frontend::FunctionSyntaxKind::Expression,
    generatorKind=js::GeneratorKind::NotGenerator, asyncKind=js::FunctionAsyncKind::SyncFunction,
    parameterListEnd=...) at /home/user/firefox/js/src/frontend/BytecodeCompiler.cpp:1108
#12 0x0000555558485b8a in CompileStandaloneFunction (cx=0x555559e9a090, options=..., srcBuf=...,
    parameterListEnd=..., syntaxKind=js::frontend::FunctionSyntaxKind::Expression,
    generatorKind=js::GeneratorKind::NotGenerator, asyncKind=js::FunctionAsyncKind::SyncFunction,
    enclosingScope=...) at /home/user/firefox/js/src/frontend/BytecodeCompiler.cpp:1708
#13 0x00005555584857f3 in js::frontend::CompileStandaloneFunction (cx=0x555559e9a090, options=..., srcBuf=...,
    parameterListEnd=..., syntaxKind=js::frontend::FunctionSyntaxKind::Expression)
    at /home/user/firefox/js/src/frontend/BytecodeCompiler.cpp:1755

#14 0x0000555557bc968e in CreateDynamicFunction (cx=0x555559e9a090, args=...,
    generatorKind=js::GeneratorKind::NotGenerator, asyncKind=js::FunctionAsyncKind::SyncFunction)
    at /home/user/firefox/js/src/vm/JSFunction.cpp:1530
#15 0x0000555557bc89a0 in js::Function (cx=0x555559e9a090, argc=2, vp=0x555559fd8c70)
    at /home/user/firefox/js/src/vm/JSFunction.cpp:1571
#16 0x00005555579100d5 in CallJSNative (cx=0x555559e9a090,
    native=0x555557bc8940 <js::Function(JSContext*, unsigned int, JS::Value*)>, reason=js::CallReason::Call,
--Type <RET> for more, q to quit, c to continue without paging--
    args=...) at /home/user/firefox/js/src/vm/Interpreter.cpp:501
#17 0x000055555791fb8a in CallJSNativeConstructor (cx=0x555559e9a090,
    native=0x555557bc8940 <js::Function(JSContext*, unsigned int, JS::Value*)>, args=...)
    at /home/user/firefox/js/src/vm/Interpreter.cpp:519
#18 0x00005555578e90ab in InternalConstruct (cx=0x555559e9a090, args=..., reason=js::CallReason::Call)
    at /home/user/firefox/js/src/vm/Interpreter.cpp:725
#19 0x00005555578e8b8a in js::ConstructFromStack (cx=0x555559e9a090, args=..., reason=js::CallReason::Call)
    at /home/user/firefox/js/src/vm/Interpreter.cpp:772
#20 0x00005555578f74ed in js::Interpret (cx=0x555559e9a090, state=...)
    at /home/user/firefox/js/src/vm/Interpreter.cpp:3272
#21 0x00005555578e7b9d in MaybeEnterInterpreterTrampoline (cx=0x555559e9a090, state=...)
    at /home/user/firefox/js/src/vm/Interpreter.cpp:395
#22 0x00005555578e78a0 in js::RunScript (cx=0x555559e9a090, state=...)
    at /home/user/firefox/js/src/vm/Interpreter.cpp:471
#23 0x00005555578e9b19 in js::ExecuteKernel (cx=0x555559e9a090, script=..., envChainArg=..., evalInFrame=...,
    result=...) at /home/user/firefox/js/src/vm/Interpreter.cpp:862
#24 0x00005555578e9e32 in js::Execute (cx=0x555559e9a090, script=..., envChain=..., rval=...)
    at /home/user/firefox/js/src/vm/Interpreter.cpp:895
#25 0x0000555557ab618a in ExecuteScript (cx=0x555559e9a090, envChain=..., script=..., rval=...)
    at /home/user/firefox/js/src/vm/CompilationAndEvaluation.cpp:548
#26 0x0000555557ab62a5 in JS_ExecuteScript (cx=0x555559e9a090, scriptArg=...)
    at /home/user/firefox/js/src/vm/CompilationAndEvaluation.cpp:572
#27 0x00005555577eaf76 in RunFile (cx=0x555559e9a090, filename=0x555559fbb2d0 "mz.js", file=0x555559fbb330,
    compileMethod=CompileUtf8::DontInflate, compileOnly=false, fullParse=false)
    at /home/user/firefox/js/src/shell/js.cpp:1315
#28 0x00005555577ea8be in Process (cx=0x555559e9a090, filename=0x555559fbb2d0 "mz.js", forceTTY=false,
    kind=FileScript) at /home/user/firefox/js/src/shell/js.cpp:2089
#29 0x00005555577bf503 in ProcessArgs (cx=0x555559e9a090, op=0x7fffffffdd70)
    at /home/user/firefox/js/src/shell/js.cpp:12010
--Type <RET> for more, q to quit, c to continue without paging--
#30 0x00005555577aee98 in Shell (cx=0x555559e9a090, op=0x7fffffffdd70)
    at /home/user/firefox/js/src/shell/js.cpp:12263
#31 0x00005555577aa20b in main (argc=2, argv=0x7fffffffdfe8) at /home/user/firefox/js/src/shell/js.cpp:12666

Summary:
A critical integer overflow vulnerability exists within the NewFunctionScopeData function of the SpiderMonkey JavaScript engine's frontend parser. The flaw is triggered when parsing a function with a large number of destructured parameters (65,535). The engine's logic incorrectly calculates the total number of parameter bindings, arriving at the sum of 131070.

This value overflows the uint16_t field designated for storing the binding count. In a debug build, this crashes with assertion. I believe, in release build, the value is silently truncated to a critical state mismatch where the engine believes the function has zero parameters while the call stack contains thousands. This memory corruption can be exploited by malicious code to hijack the program's control flow, leading to remote code execution.

Comment 1

[Tooru Fujisawa [:arai]]

Thank you for reporting!

So, the issue comes from the fact that FunctionScope::SlotInfo::varStart is uint16_t type, but we don't verify the number of non-positional formals.

https://searchfox.org/firefox-main/rev/ff87356d65bc2e9e7349ac3a24276696434f27fc/js/src/vm/Scope.h#654,663,703-704

class FunctionScope : public Scope {
...
  struct SlotInfo {
...
    uint16_t nonPositionalFormalStart = 0;
    uint16_t varStart = 0;

The overflow happens in the following code path:

https://searchfox.org/firefox-main/rev/ff87356d65bc2e9e7349ac3a24276696434f27fc/js/src/frontend/Parser.cpp#1279-1281,1364-1365,1367-1368,1374-1377

static Maybe<FunctionScope::ParserData*> NewFunctionScopeData(
    FrontendContext* fc, ParseContext::Scope& scope, bool hasParameterExprs,
    LifoAlloc& alloc, ParseContext* pc) {
...
  uint32_t numBindings =
      positionalFormals.length() + formals.length() + vars.length();
...
  if (numBindings > 0) {
    bindings = NewEmptyBindingData<FunctionScope>(fc, alloc, numBindings);
...
    InitializeBindingData(
        bindings, numBindings, positionalFormals,
        &ParserFunctionScopeSlotInfo::nonPositionalFormalStart, formals,
        &ParserFunctionScopeSlotInfo::varStart, vars);

https://searchfox.org/firefox-main/rev/ff87356d65bc2e9e7349ac3a24276696434f27fc/js/src/frontend/Parser.cpp#1068-1071,1081-1082

template <class Data, typename... Step>
static MOZ_ALWAYS_INLINE void InitializeBindingData(
    Data* data, uint32_t count, const ParserBindingNameVector& firstBindings,
    Step&&... step) {
...
      detail::InitializeIndexedBindings(data->slotInfo, start, cursor,
                                        std::forward<Step>(step)...);

https://searchfox.org/firefox-main/rev/ff87356d65bc2e9e7349ac3a24276696434f27fc/js/src/frontend/Parser.cpp#1042-1048,1053-1054

template <class SlotInfo, typename UnsignedInteger, typename... Step>
static MOZ_ALWAYS_INLINE ParserBindingName* InitializeIndexedBindings(
    SlotInfo& slotInfo, ParserBindingName* start, ParserBindingName* cursor,
    UnsignedInteger SlotInfo::* field, const ParserBindingNameVector& bindings,
    Step&&... step) {
  slotInfo.*field =
      AssertedCast<UnsignedInteger>(PointerRangeSize(start, cursor));
...
  return InitializeIndexedBindings(slotInfo, start, newCursor,
                                   std::forward<Step>(step)...);

This logic is used for initializing js::FunctionScope::SlotInfo::nonPositionalFormalStart and FunctionScope::SlotInfo::varStart fields.
Those values indicates where the slot for those variables starts within the bindings array.

In the function scope, the bindings array consists of the 3 groups:

https://searchfox.org/firefox-main/rev/ff87356d65bc2e9e7349ac3a24276696434f27fc/js/src/vm/Scope.h#700-702

// positional formals - [0, nonPositionalFormalStart)
//      other formals - [nonPositionalParamStart, varStart)
//               vars - [varStart, length)

The length there is AbstractBaseScopeData::length, which is uint32_t.

https://searchfox.org/firefox-main/rev/ff87356d65bc2e9e7349ac3a24276696434f27fc/js/src/vm/Scope.h#220-221,226

template <typename NameT>
class AbstractBaseScopeData {
...
  uint32_t length = 0;

We verify the number of positional formals in GeneralParser::functionArguments, so nonPositionalFormalStart fits uint16_t:

https://searchfox.org/firefox-main/rev/ff87356d65bc2e9e7349ac3a24276696434f27fc/js/src/frontend/Parser.cpp#2654-2657,2836-2839

template <class ParseHandler, typename Unit>
bool GeneralParser<ParseHandler, Unit>::functionArguments(
    YieldHandling yieldHandling, FunctionSyntaxKind kind,
    FunctionNodeType funNode) {
...
      if (positionalFormals.length() >= ARGNO_LIMIT) {
        error(JSMSG_TOO_MANY_FUN_ARGS);
        return false;
      }

But we don't check the number of non-positional formals and vars.

Thus, varStart can overflow from uint16_t.

length won't easily overflow given it's uint32_t.

Then, the varStart is passed to BaseAbstractBindingIter::init:

https://searchfox.org/firefox-main/rev/ff87356d65bc2e9e7349ac3a24276696434f27fc/js/src/vm/Scope.cpp#1060-1063

init(/* positionalFormalStart= */ 0,
     /* nonPositionalFormalStart= */ slotInfo.nonPositionalFormalStart,
     /* varStart= */ slotInfo.varStart,
     /* letStart= */ length,

and ultimately used as a condition to determine if a slot is a variable or a formal parameter.

https://searchfox.org/firefox-main/rev/ff87356d65bc2e9e7349ac3a24276696434f27fc/js/src/vm/Scope.h#1290-1297,1300

  void init(uint32_t positionalFormalStart, uint32_t nonPositionalFormalStart,
            uint32_t varStart, uint32_t letStart, uint32_t constStart,
#ifdef ENABLE_EXPLICIT_RESOURCE_MANAGEMENT
            uint32_t usingStart,
#endif
            uint32_t syntheticStart, uint32_t privateMethodStart, uint8_t flags,
            uint32_t firstFrameSlot, uint32_t firstEnvironmentSlot,
            mozilla::Span<AbstractBindingName<NameT>> names) {
...
    varStart_ = varStart;

https://searchfox.org/firefox-main/rev/ff87356d65bc2e9e7349ac3a24276696434f27fc/js/src/vm/Scope.h#1477-1489

BindingKind kind() const {
  MOZ_ASSERT(!done());
  if (index_ < positionalFormalStart_) {
    return BindingKind::Import;
  }
  if (index_ < varStart_) {
    // When the parameter list has expressions, the parameters act
    // like lexical bindings and have TDZ.
    if (hasFormalParameterExprs()) {
      return BindingKind::Let;
    }
    return BindingKind::FormalParameter;
  }

So, the issue confuses the logic, as varStart_ becomes smaller than the actual value, which results in treating some formal parameter slots as var slots.

But so far it won't result in the state mismatch around the number of values on the stack, which is explained in the comment #0, given the total number (length field, or the number of formals) is still the correct value. If you have any other PoC that actually exploits this issue further than hitting the assertion, please let us know.

Anyway, we should check the range of nonPositionalFormalStart, to make sure the varStart fits the storage.

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: (secure)

Comment 3

[Shaheen Fazim]

Attachment: state-corruption.js

Sorry about the summary. I believe I’m able to get state corruption in the release build.

Comment 4

[Tooru Fujisawa [:arai]]

Thank you for providing more details.

Then, can you explain a bit more about the testcase, in terms of where the collision happens and what the expected behavior?

In the testcase:

• there are many destructuring parameters, named pN, where N is in [0, 65535) range, thus the last name is p65534.
• there's one function-body-level local var variable, named p65534, this is actually treated as the same binding as the parameter's one, given the environment for those are the same, and they share the same variable name
• the function body sets the p65534 to "corrupted_by_var", and then returns the variable
• the testcase seems to expect "original_value" being returned, but that won't happen, p65534 is overwritten

so, to my understanding, the testcase isn't exploiting anything.

Let me know if there's anything I'm missing.
if you found another possibly-long testcase and the attached one is supposed to be a minimal testcase, can you attach the long one?
For example, if the body-level variable name is different one and writing into it modifies p65534 value, that may prove some corruption.

Comment 5

[Shaheen Fazim]

Yes if we have a function parameter and a var declaration with the same name inside that function, they refer to the same binding. The last assignment wins. So, seeing the function return "corrupted_by_var" is, on the surface, expected language behavior.

But the vulnerability isn't that the variable was overwritten. The vulnerability is the underlying logical flaw that allowed this situation to occur under these specific circumstances.

• The Root Cause: The bug is a type confusion in the C++ engine. Due to the integer overflow, the engine internally and incorrectly classifies the slot for the {p65534} parameter as being a var.
• The Proof: The PoC is designed to create a conflict that only a bug could cause. The engine should have treated {p65534} purely as a formal parameter. The fact that the var declaration was able to override it proves that the engine's internal state was already corrupted by the type confusion.

Comment 6

[Shaheen Fazim]

Yeah, I was completely wrong I was just trying. Sorry about that. I take back the two comments above. For now, I can only reproduce a crash from the fuzzer, since I’m not knowledgeable enough to trigger a corruption or exploit. Thank you.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2def4302110f
https://hg.mozilla.org/integration/autoland/rev/e81e25563000
Tighten the range check for the function scope. r=bthrall

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e81e25563000