diff --git a/third_party/blink/renderer/core/svg/build.gni b/third_party/blink/renderer/core/svg/build.gni index f5d0ae167fd5fc..0445447e3eba5e 100644 --- a/third_party/blink/renderer/core/svg/build.gni +++ b/third_party/blink/renderer/core/svg/build.gni @@ -370,6 +370,7 @@ blink_core_tests_svg = [ "svg_path_query_test.cc", "svg_resource_document_content_test.cc", "svg_svg_element_test.cc", + "svg_tree_scope_resources_test.cc", "svg_text_content_element_test.cc", "svg_use_element_test.cc", "unsafe_svg_attribute_sanitization_test.cc", diff --git a/third_party/blink/renderer/core/svg/svg_tree_scope_resources.h b/third_party/blink/renderer/core/svg/svg_tree_scope_resources.h index ea42b45171de85..d363ac117e2528 100644 --- a/third_party/blink/renderer/core/svg/svg_tree_scope_resources.h +++ b/third_party/blink/renderer/core/svg/svg_tree_scope_resources.h @@ -32,6 +32,8 @@ class SVGTreeScopeResources final void Trace(Visitor*) const; private: + friend class SVGTreeScopeResourcesTest; + void ProcessCustomWeakness(const LivenessBroker&); HashMap> resources_; diff --git a/third_party/blink/renderer/core/svg/svg_tree_scope_resources_test.cc b/third_party/blink/renderer/core/svg/svg_tree_scope_resources_test.cc new file mode 100644 index 00000000000000..0f669802cc0421 --- /dev/null +++ b/third_party/blink/renderer/core/svg/svg_tree_scope_resources_test.cc @@ -0,0 +1,128 @@ +// Copyright 2025 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "third_party/blink/renderer/core/svg/svg_tree_scope_resources.h" + +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/blink/renderer/core/dom/document.h" +#include "third_party/blink/renderer/core/svg/svg_resource.h" +#include "third_party/blink/renderer/core/testing/page_test_base.h" +#include "third_party/blink/renderer/platform/heap/persistent.h" +#include "third_party/blink/renderer/platform/heap/thread_state.h" +#include "third_party/blink/renderer/platform/wtf/text/atomic_string.h" +#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h" + +namespace blink { + +// Friend of SVGTreeScopeResources so the test can inspect `resources_`. +class SVGTreeScopeResourcesTest : public PageTestBase { + public: + static HashMap>& Map( + SVGTreeScopeResources& r) { + return r.resources_; + } +}; + +// Demonstrates the precondition for the use-after-free in +// `SVGTreeScopeResources::ResourceForId()`: +// +// auto& entry = resources_.insert(id, nullptr).stored_value->value; // (A) +// if (!entry) +// entry = MakeGarbageCollected(...); // (B) +// return entry.Get(); // (C) +// +// `resources_` is a *PartitionAlloc-backed* WTF::HashMap. At (B) the +// allocation may run a synchronous major GC at its safepoint +// (CppHeap::ReportBufferedAllocationSizeIfPossible → +// FinalizeIncrementalMarkingAtomically, or RetryAllocate). That GC invokes +// `ProcessCustomWeakness()`, which calls `resources_.RemoveAll(dead)`. +// Because `PartitionAllocator::IsAllocationAllowed()` is hard-coded `true`, +// the per-key `erase()` loop is allowed to `Shrink()` and free the old +// backing — the buffer that `entry` still points into. The store at (B) and +// the load at (C) are then a write-/read-after-free. +// +// This test reproduces every step except the nested-GC trigger, which it +// replaces with an explicit `CollectAllGarbageForTesting()` at exactly the +// point where (B) would allocate. It then proves that the slot referenced by +// `entry` no longer lives in the map's backing, i.e. the original backing was +// freed and `entry` is dangling. In a production build the same thing +// happens whenever the safepoint at (B) runs an atomic GC. +// +// Note: ASan does not poison PartitionAlloc memory, so the actual store +// `entry = ptr` would not be reported by ASan even though it is a UAF. This +// test therefore asserts the dangling-reference condition directly. +TEST_F(SVGTreeScopeResourcesTest, StoredValueDanglesAcrossNestedGC) { + SVGTreeScopeResources& res = GetDocument().EnsureSVGTreeScopedResources(); + + // 1) Populate `resources_` so that table_size_ ≥ 1024. Keep strong refs + // so any incidental GC during setup keeps the entries. + constexpr int kDead = 400; + { + Vector> keep; + keep.reserve(kDead); + for (int i = 0; i < kDead; ++i) { + keep.push_back( + res.ResourceForId(AtomicString("dead" + String::Number(i)))); + } + EXPECT_EQ(Map(res).size(), static_cast(kDead)); + // Drop all strong refs. The only remaining references are the + // UntracedMember<> slots in `resources_`, which Trace() does not visit. + } + + // 2) Mimic `ResourceForId()` line 20 for a brand-new id. + // LivenessBroker::IsHeapObjectAlive(nullptr) returns true, so this slot + // survives `ProcessCustomWeakness()`. + const AtomicString fresh("fresh"); + auto& entry = Map(res).insert(fresh, nullptr).stored_value->value; + void* const slot_before = static_cast(&entry); + + // 3) Mimic the synchronous GC that can fire at the + // `MakeGarbageCollected` safepoint at line 22. + // This runs Trace() (registers ProcessCustomWeakness) and then the + // weak-processing phase, which removes the kDead dead entries via + // `RemoveAll()`. Because PartitionAllocator::IsAllocationAllowed() + // is unconditionally true, the per-key erase() loop is allowed to + // Shrink() and free the old backing. + ThreadState::Current()->CollectAllGarbageForTesting( + ThreadState::StackState::kNoHeapPointers); + + // 4) The "fresh" slot survived the weak pass (nullptr is treated as alive) + // and was moved into the new, smaller backing. + ASSERT_TRUE(Map(res).Contains(fresh)); + void* const slot_after = static_cast(&Map(res).find(fresh)->value); + + // 5) If these differ, `entry` now refers to freed PartitionAlloc memory. + // In `ResourceForId()` the next statement is `entry = ` — a + // write-after-free — followed by `entry.Get()` — a read-after-free. + EXPECT_NE(slot_before, slot_after) + << "resources_ backing was NOT reallocated by ProcessCustomWeakness; " + "stored_value reference would still be valid (no UAF precondition)"; + + // 6) Dead entries were actually removed (table shrank to near-minimum). + EXPECT_LT(Map(res).size(), 10u); + + // 7) Perform exactly what `ResourceForId()` does at line 22 with the + // now-dangling reference. This is a 4-byte compressed-pointer store + // into freed PartitionAlloc memory (write-after-free). ASan does not + // poison PartitionAlloc memory so it cannot flag this directly; instead + // we observe the *secondary* correctness consequence: the store never + // reaches the live map, which still holds nullptr for "fresh". + Persistent r = + MakeGarbageCollected(GetDocument(), fresh); + entry = r.Get(); // ← write-after-free into freed B0 + LocalSVGResource* ret = entry.Get(); // ← read-after-free from B0 + + // The write went to freed memory, so the live map slot is unchanged. + EXPECT_EQ(Map(res).find(fresh)->value.Get(), + static_cast(nullptr)) + << "write through dangling stored_value reached the live map " + "(backing was not actually freed)"; + + // `ret` was loaded from the same freed slot. If the freed slot happened to + // be reused or zapped between the free and the store, this would be a wild + // pointer; here it round-trips because nothing reclaimed B0 yet. + (void)ret; +} + +} // namespace blink