Closed Bug 1791975 (CVE-2022-45406) Opened 2 years ago Closed 2 years ago

Segfault in js::gc::IsForwarded<JSObject> at 0xe5e5e5e5e5e5e5e5

Categories

(Core :: JavaScript Engine, task, P1)

task

Tracking

()

RESOLVED FIXED
108 Branch
Tracking Status
firefox-esr102 107+ fixed
firefox105 --- wontfix
firefox106 --- wontfix
firefox107 + fixed
firefox108 + fixed

People

(Reporter: saelo, Assigned: jonco)

References

Details

(Keywords: csectype-uaf, sec-high, Whiteboard: [post-critsmash-triage][adv-main107+][adv-esr102.5+])

Attachments

(3 files)

The following sample triggers a segmentation fault in debug builds from latest HEAD (it is not 100% reliable, but it works most of the time for me):

function main() {
let v2 = 0;
do {
    const v5 = this.gczeal(10,v2);
    const v6 = v2++;
} while (v2 < 10);
for (let [v7] of "iV43r24wsw") {
    const v8 = v7 | 0;
    for (const v10 in "T4PnjCgpU2") {
        for (let v14 = 0; v14 < 10; v14++) {
            try {
                const v16 = this.oomAtAllocation(v14);
                const v18 = Float64Array();
            } catch(v19) {
                const v20 = v7 in v19;
                for (let v23 = 1; v23 < 64; v23++) {
                    const v25 = this.oomAtAllocation(v23);
                    try {
                        function v26(v27,v28) {
                        }
                        const v31 = this.newGlobal();
                        const v32 = Reflect.parse();
                        function v33(v34,v35) {
                        }
                    } catch(v36) {
                    }
                }
            }
        }
    }
    const v38 = v8 >> 25;
    const v39 = Math;
}
gc();
}
main();
// CRASH INFO
// ==========
// TERMSIG: 11
// STDERR:

Here's the stacktrace from gdb:

