2009303 - (CVE-2026-4701) Assertion failure: p, at debugger/DebugScript.cpp:116

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: debug stack

gczeal(23);
callFunctionWithAsyncStack(
  function () {
    for (var i = String; --i; ) {}
    var x = newGlobal();
    var y = Debugger(x);
    y.onNewScript = function (a) {
      a.setBreakpoint(0, a);
    };
    x.eval("");
    for (var j = Uint8Array; 1; ) {
      return y;
    }
  },
  saveStack(),
  "a",
).clearAllBreakpoints();

(gdb) bt
#0  0x00005555575f8343 in MOZ_CrashSequence (aAddress=0x0, aLine=116)
    at /home/msf2/shell-cache/js-dbg-64-linux-x86_64-974b0546f470-602806/objdir-js/dist/include/mozilla/Assertions.h:237
#1  js::DebugScript::getUnbarriered (script=0x3c43f41ab060) at /home/msf2/trees/firefox/js/src/debugger/DebugScript.cpp:116
#2  0x00005555575f8c9f in js::DebugScript::destroyBreakpointSite (gcx=0x7ffff5e2e9e8, script=0x0, pc=0x7ffff7804563 <_IO_2_1_stderr_+131> "")
    at /home/msf2/trees/firefox/js/src/debugger/DebugScript.cpp:229
#3  0x0000555557621675 in js::BreakpointSite::destroyIfEmpty (this=0x7ffff5e24730, gcx=0x7ffff5e2e9e8)
    at /home/msf2/trees/firefox/js/src/debugger/Debugger.h:1505
#4  js::Breakpoint::remove (this=<optimized out>, gcx=0x7ffff5e2e9e8) at /home/msf2/trees/firefox/js/src/debugger/Debugger.cpp:478
#5  js::Debugger::CallData::clearAllBreakpoints (this=this@entry=0x7fffffffc8f0) at /home/msf2/trees/firefox/js/src/debugger/Debugger.cpp:4889
/snip

This seems to go as far back as gh rev: https://github.com/mozilla-firefox/firefox/commit/564a36e5a3ae

Run with --fuzzing-safe --no-threads --no-baseline --no-ion --more-compartments, compile with AR=ar sh ~/trees/firefox/js/src/configure --enable-debug --enable-debug-symbols --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, tested on gh rev 974b0546f470bf87a34d636a03607d788ba9305f.

Setting needinfo? from :arai as a start. :arai, any thoughts?

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Also cc'ing our GC folks since js::DebugScript::getUnbarriered is near the top of the stack.

Comment 2

[Tooru Fujisawa [:arai]]

Here's reduced testcase with comments for each step.

gczeal(23);

String + "";

var g = newGlobal({ newCompartment: true });
var dbg = Debugger(g);
dbg.onNewScript = function (script) {
  // This creates DebugScriptMap, whichi is WeakMap, and
  // puts the script.
  script.setBreakpoint(0, () => {});
};
g.eval("");

// This calls createConstructor for TypedArray,
// which performs GC during creating the prototype object.
// The GC is performed in the helper thread, and the main thread waits for it.
//
// During the GC, the script is collected (or, just not marked?),
// and the DebugScriptMap entry is also removed.
Uint8Array;

// This tries to retrieve the DebugScriptObject for the script,
// and hits the assertion failure.
dbg.clearAllBreakpoints();

DebugScript::destroyBreakpointSite assumes the script is still alive, but that's not the case here.
Simple solution would be to check the existence and abort the operation, assuming the necessary "destroy" operation is already performed.

https://searchfox.org/firefox-main/rev/b77ea8700fc2dda9e12eb4ae9b3034d9d2a716e7/js/src/debugger/DebugScript.cpp#226-229

