1820543 - (CVE-2023-29535) Assertion failure: this->flags() == 0, at gc/Cell.h:836

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P1)

Description

[lukas.bernhard]

Steps to reproduce:

On git commit 88fa8a86e65ead2abe32044bab7ca6c10895c738 the attached sample triggers an assertion violation in the GC. There seems to be an OOM condition involved; precautiously tagging s-s.
Bisecting the issue points to commit 8d993476b0fca3d388d139a207d8fe838aeb2187 related to bug 1749298.

const v1 = ("DEB1").startsWith("DEB1");
function f2(a3, a4, a5, a6) {
    return ({"constructor":this,"b":a3,"__proto__":this}).newGlobal(f2);
}
f2.newCompartment = v1; 
with (f2()) {
    function f11(a12, a13) {
        return "DEB1";
    }   
    const v15 = new FinalizationRegistry(f11);
    v15.register(f2);
}
this.reportLargeAllocationFailure();
gc()

#0  0x0000555556de20ca in JSObject::zoneFromAnyThread (this=<optimized out>)
    at js/src/vm/JSObject.h:287
#1  0x00005555579cca4e in js::MovableCellHasher<JSObject*>::match (k=@0x7ffff4cdb8a0: 0x1620d9683040,
    l=@0x7fffffffcf58: 0x1620d96490b0) at js/src/gc/Barrier.cpp:120
#2  0x00005555575d5407 in js::MovableCellHasher<js::HeapPtr<JSObject*> >::match (k=...,
    l=@0x7fffffffcf58: 0x1620d96490b0) at js/src/gc/Barrier.h:1165
#3  mozilla::detail::HashTable<mozilla::HashMapEntry<js::HeapPtr<JSObject*>, js::HeapPtr<JS::Value> >, mozilla::HashMap<js::HeapPtr<JSObject*>, js::HeapPtr<JS::Value>, js::MovableCellHasher<js::HeapPtr
<JSObject*> >, js::TrackedAllocPolicy<(js::TrackingKind)1> >::MapHashPolicy, js::TrackedAllocPolicy<(js::TrackingKind)1> >::match (
    aEntry=..., aLookup=@0x7fffffffcf58: 0x1620d96490b0)
    at obj-x86_64-pc-linux-gnu/dist/include/mozilla/HashTable.h:1735
#4  mozilla::detail::HashTable<mozilla::HashMapEntry<js::HeapPtr<JSObject*>, js::HeapPtr<JS::Value> >, mozilla::HashMap<js::HeapPtr<JSObject*>, js::HeapPtr<JS::Value>, js::MovableCellHasher<js::HeapPtr
<JSObject*> >, js::TrackedAllocPolicy<(js::TrackingKind)1> >::MapHashPolicy, js::TrackedAllocPolicy<(js::TrackingKind)1> >::lookup<(mozilla::detail::HashTable<mozilla::HashMapEntry<js::HeapPtr<JSObject
*>, js::HeapPtr<JS::Value> >, mozilla::HashMap<js::HeapPtr<JSObject*>, js::HeapPtr<JS::Value>, js::MovableCellHasher<js::HeapPtr<JSObject*> >, js::TrackedAllocPolicy<(js::TrackingKind)1> >::MapHashPoli
cy, js::TrackedAllocPolicy<(js::TrackingKind)1> >::LookupReason)1> (
    this=this@entry=0x7ffff4c54240, aLookup=@0x7fffffffcf58: 0x1620d96490b0, aKeyHash=<optimized out>,
    aKeyHash@entry=389894670)
    at obj-x86_64-pc-linux-gnu/dist/include/mozilla/HashTable.h:1765
#5  0x00005555575d5239 in mozilla::detail::HashTable<mozilla::HashMapEntry<js::HeapPtr<JSObject*>, js::HeapPtr<JS::Value> >, mozilla::HashMap<js::HeapPtr<JSObject*>, js::HeapPtr<JS::Value>, js::Movable
CellHasher<js::HeapPtr<JSObject*> >, js::TrackedAllocPolicy<(js::TrackingKind)1> >::MapHashPolicy, js::TrackedAllocPolicy<(js::TrackingKind)1> >::lookupForAdd (this=0x7ffff4c54240, aLookup=@0x7fffffffc
f58: 0x1620d96490b0)
    at obj-x86_64-pc-linux-gnu/dist/include/mozilla/HashTable.h:2104
#6  0x00005555579a4160 in mozilla::HashMap<js::HeapPtr<JSObject*>, js::HeapPtr<JS::Value>, js::MovableCellHasher<js::HeapPtr<JSObject*> >, js::TrackedAllocPolicy<(js::TrackingKind)1> >::lookupForAdd (t
his=0x7ffff4c54240,
    aLookup=@0x7fffffffcf58: 0x1620d96490b0)
    at obj-x86_64-pc-linux-gnu/dist/include/mozilla/HashTable.h:336
#7  js::WeakMap<js::HeapPtr<JSObject*>, js::HeapPtr<JS::Value> >::lookupForAdd (this=0x7ffff4c54208,
    l=@0x7fffffffcf58: 0x1620d96490b0) at js/src/gc/WeakMap.h:222
#8  js::gc::FinalizationObservers::removeCrossZoneWrapper (this=this@entry=0x7ffff4c541a0, weakSet=...,
    wrapper=wrapper@entry=0x1620d96490b0)
    at js/src/gc/FinalizationObservers.cpp:141
#9  0x00005555579a65a2 in js::gc::FinalizationObservers::updateForRemovedRecord (this=this@entry=0x7ffff4c541a0,
    wrapper=wrapper@entry=0x1620d96490b0, record=record@entry=0x1620d9686040)
    at js/src/gc/FinalizationObservers.cpp:268
#10 0x00005555579a5fc9 in js::gc::FinalizationObservers::traceWeakFinalizationRegistryEdges(JSTracer*)::$_2::operator()(js::HeapPtr<JSObject*>&) const (heapPtr=..., this=<optimized out>)
    at js/src/gc/FinalizationObservers.cpp:224
#11 JS::GCVector<js::HeapPtr<JSObject*>, 1ul, js::TrackedAllocPolicy<(js::TrackingKind)1> >::mutableEraseIf<js::gc::FinalizationObservers::traceWeakFinalizationRegistryEdges(JSTracer*)::$_2>(js::gc::Fi
nalizationObservers::traceWeakFinalizationRegistryEdges(JSTracer*)::$_2) (this=0x7ffff4cf9908, pred=...)
    at obj-x86_64-pc-linux-gnu/dist/include/js/GCVector.h:183
#12 js::gc::FinalizationObservers::traceWeakFinalizationRegistryEdges (this=this@entry=0x7ffff4c541a0, 
    trc=<optimized out>, trc@entry=0x7fffffffd100)
    at js/src/gc/FinalizationObservers.cpp:215
#13 0x00005555579a167e in js::gc::FinalizationObservers::traceWeakEdges (this=0x7ffff4c541a0, trc=0x7fffffffd100)
    at js/src/gc/FinalizationObservers.cpp:189
#14 js::gc::GCRuntime::traceWeakFinalizationObserverEdges (trc=0x7fffffffd100, zone=0x7ffff4c2b000, 
    this=<optimized out>) at js/src/gc/FinalizationObservers.cpp:176
#15 js::gc::GCRuntime::sweepZoneAfterCompacting (this=this@entry=0x7ffff7423728, trc=trc@entry=0x7fffffffd100, 
    zone=zone@entry=0x7ffff4c2b000) at js/src/gc/Compacting.cpp:458
#16 0x000055555799dfe9 in js::gc::GCRuntime::updateZonePointersToRelocatedCells (this=this@entry=0x7ffff7423728, zone=zone@entry=0x7ffff4c2b000)
    at js/src/gc/Compacting.cpp:778
#17 0x000055555799d3ef in js::gc::GCRuntime::compactPhase (this=this@entry=0x7ffff7423728, reason=reason@entry=JS::GCReason::SHARED_MEMORY_LIMIT, 
    sliceBudget=..., session=...) at js/src/gc/Compacting.cpp:90
#18 0x00005555579bfef4 in js::gc::GCRuntime::incrementalSlice (this=this@entry=0x7ffff7423728, budget=..., 
    reason=reason@entry=JS::GCReason::SHARED_MEMORY_LIMIT, budgetWasIncreased=<optimized out>) at js/src/gc/GC.cpp:3748
#19 0x00005555579c2b68 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff7423728, nonincrementalByAPI=true, budgetArg=...,
    reason=reason@entry=JS::GCReason::SHARED_MEMORY_LIMIT) at js/src/gc/GC.cpp:4212
#20 0x00005555579c3dd3 in js::gc::GCRuntime::collect (this=this@entry=0x7ffff7423728, nonincrementalByAPI=<optimized out>, budget=...,
    reason=reason@entry=JS::GCReason::SHARED_MEMORY_LIMIT) at js/src/gc/GC.cpp:4400
#21 0x0000555557998618 in js::gc::GCRuntime::gc (this=0x7ffff7423728, options=JS::GCOptions::Shrink, reason=JS::GCReason::SHARED_MEMORY_LIMIT)
    at js/src/gc/GC.cpp:4477
#22 0x000055555734c466 in JSRuntime::onOutOfMemoryCanGC (this=0x7ffff7423000, allocFunc=js::AllocFunction::Malloc, arena=5098337497085642432,
    bytes=26214400, reallocPtr=0x0) at js/src/vm/Runtime.cpp:698
#23 0x00005555575a2d14 in ReportLargeAllocationFailure (cx=cx@entry=0x7ffff7437300, argc=0, vp=<optimized out>)
    at js/src/builtin/TestingFunctions.cpp:5693
#24 0x0000555556ec1594 in CallJSNative (cx=cx@entry=0x7ffff7437300,
    native=native@entry=0x5555575a2c10 <ReportLargeAllocationFailure(JSContext*, unsigned int, JS::Value*)>, reason=reason@entry=js::CallReason::Call,
    args=...) at js/src/vm/Interpreter.cpp:459
#25 0x0000555556ec095e in js::InternalCallOrConstruct (cx=0x7ffff7437300, cx@entry=0x5555588fd290 <Interpret(JSContext*, js::RunState&)::addresses>,
    args=..., construct=construct@entry=js::NO_CONSTRUCT, reason=js::CallReason::Call, reason@entry=4294967286)
    at js/src/vm/Interpreter.cpp:553
#26 0x0000555556ec26c6 in InternalCall (cx=0x7ffff79f8a00 <_IO_stdfile_2_lock>, args=..., reason=1497881232)
    at js/src/vm/Interpreter.cpp:620
#27 0x0000555556eb405a in js::CallFromStack (cx=0x7ffff79f8a00 <_IO_stdfile_2_lock>, cx@entry=0xffff800000000000, args=..., reason=<optimized out>)
    at js/src/vm/Interpreter.cpp:625
#28 Interpret (cx=0x7ffff79f8a00 <_IO_stdfile_2_lock>, cx@entry=0x7ffff7437300, state=...)
    at js/src/vm/Interpreter.cpp:3368
#29 0x0000555556ea6fd5 in js::RunScript (cx=cx@entry=0x7ffff7437300, state=...) at js/src/vm/Interpreter.cpp:431
#30 0x0000555556ec4722 in js::ExecuteKernel (cx=cx@entry=0x7ffff7437300, script=script@entry=..., envChainArg=envChainArg@entry=...,
    evalInFrame=evalInFrame@entry=..., result=...) at js/src/vm/Interpreter.cpp:818
#31 0x0000555556ec4dd1 in js::Execute (cx=cx@entry=0x7ffff7437300, script=script@entry=..., envChain=..., rval=rval@entry=...)
    at js/src/vm/Interpreter.cpp:850
#32 0x0000555557071b86 in ExecuteScript (cx=cx@entry=0x7ffff7437300, envChain=..., script=..., rval=rval@entry=...)
    at js/src/vm/CompilationAndEvaluation.cpp:472
#33 0x0000555557071e60 in JS_ExecuteScript (cx=cx@entry=0x7ffff7437300, scriptArg=scriptArg@entry=...)
    at js/src/vm/CompilationAndEvaluation.cpp:496

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1820543 - Update weak maps before finalization observer edges when compacting r?sfink

The latter relies on being able to access the former.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1820543 - Add test r?sfink

Depends on D171723

Comment 3

[Andrew McCreight [:mccr8]]

I'll assume this is sec-high-ish.

Comment 4

[Jon Coppeard (:jonco)]

Comment on attachment 9321380 [details]
Bug 1820543 - Update weak maps before finalization observer edges when compacting r?sfink

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Very difficult.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: Everything back to FF 95
• If not all supported branches, which bug introduced the flaw?: Bug 1749298
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Should be trivial.
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely. This is a small change that shouldn't affect anything else. The usual fuzzing on central is a good idea though.
• Is Android affected?: Yes

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1749298

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1749298

Comment 7

[Tom Ritter [:tjr]]

Comment on attachment 9321380 [details]
Bug 1820543 - Update weak maps before finalization observer edges when compacting r?sfink

Approved to land and uplift

Comment 8

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

Update weak maps before finalization observer edges when compacting r=sfink
https://hg.mozilla.org/integration/autoland/rev/82288e840ac0b115e307fbcf1c2460d8d84b5639
https://hg.mozilla.org/mozilla-central/rev/82288e840ac0

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jonco, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox112 to wontfix.

For more information, please visit auto_nag documentation.

Comment 10

[Jon Coppeard (:jonco)]

Comment on attachment 9321380 [details]
Bug 1820543 - Update weak maps before finalization observer edges when compacting r?sfink

Beta/Release Uplift Approval Request

• User impact if declined: Possible crash / security vulnerability.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a simple change that is already in central.
• String changes made/needed:
• Is Android affected?: Yes

Comment 11

[Dianna Smith [:diannaS]]

Comment on attachment 9321380 [details]
Bug 1820543 - Update weak maps before finalization observer edges when compacting r?sfink

Approved for 112.0b4

Comment 12

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/34d5cf491ccb

Comment 13

[Dianna Smith [:diannaS]]

Comment on attachment 9321380 [details]
Bug 1820543 - Update weak maps before finalization observer edges when compacting r?sfink

Approved for 102.10esr

Comment 14

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/d58e0a95e12d

Comment 15

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

2 months ago, Tom Ritter [:tjr] placed a reminder on the bug using the whiteboard tag [reminder-test 2023-05-23] .

jonco, please refer to the original comment to better understand the reason for the reminder.

Comment 17

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

Add test r=sfink
https://hg.mozilla.org/integration/autoland/rev/da17e0004c114289b1cc2785d46f55a6b287603f
https://hg.mozilla.org/mozilla-central/rev/da17e0004c11