Medium CVSS 9.8 firefox Memory Corruption 🔧 Commit mapped

Overview

Medium
Severity
9.8
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionThe JavaScript garbage collector could mis-color cross-compartment objects if OOM conditions were detected at the right point between two passes. This could have led to memory corruption.
ComponentSpiderMonkey
Bug ClassMemory Corruption
Tracker1911288
Fix commita107c5b58903 (firefox) +55/-21
CWECWE-787, CWE-416 (Out-of-bounds write, Use-after-free)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H
CISA KEVNot listed
Creditedthe Mozilla Fuzzing Team
Disclosed2024-09-03

Background

Tri-color marking
SpiderMonkey’s garbage collector paints reachable objects MarkColor::Black (strongly live) or MarkColor::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 Compartment into another, traced during zone GC by traceIncomingCrossCompartmentEdgesForZoneGC so 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 enum passed to wrapper tracing that filters which cross-compartment edges are followed (AllEdges, NonGrayEdges, GrayEdges, and now BlackEdges).

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.

Key insight
The core mistake was treating a wrapper’s mark color as immutable during marking and only ever tracing gray-colored cross-compartment edges, so barrier-promoted (now-black) wrappers were never traced; the fix adds a BlackEdges pass in black color that mops up precisely those promoted edges.

Attack Path

  1. 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.
  2. Trigger OOM at the seam Drive allocation with oomTest so 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.
  3. Promote a gray wrapper to black A write barrier flips the recorded gray cross-compartment wrapper to black, so markGrayRoots no longer sees it under the GrayEdges filter.
  4. Skip the live target Because only GrayEdges are traced, the promoted wrapper’s target is never marked and is treated as garbage even though it remains referenced.
  5. 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

An attacker running JavaScript in the content process gains a garbage-collector-induced use-after-free, freeing an object that is still referenced and yielding a dangling pointer usable for memory corruption. Reaching it requires precise timing: an OOM must occur in the small window between the two marking passes while a gray-to-black barrier promotion is in flight, which is why the severity is rated moderate. Success gives an in-process memory-corruption primitive that could be chained toward exploitation but is not directly a full sandbox escape.

Changed Functions

FunctionChangeNotes
MakeScopeExit
js/src/gc/Sweeping.cpp
modified
if
js/src/gc/Sweeping.cpp
modified
if
js/src/vm/Compartment.cpp
modified
switch
js/src/vm/Compartment.cpp
modified
oomTest
js/src/jit-test/tests/gc/bug-1911288.js
modified

Files Changed

  • js/src/gc/Sweeping.cpp
  • js/src/gc/Verifier.cpp
  • js/src/jit-test/tests/gc/bug-1911288.js
  • js/src/vm/Compartment.cpp
  • js/src/vm/Compartment.h

Audit Directions

  • Multi-pass marking transitions
    Review 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 changes
    Look 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 GC
    Audit incremental/zone-GC slice boundaries and OOM handling for windows where an invariant is only transiently true, especially around cross-compartment edge tracing.
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});
+});
Loading diff…

Regression Test / PoC

shipped with the fix
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});
+});
Loading diff…