WebKit · WebKit › JSC DFG
CVE-2024-54543
OOB in WebKit (JSC DFG)
Overview
Medium
Severity
8.8
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- DFG CSE
- The DFG JIT’s common-subexpression-elimination phase, which removes redundant loads/stores by matching HeapLocations and can convert a redundant store into an aliased store.
- HeapLocation / LocationKind
- An abstract memory-location key (a LocationKind plus base/index) used by CSE to decide whether two heap accesses touch the same location.
- PutByValAlias
- A specialized PutByVal node the JIT emits when a store is known to alias a previously bounds-checked access, so it stores directly with no bounds check.
- Out-of-bounds sane chain
- An array-access mode where an index beyond the length is safe because the prototype chain has no interfering indexed properties, so reads return undefined rather than trapping.
- ArrayMode / isInBounds / isOutOfBounds
- DFG per-node classification of how an indexed access behaves; PutByValAlias is only valid when the mode is in-bounds.
- Butterfly / public length
- The out-of-line storage for a JS array/object and its stored length; a bounds-check-free store past public length writes out of bounds.
Root Cause Analysis
The patch fixes a DFG common-subexpression-elimination (CSE) modeling error that let out-of-bounds array stores be miscompiled into bounds-check-free PutByValAlias stores. CSE tracks memory effects via HeapLocation values keyed by a LocationKind plus base/index; two accesses are considered to touch the same abstract location only if their HeapLocations match. For indexed properties, JSC distinguishes ordinary in-bounds accesses from ‘out-of-bounds sane chain’ accesses (where an OOB index is safe because the prototype chain is sane and reads simply return undefined). In DFGClobberize.h the def() calls for PutByVal (Int32/Double/Contiguous and the typed-array path) and for DataViewSet used the in-bounds LocationKind (indexedPropertyLocForResultType) even when node->arrayMode().isOutOfBounds() was true, so an out-of-bounds store and an in-bounds access to the same (base,index) produced identical HeapLocations. The violated invariant is that CSE must never treat an out-of-bounds access as equivalent to an in-bounds one. Because they were conflated, CSE could match an out-of-bounds PutByVal against a prior in-bounds access at the same location and rewrite it to PutByValAlias — the aliased-store form that assumes the bounds were already checked and therefore emits no bounds check at all. In DFGSpeculativeJIT.cpp, compileContiguousPutByVal, compileDoublePutByVal and jumpForTypedArrayOutOfBounds all treat PutByValAlias as unconditionally in-bounds (jumpForTypedArrayOutOfBounds returns an empty Jump(), skipping the check). An attacker-controlled OOB index thus reaches a raw store with no bounds check, giving an out-of-bounds write.
The fix adds indexedPropertyLocToOutOfBoundsSaneChain(), a new IndexedPropertyInt52OutOfBoundsSaneChainLoc kind (and reorders the enum), and routes OOB accesses through the sane-chain LocationKind in clobberize so they can no longer be CSE-matched with in-bounds accesses; this prevents the illegal PutByVal->PutByValAlias conversion. The SpeculativeJIT changes add ASSERT_ENABLED in-bounds assertions/breakpoints on the PutByValAlias paths to catch any future violation of the ‘PutByValAlias implies in-bounds’ invariant. The regression test put-by-val-alias-out-of-bounds.js performs array[i] &= 2 over a Uint32Array(65535) with i running well past the length, exactly the read-modify-write pattern that CSE turns into an aliased OOB store. (The clobberize refactor to a local mode variable and the deletion of printInternal(LocationKind) dead code are non-security cleanups; the CSE dataLog additions are tracing only.)
Key insight
Conflating out-of-bounds sane-chain accesses with in-bounds accesses in CSE’s HeapLocation model let an OOB PutByVal be rewritten as a bounds-check-free PutByValAlias; giving OOB accesses distinct location kinds restores the invariant that PutByValAlias is only ever emitted for genuinely in-bounds stores.
Attack Path
- Tier up to DFG Repeatedly execute a small function so JSC compiles it in the DFG, enabling CSE and array-mode specialization.
- Emit a read-modify-write on an indexed array Use array[i] op= value (e.g. array[i] &= 2) which lowers to a GetByVal followed by a PutByVal on the same base and index — the pattern CSE tries to alias.
- Make the access out-of-bounds sane-chain Drive the index i beyond the array length with a sane prototype chain so the access is compiled in an out-of-bounds mode rather than being rejected.
- Get the store turned into PutByValAlias Because the OOB access shares a HeapLocation with an in-bounds access, CSE matches them and rewrites the PutByVal to PutByValAlias, which the SpeculativeJIT compiles with no bounds check.
- Perform the out-of-bounds write The bounds-check-free aliased store writes the value at base+index*elementSize past the buffer/butterfly, giving a controlled-offset OOB write (index and value are attacker-controlled).
- Escalate to arbitrary R/W and RCE Standard JSC exploitation (background): groom adjacent objects/butterflies, corrupt a length or structure to build addrof/fakeobj and arbitrary read/write, then achieve code execution inside the WebContent process.
Impact Assessment
The primitive is a controlled out-of-bounds write on an indexed array/typed array: attacker-chosen index and value stored with the bounds check elided, one of the strongest JSC primitives. Realistic escalation is straightforward to arbitrary read/write (corrupt an adjacent butterfly length or a typed-array vector/length, build addrof/fakeobj) and then to remote code execution. It is reachable purely from crafted JavaScript, matching the CVE’s ‘maliciously crafted web content -> memory corruption’ wording. It is confined to the WebContent process under the WebKit sandbox, so full compromise still needs a separate sandbox escape.
Changed Functions
| Function | Change | Notes |
|---|---|---|
switchSource/JavaScriptCore/dfg/DFGHeapLocation.cpp |
modified |
Files Changed
Source/JavaScriptCore/dfg/DFGCSEPhase.cppSource/JavaScriptCore/dfg/DFGClobberize.hSource/JavaScriptCore/dfg/DFGHeapLocation.cpp
Audit Directions
- Every clobberize def() for indexed/DataView accessesIn DFGClobberize.h audit each GetByVal/PutByVal/DataViewGet/DataViewSet arm for whether it selects the OutOfBoundsSaneChain LocationKind when arrayMode().isOutOfBounds(); grep for indexedPropertyLocForResultType, indexedPropertyLocToOutOfBoundsSaneChain, and any def(HeapLocation(…)) that ignores isOutOfBounds().
- PutByValAlias emission sitesGrep the SpeculativeJIT and FTL for PutByValAlias and confirm every path assumes/asserts isInBounds() and mayBeResizableOrGrowableSharedTypedArray() is false; check jumpForTypedArrayOutOfBounds-style helpers that return an empty Jump().
- Alias conversion logicFind where PutByVal is downgraded to PutByValAlias (ArrayMode/CSE/FixupPhase) and verify the aliasing candidate’s HeapLocation truly matches only in-bounds accesses; look for the LocationKind comparisons and canonicalResultRepresentation usage.
- Completeness of sane-chain LocationKindsAudit the LocationKind enum and indexedPropertyLocToOutOfBoundsSaneChain switch for any result representation lacking a distinct OOB variant (the Int52 case was the missing one); ensure no default/RELEASE_ASSERT_NOT_REACHED path silently reuses an in-bounds kind.
Patch
diff --git a/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp b/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp
index 1909d7741fed..1c044e4bbf2c 100644
--- a/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp
@@ -524,6 +524,7 @@ private:
bool run(BasicBlock* block)
{
+ dataLogLnIf(DFGCSEPhaseInternal::verbose, "Starting block: ", block->index);
m_maps.clear();
m_changed = false;
m_block = block;
@@ -607,11 +608,13 @@ private:
void write(AbstractHeap heap)
{
+ dataLogLnIf(DFGCSEPhaseInternal::verbose, "\tWrite to heap ", heap);
m_maps.write(heap);
}
void def(PureValue value)
{
+ dataLogLnIf(DFGCSEPhaseInternal::verbose, "\tDef of value ", value, " at node ", m_node->index());
Node* match = m_maps.addPure(value, m_node);
if (!match)
return;
@@ -622,6 +625,7 @@ private:
void def(const HeapLocation& location, const LazyNode& value)
{
+ dataLogLnIf(DFGCSEPhaseInternal::verbose, "\tDef to ", location, " of value ", value, " at node ", m_node->index());
LazyNode match = m_maps.addImpure(location, value);
if (!match)
return;
diff --git a/Source/JavaScriptCore/dfg/DFGClobberize.h b/Source/JavaScriptCore/dfg/DFGClobberize.h
index 2fc89697731f..592a668ba9ee 100644
--- a/Source/JavaScriptCore/dfg/DFGClobberize.h
+++ b/Source/JavaScriptCore/dfg/DFGClobberize.h
@@ -60,14 +60,6 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
// easiest way of indicating that something cannot be hoisted is to claim
// that it side-effects some miscellaneous thing.
//
- // - We cannot hoist forward-exiting nodes without some additional effort. I
- // believe that what it comes down to is that forward-exiting generally have
- // their NodeExitsForward cleared upon hoist, except for forward-exiting
- // nodes that take bogus state as their input. Those are substantially
- // harder. We disable it for now. In the future we could enable it by having
- // versions of those nodes that backward-exit instead, but I'm not convinced
- // of the soundness.
- //
// - Some nodes lie, and claim that they do not read the JSCell_structureID,
// JSCell_typeInfoFlags, etc. These are nodes that use the structure in a way
// that does not depend on things that change under structure transitions.
@@ -1116,8 +1108,11 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
if (mode.mayBeResizableOrGrowableSharedTypedArray()) {
write(MiscFields);
write(TypedArrayProperties);
- } else
+ } else {
+ if (mode.isOutOfBounds())
+ indexedPropertyLoc = indexedPropertyLocToOutOfBoundsSaneChain(indexedPropertyLoc);
def(HeapLocation(indexedPropertyLoc, TypedArrayProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
+ }
return;
// We should not get an AnyTypedArray in a GetByVal as AnyTypedArray is only created from intrinsics, which
// are only added from Inline Caching a GetById.
@@ -1167,7 +1162,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
return;
case Array::Int32:
- if (node->arrayMode().isOutOfBounds()) {
+ if (mode.isOutOfBounds()) {
clobberTop();
return;
}
@@ -1175,14 +1170,14 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
read(Butterfly_vectorLength);
read(IndexedInt32Properties);
write(IndexedInt32Properties);
- if (node->arrayMode().mayStoreToHole())
+ if (mode.mayStoreToHole())
write(Butterfly_publicLength);
def(HeapLocation(indexedPropertyLoc, IndexedInt32Properties, base, index), LazyNode(value));
def(HeapLocation(IndexedPropertyInt32OutOfBoundsSaneChainLoc, IndexedInt32Properties, base, index), LazyNode(value));
return;
case Array::Double:
- if (node->arrayMode().isOutOfBounds()) {
+ if (mode.isOutOfBounds()) {
clobberTop();
return;
}
@@ -1190,7 +1185,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
read(Butterfly_vectorLength);
read(IndexedDoubleProperties);
write(IndexedDoubleProperties);
- if (node->arrayMode().mayStoreToHole())
+ if (mode.mayStoreToHole())
write(Butterfly_publicLength);
def(HeapLocation(IndexedPropertyDoubleLoc, IndexedDoubleProperties, base, index), LazyNode(value));
def(HeapLocation(IndexedPropertyDoubleSaneChainLoc, IndexedDoubleProperties, base, index), LazyNode(value));
@@ -1198,7 +1193,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
return;
case Array::Contiguous:
- if (node->arrayMode().isOutOfBounds()) {
+ if (mode.isOutOfBounds()) {
clobberTop();
return;
}
@@ -1206,7 +1201,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
read(Butterfly_vectorLength);
read(IndexedContiguousProperties);
write(IndexedContiguousProperties);
- if (node->arrayMode().mayStoreToHole())
+ if (mode.mayStoreToHole())
write(Butterfly_publicLength);
def(HeapLocation(indexedPropertyLoc, IndexedContiguousProperties, base, index), LazyNode(value));
def(HeapLocation(IndexedPropertyJSOutOfBoundsSaneChainLoc, IndexedContiguousProperties, base, index), LazyNode(value));
@@ -1221,12 +1216,12 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
read(Butterfly_vectorLength);
read(IndexedArrayStorageProperties);
write(IndexedArrayStorageProperties);
- if (node->arrayMode().mayStoreToHole())
+ if (mode.mayStoreToHole())
write(Butterfly_publicLength);
return;
case Array::SlowPutArrayStorage:
- if (node->arrayMode().mayStoreToHole()) {
+ if (mode.mayStoreToHole()) {
clobberTop();
return;
}
@@ -1246,7 +1241,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
case Array::Float16Array:
case Array::Float32Array:
case Array::Float64Array:
- if (node->arrayMode().mayBeResizableOrGrowableSharedTypedArray()) {
+ if (mode.mayBeResizableOrGrowableSharedTypedArray()) {
read(TypedArrayProperties);
read(MiscFields);
write(TypedArrayProperties);
@@ -2266,7 +2261,7 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
write(MiscFields);
write(TypedArrayProperties);
} else {
- LocationKind indexedPropertyLoc = indexedPropertyLocForResultType(node->result());
+ LocationKind indexedPropertyLoc = indexedPropertyLocToOutOfBoundsSaneChain(indexedPropertyLocForResultType(node->result()));
def(HeapLocation(indexedPropertyLoc, AbstractHeap(TypedArrayProperties, node->dataViewData().asQuadWord), node->child1(), node->child2(), node->child3()), LazyNode(node));
}
return;
diff --git a/Source/JavaScriptCore/dfg/DFGHeapLocation.cpp b/Source/JavaScriptCore/dfg/DFGHeapLocation.cpp
index 3675e7845f7b..21cc6315d8da 100644
--- a/Source/JavaScriptCore/dfg/DFGHeapLocation.cpp
+++ b/Source/JavaScriptCore/dfg/DFGHeapLocation.cpp
@@ -47,254 +47,5 @@ void HeapLocation::dump(PrintStream& out) const
} } // namespace JSC::DFG
-namespace WTF {
-
-using namespace JSC::DFG;
-
-void printInternal(PrintStream& out, LocationKind kind)
-{
- switch (kind) {
- case InvalidLocationKind:
- out.print("InvalidLocationKind");
- return;
-
- case InvalidationPointLoc:
- out.print("InvalidationPointLoc");
- return;
-
- case TypeOfIsObjectLoc:
- out.print("TypeOfIsObjectLoc");
- return;
-
- case TypeOfIsFunctionLoc:
- out.print("TypeOfIsFunctionLoc");
- return;
-
- case IsCallableLoc:
- out.print("IsCallableLoc");
- return;
-
- case IsConstructorLoc:
- out.print("IsConstructorLoc");
- return;
-
- case GetterLoc:
- out.print("GetterLoc");
- return;
-
- case SetterLoc:
- out.print("SetterLoc");
- return;
-
- case StackLoc:
- out.print("StackLoc");
- return;
-
- case StackPayloadLoc:
- out.print("StackPayloadLoc");
- return;
-
- case GlobalProxyTargetLoc:
- out.print("GlobalProxyTargetLoc");
- return;
-
- case ArrayLengthLoc:
- out.print("ArrayLengthLoc");
- return;
-
- case ArrayMaskLoc:
- out.print("ArrayMaskLoc");
- return;
-
- case VectorLengthLoc:
- out.print("VectorLengthLoc");
- return;
-
- case ButterflyLoc:
- out.print("ButterflyLoc");
- return;
-
- case CheckTypeInfoFlagsLoc:
- out.print("CheckTypeInfoFlagsLoc");
- return;
-
- case OverridesHasInstanceLoc:
- out.print("OverridesHasInstanceLoc");
- return;
-
- case ClosureVariableLoc:
- out.print("ClosureVariableLoc");
- return;
-
- case DirectArgumentsLoc:
- out.print("DirectArgumentsLoc");
- return;
-
- case GlobalVariableLoc:
- out.print("GlobalVariableLoc");
- return;
-
- case HasIndexedPropertyLoc:
- out.print("HasIndexedPorpertyLoc");
- return;
-
- case EnumeratorNextUpdateIndexAndModeLoc:
- out.print("EnumeratorNextUpdateIndexAndModeLoc");
- return;
-
- case IndexedPropertyDoubleLoc:
- out.print("IndexedPropertyDoubleLoc");
- return;
-
- case IndexedPropertyDoubleSaneChainLoc:
- out.print("IndexedPropertyDoubleSaneChainLoc");
- return;
-
- case IndexedPropertyDoubleOutOfBoundsSaneChainLoc:
- out.print("IndexedPropertyDoubleOutOfBoundsSaneChainLoc");
- return;
-
- case IndexedPropertyDoubleOrOtherOutOfBoundsSaneChainLoc:
- out.print("IndexedPropertyDoubleOrOtherOutOfBoundsSaneChainLoc");
- return;
-
- case IndexedPropertyInt32Loc:
- out.print("IndexedPropertyInt32Loc");
- return;
-
- case IndexedPropertyInt32OutOfBoundsSaneChainLoc:
- out.print("IndexedPropertyInt32OutOfBoundsSaneChainLoc");
- return;
-
- case IndexedPropertyInt52Loc:
- out.print("IndexedPropertyInt52Loc");
- return;
-
- case IndexedPropertyJSLoc:
- out.print("IndexedPropertyJSLoc");
- return;
-
- case IndexedPropertyJSOutOfBoundsSaneChainLoc:
- out.print("IndexedPropertyJSOutOfBoundsSaneChainLoc");
- return;
-
- case IndexedPropertyStorageLoc:
- out.print("IndexedPropertyStorageLoc");
- return;
-
- case NamedPropertyLoc:
- out.print("NamedPropertyLoc");
- return;
-
- case TypedArrayByteOffsetLoc:
- out.print("TypedArrayByteOffsetLoc");
- return;
-
- case TypedArrayByteOffsetInt52Loc:
- out.print("TypedArrayByteOffsetInt52Loc");
- return;
-
- case TypedArrayLengthInt52Loc:
- out.print("TypedArrayLengthInt52Loc");
- return;
-
- case PrototypeLoc:
- out.print("PrototypeLoc");
- return;
-
- case StructureLoc:
- out.print("StructureLoc");
- return;
-
- case RegExpObjectLastIndexLoc:
- out.print("RegExpObjectLastIndexLoc");
- return;
-
- case DateFieldLoc:
- out.print("DateFieldLoc");
- return;
-
- case MapBucketLoc:
- out.print("MapBucketLoc");
- return;
-
- case MapBucketHeadLoc:
- out.print("MapBucketHeadLoc");
- return;
-
- case MapBucketKeyLoc:
- out.print("MapBucketKeyLoc");
- return;
-
- case MapBucketValueLoc:
- out.print("MapBucketValueLoc");
- return;
-
- case MapBucketNextLoc:
- out.print("MapBucketNextLoc");
- return;
-
- case MapIteratorNextLoc:
- out.print("MapIteratorNextLoc");
- return;
-
- case MapIteratorKeyLoc:
- out.print("MapIteratorKeyLoc");
- return;
-
- case MapIteratorValueLoc:
- out.print("MapIteratorValueLoc");
- return;
-
- case MapStorageLoc:
- out.print("MapStorageLoc");
- return;
-
- case MapIterationNextLoc:
- out.print("MapIterationNextLoc");
- return;
-
- case MapIterationEntryLoc:
- out.print("MapIterationEntryLoc");
- return;
-
- case MapIterationEntryKeyLoc:
- out.print("MapIterationEntryKeyLoc");
- return;
-
- case MapIterationEntryValueLoc:
- out.print("MapIterationEntryValueLoc");
- return;
-
- case MapEntryKeyLoc:
- out.print("MapEntryKeyLoc");
- return;
-
- case MapEntryValueLoc:
- out.print("MapEntryValueLoc");
- return;
-
- case LoadMapValueLoc:
- out.print("LoadMapValueLoc");
- return;
-
- case WeakMapGetLoc:
- out.print("WeakMapGetLoc");
- return;
-
- case InternalFieldObjectLoc:
- out.print("InternalFieldObjectLoc");
- return;
-
- case DOMStateLoc:
... (file diff truncated)
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/JSTests/stress/put-by-val-alias-out-of-bounds.js b/JSTests/stress/put-by-val-alias-out-of-bounds.js
new file mode 100644
index 000000000000..ebd74a75ae6d
--- /dev/null
+++ b/JSTests/stress/put-by-val-alias-out-of-bounds.js
@@ -0,0 +1,9 @@
+let array = new Uint32Array(65535);
+function test() {
+ for (let i = 0; i < 1e5; i += 6)
+ array[i] &= 2;
+ return array;
+}
+noInline(test);
+
+test();
\ No newline at end of file
Loading diff…
Original Bug Report
The reporter's bug is still restricted on the tracker.
References
On This Page