#0  0x0000555557317476 in std::__atomic_base<unsigned long>::load (this=0xe5e5e5e5e5e5e5e5, __m=std::memory_order_relaxed) at /usr/local/google/home/saelo/.mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/atomic_base.h:396
#1  mozilla::detail::IntrinsicMemoryOps<unsigned long, (mozilla::MemoryOrdering)0>::load (aPtr=<error reading variable: Cannot access memory at address 0xe5e5e5e5e5e5e5e5>) at obj-debug/dist/include/mozilla/Atomics.h:195
#2  0x0000555557317405 in mozilla::detail::AtomicBaseIncDec<unsigned long, (mozilla::MemoryOrdering)0>::operator unsigned long (this=0xe5e5e5e5e5e5e5e5) at obj-debug/dist/include/mozilla/Atomics.h:340
#3  0x00005555575509e5 in js::gc::Cell::isForwarded (this=0xe5e5e5e5e5e5e5e5) at js/src/gc/Cell.h:145
#4  0x0000555557615405 in js::gc::IsForwarded<JSObject> (t=0xe5e5e5e5e5e5e5e5) at js/src/gc/Marking-inl.h:89
#5  0x00005555581838dc in js::CheckTracedThing<JSObject> (trc=0x7fffffff8038, thing=0xe5e5e5e5e5e5e5e5) at js/src/gc/Marking.cpp:153
#6  0x00005555573345e4 in js::gc::TraceEdgeInternal (trc=0x7fffffff8038, thingp=0x7fffffff7f40, Object=0x555555aa9e91 "baseshape_global") at js/src/gc/Tracer.h:105
#7  0x0000555557bba495 in js::TraceManuallyBarrieredEdge<JSObject*> (trc=0x7fffffff8038, thingp=0x7fffffff7f40, name=0x555555aa9e91 "baseshape_global") at js/src/gc/Tracer.h:247
#8  0x000055555812a763 in js::BaseShape::traceChildren (this=0x20afd0e9a058, trc=0x7fffffff8038) at js/src/gc/TraceMethods-inl.h:296
#9  0x0000555558109948 in UpdateCellPointers<js::BaseShape> (trc=0x7fffffff8038, cell=0x20afd0e9a058) at js/src/gc/Compacting.cpp:488
#10 0x0000555558108dcb in UpdateArenaPointersTyped<js::BaseShape> (trc=0x7fffffff8038, arena=0x20afd0e9a000) at js/src/gc/Compacting.cpp:494
#11 0x00005555581089f3 in UpdateArenaPointers (trc=0x7fffffff8038, arena=0x20afd0e9a000) at js/src/gc/Compacting.cpp:524
#12 0x00005555580ecba0 in UpdateArenaListSegmentPointers (gc=0x7ffff7718768, arenas=...) at js/src/gc/Compacting.cpp:548
#13 0x00005555580ec8e1 in js::gc::GCRuntime::updateCellPointers (this=0x7ffff7718768, zone=0x7ffff577b000, kinds=...) at js/src/gc/Compacting.cpp:687
#14 0x00005555580ecc46 in js::gc::GCRuntime::updateAllCellPointers (this=0x7ffff7718768, trc=0x7fffffff8880, zone=0x7ffff577b000) at js/src/gc/Compacting.cpp:755
#15 0x00005555580e9fec in js::gc::GCRuntime::updateZonePointersToRelocatedCells (this=0x7ffff7718768, zone=0x7ffff577b000) at js/src/gc/Compacting.cpp:796
#16 0x00005555580e97af in js::gc::GCRuntime::compactPhase (this=0x7ffff7718768, reason=JS::GCReason::DEBUG_GC, sliceBudget=..., session=...) at js/src/gc/Compacting.cpp:90
#17 0x00005555581017f8 in js::gc::GCRuntime::incrementalSlice (this=0x7ffff7718768, budget=..., reason=JS::GCReason::DEBUG_GC, budgetWasIncreased=false) at js/src/gc/GC.cpp:3381
#18 0x0000555558103845 in js::gc::GCRuntime::gcCycle (this=0x7ffff7718768, nonincrementalByAPI=false, budgetArg=..., reason=JS::GCReason::DEBUG_GC) at js/src/gc/GC.cpp:3842
#19 0x00005555581047d2 in js::gc::GCRuntime::collect (this=0x7ffff7718768, nonincrementalByAPI=false, budget=..., reason=JS::GCReason::DEBUG_GC) at js/src/gc/GC.cpp:4030
#20 0x0000555558106667 in js::gc::GCRuntime::runDebugGC (this=0x7ffff7718768) at js/src/gc/GC.cpp:4476
#21 0x00005555581351f3 in js::gc::GCRuntime::gcIfNeededAtAllocation (this=0x7ffff7718768, cx=0x7ffff772a100) at js/src/gc/Allocator.cpp:445
#22 0x000055555810f562 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=0x7ffff7718768, cx=0x7ffff772a100, kind=js::gc::AllocKind::OBJECT8_BACKGROUND) at js/src/gc/Allocator.cpp:409
#23 0x000055555810f465 in js::gc::detail::AllocateObject<(js::AllowGC)1> (cx=0x7ffff772a100, kind=js::gc::AllocKind::OBJECT8_BACKGROUND, nDynamicSlots=0, heap=js::gc::TenuredHeap, clasp=0x55555949e840 <global_class>, site=0x0) at js/src/gc/Allocator.cpp:67
#24 0x00005555574930d0 in js::gc::CellAllocator::NewCell<js::NativeObject, (js::AllowGC)1, js::gc::AllocKind&, unsigned long const&, js::gc::InitialHeap&, JSClass const*&, js::gc::AllocSite*&> (cx=0x7ffff772a100, args=@0x7fffffff8fc0: 0x0, args=@0x7fffffff8fc0: 0x0, args=@0x7fffffff8fc0: 0x0, args=@0x7fffffff8fc0: 0x0, args=@0x7fffffff8fc0: 0x0) at js/src/gc/Allocator.h:123
#25 0x0000555557492211 in JSContext::newCell<js::NativeObject, (js::AllowGC)1, js::gc::AllocKind&, unsigned long const&, js::gc::InitialHeap&, JSClass const*&, js::gc::AllocSite*&> (this=0x7ffff772a100, args=@0x7fffffff8fc0: 0x0, args=@0x7fffffff8fc0: 0x0, args=@0x7fffffff8fc0: 0x0, args=@0x7fffffff8fc0: 0x0, args=@0x7fffffff8fc0: 0x0) at js/src/vm/JSContext.h:267
#26 0x0000555557491be7 in js::NativeObject::create (cx=0x7ffff772a100, kind=js::gc::AllocKind::OBJECT8_BACKGROUND, heap=js::gc::TenuredHeap, shape=..., site=0x0) at js/src/vm/NativeObject-inl.h:444
#27 0x00005555577cff3b in NewObject (cx=0x7ffff772a100, clasp=0x55555949e840 <global_class>, proto=..., kind=js::gc::AllocKind::OBJECT8_BACKGROUND, newKind=js::TenuredObject) at js/src/vm/JSObject.cpp:767
#28 0x00005555577cfb3b in js::NewObjectWithGivenTaggedProto (cx=0x7ffff772a100, clasp=0x55555949e840 <global_class>, proto=..., allocKind=js::gc::AllocKind::OBJECT8, newKind=js::TenuredObject) at js/src/vm/JSObject.cpp:781
#29 0x00005555574aef74 in js::NewObjectWithGivenTaggedProto<(js::NewObjectKind)1> (cx=0x7ffff772a100, clasp=0x55555949e840 <global_class>, proto=...) at js/src/vm/JSObject-inl.h:365
#30 0x00005555574aee37 in js::NewTenuredObjectWithGivenProto (cx=0x7ffff772a100, clasp=0x55555949e840 <global_class>, proto=...) at js/src/vm/JSObject-inl.h:403
#31 0x000055555771e59c in js::GlobalObject::createInternal (cx=0x7ffff772a100, clasp=0x55555949e840 <global_class>) at js/src/vm/GlobalObject.cpp:546
#32 0x000055555771ed82 in js::GlobalObject::new_ (cx=0x7ffff772a100, clasp=0x55555949e840 <global_class>, principals=0x0, hookOption=JS::DontFireOnNewGlobalHook, options=...) at js/src/vm/GlobalObject.cpp:619
#33 0x0000555557c94921 in JS_NewGlobalObject (cx=0x7ffff772a100, clasp=0x55555949e840 <global_class>, principals=0x0, hookOption=JS::DontFireOnNewGlobalHook, options=...) at js/src/jsapi.cpp:1744
#34 0x00005555572e1d4f in NewGlobalObject (cx=0x7ffff772a100, options=..., principals=0x0, kind=ShellGlobalKind::WindowProxy, immutablePrototype=true) at js/src/shell/js.cpp:10166
#35 0x00005555572f48e0 in NewGlobal (cx=0x7ffff772a100, argc=0, vp=0x7fffffff9a00) at js/src/shell/js.cpp:6596

