CVE-2024-8384
Overview
Background
- Tri-color marking
- SpiderMonkey’s garbage collector paints reachable objects
MarkColor::Black(strongly live) orMarkColor::Gray(live only via gray roots), and the color must be consistent for an object to be swept correctly. - Cross-compartment edges
- wrapper objects that point from one
Compartmentinto another, traced during zone GC bytraceIncomingCrossCompartmentEdgesForZoneGCso that targets in collected zones stay alive. - Write barrier
- a hook that can promote a gray object to black at any time, meaning a wrapper recorded as a gray edge may already have become black by the time gray roots are marked.
- `EdgeSelector`
- the
enumpassed to wrapper tracing that filters which cross-compartment edges are followed (AllEdges,NonGrayEdges,GrayEdges, and nowBlackEdges).
Root Cause Analysis
During gray-root marking, GCRuntime::markGrayRoots traced incoming cross-compartment edges using only Compartment::GrayEdges, so any wrapper whose color had been flipped from gray to black by a write barrier between the black-marking and gray-marking passes was skipped entirely. ShouldTraceWrapper classified such a barrier-promoted wrapper as non-gray and therefore excluded it from the GrayEdges set, leaving its cross-compartment target unmarked while the object graph still assumed the target was reachable. The invariant violated is that every live cross-compartment target must be marked at least once before sweeping; under an OOM-triggered timing window this invariant broke and a still-referenced object could be collected, producing a use-after-free.
The fix runs the gray-edge trace inside an explicit AutoSetMarkColor setColorGray scope and then performs a second trace over Compartment::BlackEdges in black color to cover exactly those wrappers that a barrier had promoted, ensuring their targets are marked. It also makes ShouldTraceWrapper a total switch that recognizes the new BlackEdges selector via isMarkedBlack, so no live edge falls through the classification.
BlackEdges pass in black color that mops up precisely those promoted edges.Attack Path
- Set up cross-compartment references Create a second global in a new compartment and store a shared object so a gray cross-compartment wrapper edge exists between the two zones.
- Trigger OOM at the seam
Drive allocation with
oomTestso that an out-of-memory condition lands in the narrow window between the black-marking and gray-root-marking passes of an incremental/zone GC. - Promote a gray wrapper to black
A write barrier flips the recorded gray cross-compartment wrapper to black, so
markGrayRootsno longer sees it under theGrayEdgesfilter. - Skip the live target
Because only
GrayEdgesare traced, the promoted wrapper’s target is never marked and is treated as garbage even though it remains referenced. - Corrupt memory via UAF The mis-colored target is swept and freed while live pointers remain, leaving a dangling reference an attacker can reuse to corrupt memory.
Impact Assessment
Changed Functions
| Function | Change | Notes |
|---|---|---|
MakeScopeExitjs/src/gc/Sweeping.cpp |
modified | |
ifjs/src/gc/Sweeping.cpp |
modified | |
ifjs/src/vm/Compartment.cpp |
modified | |
switchjs/src/vm/Compartment.cpp |
modified | |
oomTestjs/src/jit-test/tests/gc/bug-1911288.js |
modified |
Files Changed
js/src/gc/Sweeping.cppjs/src/gc/Verifier.cppjs/src/jit-test/tests/gc/bug-1911288.jsjs/src/vm/Compartment.cppjs/src/vm/Compartment.h
Audit Directions
- Multi-pass marking transitionsReview every place that marks in one color and later re-scans in another (gray-vs-black), checking that objects whose color changed between passes are still covered.
- Barrier-induced color changesLook for code that filters objects by current mark color while a write barrier can concurrently promote gray to black, since such filters can silently drop live edges.
- OOM and interrupt seams in GCAudit incremental/zone-GC slice boundaries and OOM handling for windows where an invariant is only transiently true, especially around cross-compartment edge tracing.
Patch
diff --git a/js/src/gc/Sweeping.cpp b/js/src/gc/Sweeping.cpp
index 4175fc68be2..2d32900fc02 100644
--- a/js/src/gc/Sweeping.cpp
+++ b/js/src/gc/Sweeping.cpp
@@ -566,23 +566,32 @@ IncrementalProgress GCRuntime::markWeakReferencesInCurrentGroup(
IncrementalProgress GCRuntime::markGrayRoots(SliceBudget& budget,
gcstats::PhaseKind phase) {
- MOZ_ASSERT(marker().markColor() == MarkColor::Gray);
+ MOZ_ASSERT(marker().markColor() == MarkColor::Black);
gcstats::AutoPhase ap(stats(), phase);
- AutoUpdateLiveCompartments updateLive(this);
- marker().setRootMarkingMode(true);
- auto guard =
- mozilla::MakeScopeExit([this]() { marker().setRootMarkingMode(false); });
+ {
+ AutoSetMarkColor setColorGray(marker(), MarkColor::Gray);
- IncrementalProgress result =
- traceEmbeddingGrayRoots(marker().tracer(), budget);
- if (result == NotFinished) {
- return NotFinished;
+ AutoUpdateLiveCompartments updateLive(this);
+ marker().setRootMarkingMode(true);
+ auto guard = mozilla::MakeScopeExit(
+ [this]() { marker().setRootMarkingMode(false); });
+
+ IncrementalProgress result =
+ traceEmbeddingGrayRoots(marker().tracer(), budget);
+ if (result == NotFinished) {
+ return NotFinished;
+ }
+
+ Compartment::traceIncomingCrossCompartmentEdgesForZoneGC(
+ marker().tracer(), Compartment::GrayEdges);
}
+ // Also mark any incoming cross compartment edges that were originally gray
+ // but have been marked black by a barrier.
Compartment::traceIncomingCrossCompartmentEdgesForZoneGC(
- marker().tracer(), Compartment::GrayEdges);
+ marker().tracer(), Compartment::BlackEdges);
return Finished;
}
@@ -1144,8 +1153,6 @@ IncrementalProgress GCRuntime::markGrayRootsInCurrentGroup(
JS::GCContext* gcx, SliceBudget& budget) {
gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::MARK);
- AutoSetMarkColor setColorGray(marker(), MarkColor::Gray);
-
// Check that the zone state is set correctly for the current sweep group as
// that determines what gets marked.
MOZ_ASSERT(atomsZone()->wasGCStarted() ==
diff --git a/js/src/gc/Verifier.cpp b/js/src/gc/Verifier.cpp
index f139d542325..a2a330c3595 100644
--- a/js/src/gc/Verifier.cpp
+++ b/js/src/gc/Verifier.cpp
@@ -607,9 +607,13 @@ void js::gc::MarkingValidator::nonIncrementalMark(AutoGCSession& session) {
zone->changeGCState(zone->initialMarkingState(), Zone::MarkBlackAndGray);
}
- AutoSetMarkColor setColorGray(*gcmarker, MarkColor::Gray);
-
+ /*
+ * markAllGrayReferences may mark both gray and black, so it manages the
+ * mark color internally.
+ */
gc->markAllGrayReferences(gcstats::PhaseKind::MARK_GRAY);
+
+ AutoSetMarkColor setColorGray(*gcmarker, MarkColor::Gray);
gc->markAllWeakReferences();
/* Restore zone state. */
diff --git a/js/src/vm/Compartment.cpp b/js/src/vm/Compartment.cpp
index 75008f0bfd5..45857b197a4 100644
--- a/js/src/vm/Compartment.cpp
+++ b/js/src/vm/Compartment.cpp
@@ -485,13 +485,18 @@ bool Compartment::wrap(JSContext* cx, MutableHandle<GCVector<Value>> vec) {
static inline bool ShouldTraceWrapper(JSObject* wrapper,
Compartment::EdgeSelector whichEdges) {
- if (whichEdges == Compartment::AllEdges) {
- return true;
+ switch (whichEdges) {
+ case Compartment::AllEdges:
+ return true;
+ case Compartment::NonGrayEdges:
+ return !wrapper->isMarkedGray();
+ case Compartment::GrayEdges:
+ return wrapper->isMarkedGray();
+ case Compartment::BlackEdges:
+ return wrapper->isMarkedBlack();
+ default:
+ MOZ_CRASH("Unexpected EdgeSelector value");
}
-
- bool isGray = wrapper->isMarkedGray();
- return (whichEdges == Compartment::NonGrayEdges && !isGray) ||
- (whichEdges == Compartment::GrayEdges && isGray);
}
void Compartment::traceWrapperTargetsInCollectedZones(JSTracer* trc,
diff --git a/js/src/vm/Compartment.h b/js/src/vm/Compartment.h
index 6cea7a7c3db..34aaf2dd00c 100644
--- a/js/src/vm/Compartment.h
+++ b/js/src/vm/Compartment.h
@@ -417,7 +417,7 @@ class JS::Compartment {
* dangling (full GCs naturally follow pointers across compartments) and
* when compacting to update cross-compartment pointers.
*/
- enum EdgeSelector { AllEdges, NonGrayEdges, GrayEdges };
+ enum EdgeSelector { AllEdges, NonGrayEdges, GrayEdges, BlackEdges };
void traceWrapperTargetsInCollectedZones(JSTracer* trc,
EdgeSelector whichEdges);
static void traceIncomingCrossCompartmentEdgesForZoneGC(
diff --git a/js/src/jit-test/tests/gc/bug-1911288.js b/js/src/jit-test/tests/gc/bug-1911288.js
new file mode 100644
index 00000000000..335d6adb575
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug-1911288.js
@@ -0,0 +1,18 @@
+Object.defineProperty(this, "x", {});
+try {
+ oomTest(function() {
+ let m = parseModule(``);
+ });
+ var dbg = new Debugger;
+ dbg.onNewGlobalObject = function (global) {}
+ let t39 = {};
+ addMarkObservers([t39]);
+ let g33 = newGlobal({newCompartment: true});
+ g33.t39 = t39;
+ g33.eval("grayRoot().push(t);");
+} catch (exc) {}
+gcparam("allocationThreshold", 1 /* MB */);
+gcparam("incrementalGCEnabled", false);
+oomTest(function() {
+ var lfGlobal = newGlobal({sameZoneAs: this});
+});
Regression Test / PoC
diff --git a/js/src/jit-test/tests/gc/bug-1911288.js b/js/src/jit-test/tests/gc/bug-1911288.js
new file mode 100644
index 00000000000..335d6adb575
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug-1911288.js
@@ -0,0 +1,18 @@
+Object.defineProperty(this, "x", {});
+try {
+ oomTest(function() {
+ let m = parseModule(``);
+ });
+ var dbg = new Debugger;
+ dbg.onNewGlobalObject = function (global) {}
+ let t39 = {};
+ addMarkObservers([t39]);
+ let g33 = newGlobal({newCompartment: true});
+ g33.t39 = t39;
+ g33.eval("grayRoot().push(t);");
+} catch (exc) {}
+gcparam("allocationThreshold", 1 /* MB */);
+gcparam("incrementalGCEnabled", false);
+oomTest(function() {
+ var lfGlobal = newGlobal({sameZoneAs: this});
+});