Closed Bug 2029435 Opened 7 months ago Closed 6 months ago

Assertion failure: pcOffset < isd->codeLength() in [@ js::jit::IonEntry::callStackAtAddr] due to weak script dedup key

Categories

(Core :: JavaScript Engine: JIT, defect, P3)

defect

Tracking

()

RESOLVED FIXED
152 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox-esr140 --- unaffected
firefox150 --- wontfix
firefox151 --- wontfix
firefox152 --- fixed

People

(Reporter: bugmon, Assigned: jandem)

References

(Blocks 2 open bugs)

Details

(5 keywords, Whiteboard: [adv-main152+r])

Attachments

(4 files, 1 external file)

The Ion native-to-bytecode map deduplicates inlined scripts using ScriptSourceAndExtent::matches(), which compares only (ScriptSource*, toStringStart, toStringEnd). This key is not unique: a private instance accessor (e.g. get #x() {...}) and the parser-synthesized private-method-initializer lambda created for it share the exact same ScriptSource and toString extent while having completely different bytecode (different SharedImmutableScriptData and codeLength).

When Ion inlines both into the same compilation, createNativeToBytecodeScriptList() in CodeGenerator-shared.cpp adds only the first one encountered to the scriptList. Later, JitcodeRegionEntry::WriteRun() looks up the other script by the same weak key, gets the wrong scriptIdx, and encodes a pcOffset taken from the larger script against an entry whose stored sharedData belongs to the smaller script. At sample time, IonEntry::callStackAtAddr() reads back (scriptIdx, pcOffset), fetches the wrong ImmutableScriptData, and computes pc = code + pcOffset with pcOffset >= isd->codeLength(), tripping the debug assertion.

In release builds the out-of-range pc is passed to PCToLineNumber, which only computes target = pc - code and iterates the bounded source-note range [notes, notesEnd); the pc pointer is never dereferenced. Consequently the release-build impact is limited to incorrect line/column numbers reported to the Gecko Profiler, with no memory-safety violation. This is a genuine engine-invariant violation reachable from JS with profiling enabled, but of low security severity.

Build Info

Affected Code

File: js/src/jit/JitcodeMap.h, line 219-234

struct ScriptSourceAndExtent {
  RefPtr<ScriptSource> scriptSource;
  uint32_t toStringStart;
  uint32_t toStringEnd;

  explicit ScriptSourceAndExtent(JSScript* script)
      : scriptSource(script->scriptSource()),
        toStringStart(script->toStringStart()),
        toStringEnd(script->toStringEnd()) {}

  bool matches(JSScript* script) const {
    return scriptSource == script->scriptSource() &&
           toStringStart == script->toStringStart() &&
           toStringEnd == script->toStringEnd();
  }
};

File: js/src/jit/shared/CodeGenerator-shared.cpp, line 758-777

InlineScriptTree* tree = gen->outerInfo().inlineScriptTree();
for (;;) {
  // Add script from current tree.
  bool found = false;
  for (uint32_t i = 0; i < scripts.length(); i++) {
    if (scripts[i].scriptData.sourceAndExtent.matches(tree->script())) {
      found = true;   // BUG: matches a *different* JSScript with same extent
      break;
    }
  }
  if (!found) {
    UniqueChars str =
        GeckoProfilerRuntime::allocProfileString(cx, tree->script());
    if (!str) {
      return false;
    }
    if (!scripts.emplaceBack(tree->script(), std::move(str))) {
      return false;
    }
  }

File: js/src/jit/JitcodeMap.cpp, line 748-764

for (uint8_t i = 0; i < scriptDepth; i++) {
  // Find the index of the script within the list.
  // NB: scriptList is guaranteed to contain curTree->script()
  uint32_t scriptIdx = 0;
  for (; scriptIdx < scriptList.length(); scriptIdx++) {
    if (scriptList[scriptIdx].scriptData.sourceAndExtent.matches(
            curTree->script())) {
      break;   // BUG: may resolve to entry with different sharedData
    }
  }
  MOZ_ASSERT(scriptIdx < scriptList.length());

  uint32_t pcOffset = curTree->script()->pcToOffset(curPc);
  ...
  WriteScriptPc(writer, scriptIdx, pcOffset);

File: js/src/jit/JitcodeMap.cpp, line 101-116

if (count == 0) {
  pcOffset = region.findPcOffset(ptrOffset, pcOffset);
}

const IonScriptData& scriptData = getScriptData(scriptIdx);
ImmutableScriptData* isd = scriptData.sharedData->get();
jsbytecode* code = isd->code();
jsbytecode* pc = code + pcOffset;
MOZ_ASSERT(pcOffset < isd->codeLength());   // <-- fires

SrcNote* notes = isd->notes();
SrcNote* notesEnd = notes + isd->noteLength();

JS::LimitedColumnNumberOneOrigin col;
uint32_t line = PCToLineNumber(scriptData.lineno, scriptData.column, notes,
                               notesEnd, code, pc, &col);

File: js/src/vm/JSScript.cpp, line 2860-2867

ptrdiff_t offset = 0;
ptrdiff_t target = pc - code;   // pc only used arithmetically
for (SrcNoteIterator iter(notes, notesEnd); !iter.atEnd(); ++iter) {
  const auto* sn = *iter;
  offset += sn->delta();
  if (offset > target) {
    break;
  }

The deduplication key (scriptSource, toStringStart, toStringEnd) is insufficient to distinguish a private accessor body from its synthesized private-method-initializer lambda. The wrong scriptIdx is recorded with a pcOffset belonging to a different, larger bytecode buffer; on readback the pcOffset exceeds the stored script's codeLength().

Exploit Chain

  1. Page enables the Gecko Profiler (or user has it enabled) so JIT code emits native-to-bytecode maps.
  2. Define a class with a private instance accessor get #x() whose body has a large bytecode size, and use it from a method; also instantiate the class so the synthesized private-method-initializer lambda runs in the constructor.
  3. Call both paths from one outer function in a hot loop so Ion compiles the outer function and inlines both the tiny initializer lambda and the large getter body into the same IonScript.
  4. createNativeToBytecodeScriptList walks the InlineScriptTree; because InlineScriptTree::addCallee prepends children (LIFO), arranging c.read() before new C() in source order causes the constructor branch (and thus the initializer lambda) to be visited first in DFS and added to scriptList; when the getter body is visited, ScriptSourceAndExtent::matches() returns true against the initializer entry, so no new entry is added.
  5. JitcodeRegionEntry::WriteRun encodes (scriptIdx_of_initializer, pcOffset_in_getter_body) for frames inside the inlined getter.
  6. A profiler sample (or readGeckoProfilingStack) lands inside the inlined getter; IonEntry::callStackAtAddr reads back scriptIdx and pcOffset, loads the initializer's ImmutableScriptData (small codeLength), computes pc = code + pcOffset where pcOffset >= codeLength().
  7. Debug builds hit MOZ_ASSERT(pcOffset < isd->codeLength()); release builds proceed into PCToLineNumber which iterates a bounded source-note array using pc only as an arithmetic target, yielding an incorrect line/column in profiler output.

Steps to Reproduce

  1. Build a debug SpiderMonkey JS shell.
  2. Save the testcase as testcase.js.
  3. Run: js --fuzzing-safe --fast-warmup --no-threads testcase.js
  4. Observe: Assertion failure: pcOffset < isd->codeLength(), at js/src/jit/JitcodeMap.cpp:109

Security Impact

  • Severity: Low
  • Attacker capability: With the Gecko Profiler active, an attacker can cause the profiler to compute a jsbytecode* that points past the end of one script's bytecode buffer. However, that pointer is never dereferenced: PCToLineNumber only subtracts it from the buffer base and iterates a separately-bounded source-note array. The practical effect in release builds is incorrect line/column numbers in profiler samples for the affected inlined frame. In debug builds it is a reliable assertion-failure DoS.
  • Preconditions: Gecko Profiler (JS stack sampling) must be enabled for the content runtime; Ion must compile a function that inlines both a private instance accessor body and its synthesized private-method-initializer lambda; a sample (or readGeckoProfilingStack) must land inside the inlined accessor body.

ASAN Report

[761] Assertion failure: pcOffset < isd->codeLength(), at /firefox/js/src/jit/JitcodeMap.cpp:109
#01: ???[/firefox/obj-js-debug/dist/bin/js +0x3997b8b]
#02: JS::ProfilingFrameIterator::extractStack(JS::ProfilingFrameIterator::Frame*, unsigned int, unsigned int) const[/firefox/obj-js-debug/dist/bin/js +0x3c7b6d1]
#03: ???[/firefox/obj-js-debug/dist/bin/js +0x29dc537]
#04: ??? (???:???)
UndefinedBehaviorSanitizer:DEADLYSIGNAL
==761==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x575a033f7ba3 bp 0x7ffe248430c0 sp 0x7ffe24842fd0 T761)
==761==The signal is caused by a WRITE memory access.
==761==Hint: address points to the zero page.
    #0 0x575a033f7ba3 in MOZ_CrashSequence(void*, long) /firefox/obj-js-debug/dist/include/mozilla/Assertions.h:235:3
    #1 0x575a033f7ba3 in js::jit::IonEntry::callStackAtAddr(void*, js::jit::CallStackFrameInfo*, unsigned int) const /firefox/js/src/jit/JitcodeMap.cpp:109:5
    #2 0x575a036db6d0 in JS::ProfilingFrameIterator::extractStack(JS::ProfilingFrameIterator::Frame*, unsigned int, unsigned int) const /firefox/js/src/vm/Stack.cpp:773:27
    #3 0x575a0243c536 in ReadGeckoProfilingStack(JSContext*, unsigned int, JS::Value*) /firefox/js/src/builtin/TestingFunctions.cpp:5154:26
    #4 0x0cfc080db2f8  (<unknown module>)

UndefinedBehaviorSanitizer can not provide additional info.
SUMMARY: UndefinedBehaviorSanitizer: SEGV /firefox/obj-js-debug/dist/include/mozilla/Assertions.h:235:3 in MOZ_CrashSequence(void*, long)
==761==ABORTING

Attached file testcase.js —
Attached file flags.txt —
Attached patch fix.patch — — Splinter Review
Attached file crash_stack.txt —
Group: core-security → javascript-core-security
Severity: -- → S3
Priority: -- → P3
Flags: needinfo?(jdemooij)
Attached file (secure) —

This fixes an issue with private field accessors and Ion scripts, because the
scriptSource/toStringStart/toStringEnd fields are not sufficient to uniquely
identify functions created for this.

Assignee: nobody → jdemooij
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(jdemooij)

https://hg-edge.mozilla.org/mozilla-central/rev/8c4227083f79

Doesn't seem like this is worth uplifting, but feel free to nominate if you feel otherwise.

Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 152 Branch
QA Whiteboard: [sec] [qa-triage-done-c153/b152]
Whiteboard: [adv-main152+r]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: