Closed Bug 1820543 (CVE-2023-29535) Opened 1 year ago Closed 1 year ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
113 Branch
Tracking Status
firefox-esr102 112+ fixed
firefox110 --- wontfix
firefox111 --- wontfix
firefox112 + fixed
firefox113 + fixed

People

(Reporter: lukas.bernhard, Assigned: jonco)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: csectype-uaf, regression, sec-high, Whiteboard: [adv-main112+][adv-esr102.10+] )

Attachments

(3 files)

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
Group: firefox-core-security → core-security
Component: Untriaged → JavaScript: GC
Product: Firefox → Core
Assignee: nobody → jcoppeard

The latter relies on being able to access the former.

Depends on D171723

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

Group: core-security → javascript-core-security
Keywords: regression
Regressed by: 1749298

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
Attachment #9321380 - Flags: sec-approval?

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

Severity: -- → S2
Priority: -- → P1
Flags: sec-bounty?

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

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

Approved to land and uplift

Attachment #9321380 - Flags: sec-approval? → sec-approval+
Whiteboard: [reminder-test 2023-05-23]
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 113 Branch

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.

Flags: needinfo?(jcoppeard)

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
Flags: needinfo?(jcoppeard)
Attachment #9321380 - Flags: approval-mozilla-beta?
Attachment #9321380 - Flags: approval-mozilla-esr102?

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

Approved for 112.0b4

Attachment #9321380 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify-

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

Approved for 102.10esr

Attachment #9321380 - Flags: approval-mozilla-esr102? → approval-mozilla-esr102+
Flags: sec-bounty? → sec-bounty+
Whiteboard: [reminder-test 2023-05-23] → [reminder-test 2023-05-23][adv-main112+]
Whiteboard: [reminder-test 2023-05-23][adv-main112+] → [reminder-test 2023-05-23][adv-main112+][adv-esr102.10+]
Attached file advisory.txt
Alias: CVE-2023-29535

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.

Flags: needinfo?(jcoppeard)
Whiteboard: [reminder-test 2023-05-23][adv-main112+][adv-esr102.10+] → [adv-main112+][adv-esr102.10+]
Flags: needinfo?(jcoppeard)
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: