1791975 - (CVE-2022-45406) Segfault in js::gc::IsForwarded<JSObject> at 0xe5e5e5e5e5e5e5e5

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Samuel Groß]

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.

Comment 1

[Jon Coppeard (:jonco)]

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

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1791975 - Don't sweep realms that were allocated during incremental GC r?jandem

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.

Comment 3

[Andrew McCreight [:mccr8]]

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.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1791975 - Add test r?jandem

Depends on D158022

Comment 5

[Jon Coppeard (:jonco)]

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

Comment 6

[Tom Ritter [:tjr]]

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

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

Comment 7

[Tom Ritter [:tjr]]

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

Comment 8

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

Don't sweep realms that were allocated during incremental GC r=jandem
https://hg.mozilla.org/integration/autoland/rev/2815b08bfbd2f4abecf1e356461c0e2fc36bc025
https://hg.mozilla.org/mozilla-central/rev/2815b08bfbd2

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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

Comment 10

[Jon Coppeard (:jonco)]

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

Comment 11

[Jon Coppeard (:jonco)]

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.

Comment 12

[Jon Coppeard (:jonco)]

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.

Comment 13

[Jon Coppeard (:jonco)]

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

Comment 14

[Donal Meehan [:dmeehan]]

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

Approved for 107.0b6.

Comment 15

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f0d1aeb4509e

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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

Comment 17

[Jan de Mooij [:jandem]]

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.

Comment 18

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/7cac6b2502ad

Comment 19

[Donal Meehan [:dmeehan]]

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

Approved for 102.5esr.

Comment 20

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 21

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

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.

Comment 22

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

Add test r=jandem
https://hg.mozilla.org/integration/autoland/rev/49d22eea6e5f33c42c9f23546ae09e2c3edbb3a4
https://hg.mozilla.org/mozilla-central/rev/49d22eea6e5f