void DebugScript::destroyBreakpointSite(JS::GCContext* gcx, JSScript* script,
                                        jsbytecode* pc) {
  // Avoid barriers during sweeping. |debug| does not escape.
  DebugScript* debug = getUnbarriered(script);

Comment 3

[Tooru Fujisawa [:arai]]

Just checking script->hasDebugScript() before the access doesn't work.
script->hasDebugScript() is still returning true.

Given the map entry is removed by the following stack, the change is not reflected to the JSScript.
This means, we need either:

• Somehow reflect the removal to JSScript flag
• Check the actual WeakMap entry existence on each access.

frame #0: mozilla::detail::HashTable<mozilla::HashMapEntry<js::PreBarriered<JSScript*>, js::PreBarriered<js::DebugScriptObject*>>, mozilla::HashMap<js::PreBarriered<JSScript*>, js::PreBarriered<js::DebugScriptObject*>, js::WeakMapKeyHasher<js::PreBarriered<JSScript*>>, js::TrackedAllocPolicy<(js::TrackingKind)1>>::MapHashPolicy, js::TrackedAllocPolicy<(js::TrackingKind)1>>::remove at obj/sm-pds/dist/include/mozilla/HashTable.h:2000:19
frame #1: mozilla::detail::HashTable<mozilla::HashMapEntry<js::PreBarriered<JSScript*>, js::PreBarriered<js::DebugScriptObject*>>, mozilla::HashMap<js::PreBarriered<JSScript*>, js::PreBarriered<js::DebugScriptObject*>, js::WeakMapKeyHasher<js::PreBarriered<JSScript*>>, js::TrackedAllocPolicy<(js::TrackingKind)1>>::MapHashPolicy, js::TrackedAllocPolicy<(js::TrackingKind)1>>::ModIterator::remove at obj/sm-pds/dist/include/mozilla/HashTable.h:1483:14
frame #2: mozilla::detail::HashTable<mozilla::HashMapEntry<js::PreBarriered<JSScript*>, js::PreBarriered<js::DebugScriptObject*>>, mozilla::HashMap<js::PreBarriered<JSScript*>, js::PreBarriered<js::DebugScriptObject*>, js::WeakMapKeyHasher<js::PreBarriered<JSScript*>>, js::TrackedAllocPolicy<(js::TrackingKind)1>>::MapHashPolicy, js::TrackedAllocPolicy<(js::TrackingKind)1>>::Enum::removeFront at obj/sm-pds/dist/include/mozilla/HashTable.h:1572:32
frame #3: js::WeakMap<JSScript*, js::DebugScriptObject*, js::TrackedAllocPolicy<(js::TrackingKind)1>>::traceWeakEdges at js/src/gc/WeakMap-inl.h:367:9
frame #4: JS::Zone::sweepWeakMaps at js/src/gc/WeakMap.cpp:158:10
frame #5: js::gc::GCRuntime::sweepWeakMaps at js/src/gc/Sweeping.cpp:1411:11

Comment 4

[Tooru Fujisawa [:arai]]

During the traceWeakEdges call, the SweepingTracer::onEdge for the JSScript makes the pointer into nullptr.
zone->isGCSweeping() is true, and cell->isMarkedAny() is false.
that would mean the JSScript is not strongly held by anything, right?

https://searchfox.org/firefox-main/rev/974b0546f470bf87a34d636a03607d788ba9305f/js/src/gc/Marking.cpp#2759-2760

if ((zone->isGCSweeping() || zone->isAtomsZone()) && !cell->isMarkedAny()) {
  *thingp = nullptr;

That would mean, all references to the JSScript is weak, including js::JSBreakpointSite::script ?
or maybe the breakpoint site is inside weak reference, or somehow not traced?

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Tooru Fujisawa [:arai] from comment #4)
It looks like this is an interaction between incremental sweeping and the debugger.

During the traceWeakEdges call, the SweepingTracer::onEdge for the JSScript makes the pointer into nullptr.
zone->isGCSweeping() is true, and cell->isMarkedAny() is false.
that would mean the JSScript is not strongly held by anything, right?

Yes, it means that the script is dead and going to be finalized. This part is clearing out references to the dead script.

That would mean, all references to the JSScript is weak, including js::JSBreakpointSite::script ?
or maybe the breakpoint site is inside weak reference, or somehow not traced?

I expect the breakpoint is dead too which is why its script hasn't been traced.

You could try checking | IsAboutToBeFinalized(script) | at the start of DebugScript::destroyBreakpointSite. If that returns true then the script is dead anyway so you can just return at that point (probably, I haven't tried it).

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: (secure)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/0a30c6b6fc37
https://hg.mozilla.org/integration/autoland/rev/fcf36db6185d
Check the corresponding script before trying to get DebugScript. r=jonco

Comment 8

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

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

Comment 9

[Donal Meehan [:dmeehan]]

Not sure :arai about uplifts and afftected versions? Asking since it's a sec-moderate, but it will need a rebased patch for ESR140

Comment 10

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: Possible UAF while using DevTools Debugger
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: none
• Risk associated with taking this patch: low
• Explanation of risk level: This adds early return to the affected case, where the equivalent code path exists for the similar condition and have already been exercised.
• String changes made/needed: none
• Is Android affected?: yes

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D278451

Comment 12

[Tooru Fujisawa [:arai]]

I don't think beta needs uplift.
This is Debugger-specific and there's no easy way to exploit it, or even reproduce it in browser.

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/69549de5c4f3