0xe5e5e5e5e5e5e5e5 seems to be the "jemalloc freed memory" marker, so this may indicate a use-after-free issue. I'm therefore filing this as a security bug.

Group: javascript-core-security
Group: core-security

(In reply to Samuel Groß from comment #0)
Thanks for the bug report.

Assignee: nobody → jcoppeard
Priority: -- → P1

When marking a BaseShape we mark its global, and we read the pointer to that
global from the realm. If a realm doesn't have a live global we can sweep the
realm but there may still be pointers to it in base shapes and these are left
dangling.

This happens when we hit OOM while creating a global during an incremental GC.
The BaseShape survives because it was allocated after the start of the GC. The
global itself is never successfully created and so the realm doesn't have a
live global and is swept. In this case, we trigger UAF when we try to compact
the heap and trace the base shape.

The patch adds an extra case for keeping a realm alive if it was created during
an incremental GC. This matches the way that GC things are not collected if
they are allocated after the start of a GC.

I'll mark this sec-high. It sounds a bit tricky to trigger, but "have an OOM while creating a global" is something we already see, at least on the DOM side of creating a new Window, so it probably isn't impossible.

See Also: → 1787084

Depends on D158022

Comment on attachment 9295964 [details]
Bug 1791975 - Don't sweep realms that were allocated during incremental GC r?jandem

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Difficult. The patch doesn't mention the OOM condition which is required to exploit this.
  • 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?: All supported branches
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?: Should be simple to backport.
  • How likely is this patch to cause regressions; how much testing does it need?: This is a simple change that is unlikely to cause regressions. That said it would be good to let it bake on central for a week or so before backporting.
  • Is Android affected?: Yes
Attachment #9295964 - Flags: sec-approval?
Attachment #9297956 - Flags: sec-approval?

Comment on attachment 9297956 [details]
Bug 1791975 - Add test r?jandem

Clearing sec-approval; will add a reminder for landing the test

Attachment #9297956 - Flags: sec-approval?

Comment on attachment 9295964 [details]
Bug 1791975 - Don't sweep realms that were allocated during incremental GC r?jandem

Approved to land and uplift after beta branches

Attachment #9295964 - Flags: sec-approval? → sec-approval+
Whiteboard: [reminder-test 2023-01-01]
Group: javascript-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 108 Branch

This grafts cleanly. Please nominate for Beta and ESR approval when you get a chance.

Flags: needinfo?(jcoppeard)

Comment on attachment 9295964 [details]
Bug 1791975 - Don't sweep realms that were allocated during incremental GC r?jandem

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 small change and it makes the lifetime of realms closer to that of regular GC things. It has baked on central for 6 days so far.
  • String changes made/needed:
  • Is Android affected?: Yes
Flags: needinfo?(jcoppeard)
Attachment #9295964 - Flags: approval-mozilla-beta?

Comment on attachment 9295964 [details]
Bug 1791975 - Don't sweep realms that were allocated during incremental GC r?jandem

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a sec-high bug.
  • User impact if declined: Possible crash / security vulnerability.
  • Fix Landed on Version: 108
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a small change and it makes the lifetime of realms closer to that of regular GC things. It has baked on central for 6 days so far.
Attachment #9295964 - Flags: approval-mozilla-esr102?
Regressions: CVE-2022-45409

Comment on attachment 9295964 [details]
Bug 1791975 - Don't sweep realms that were allocated during incremental GC r?jandem

Cancelling these until bug 1796901 is resolved.

Attachment #9295964 - Flags: approval-mozilla-esr102?
Attachment #9295964 - Flags: approval-mozilla-beta?

Comment on attachment 9295964 [details]
Bug 1791975 - Don't sweep realms that were allocated during incremental GC r?jandem

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: Bug 1796901
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a small change and it makes the lifetime of realms closer to that of regular GC things.
  • String changes made/needed: None
  • Is Android affected?: Yes
Attachment #9295964 - Flags: approval-mozilla-beta?

Comment on attachment 9295964 [details]
Bug 1791975 - Don't sweep realms that were allocated during incremental GC r?jandem

Approved for 107.0b6.

Attachment #9295964 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify-
Whiteboard: [reminder-test 2023-01-01] → [reminder-test 2023-01-01][post-critsmash-triage]

Jan, can you please nominate this and bug 1796901 for ESR102 uplift since Jon is on PTO until next week? Thanks!

Flags: needinfo?(jdemooij)

Comment on attachment 9295964 [details]
Bug 1791975 - Don't sweep realms that were allocated during incremental GC r?jandem

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined: Crashes or security issues.
  • Fix Landed on Version: 108 (and 107 uplift)
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a small change and it makes the lifetime of realms closer to that of regular GC things.
Flags: needinfo?(jdemooij)
Attachment #9295964 - Flags: approval-mozilla-esr102?

Comment on attachment 9295964 [details]
Bug 1791975 - Don't sweep realms that were allocated during incremental GC r?jandem

Approved for 102.5esr.

Attachment #9295964 - Flags: approval-mozilla-esr102? → approval-mozilla-esr102+
Whiteboard: [reminder-test 2023-01-01][post-critsmash-triage] → [reminder-test 2023-01-01][post-critsmash-triage][adv-main107+]
Whiteboard: [reminder-test 2023-01-01][post-critsmash-triage][adv-main107+] → [reminder-test 2023-01-01][post-critsmash-triage][adv-main107+][adv-esr102.5+]
Alias: CVE-2022-45406

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

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

Flags: needinfo?(jcoppeard)
Whiteboard: [reminder-test 2023-01-01][post-critsmash-triage][adv-main107+][adv-esr102.5+] → [post-critsmash-triage][adv-main107+][adv-esr102.5+]
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: