PatchGap
WebKit · WebKit › WebCore DOM

CVE-2024-54502

UAF in WebKit (WebCore DOM)
Medium CVSS 6.5 webkit UAF 🔧 Commit mapped

Overview

Medium
Severity
6.5
CVSS
No
Exploited ITW
Fixed
Fix Status
DescriptionProcessing maliciously crafted web content may lead to an unexpected process crash
ComponentWebKit
Sub-componentWebCore DOM
Bug ClassUAF
Tracker281912
Fix commite7b7957de026 (WebKit/WebKit)
CWECWE-125 (Out-of-bounds read)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:N/I:N/A:H
CISA KEVNot listed
CreditedBrendon Tiszka of Google Project Zero
Disclosed2024-12-11

Background

DocumentFontLoader
Per-Document helper that drives pending @font-face loads.
Re-entrancy via resource loads
Font/resource loading can fire events (e.g. iframe onload) that run author script mid-operation.
Ref protectedThis
Taking a strong reference to self at the top of a method keeps the object alive across callbacks that might otherwise free it.
makeUniqueWithoutRefCountedCheck
Creates a unique_ptr to a type that is (unusually) ref-counted, bypassing the standard assertion.

Root Cause Analysis

Document owns a DocumentFontLoader as a plain unique_ptr member (m_fontLoader) with no independent reference counting. DocumentFontLoader::loadPendingFonts() iterates and starts pending font loads, and font loading can synchronously run author script — for example an <iframe onload> handler that removes itself, which can tear down its Document and, transitively, free the DocumentFontLoader while loadPendingFonts() is still executing on it. Operating on the freed loader (or the freed Document it points at) after the callback is a use-after-free.

The fix gives DocumentFontLoader lifetime tied to its Document: it adds ref()/deref() that forward to m_document, is created with makeUniqueWithoutRefCountedCheck, and loadPendingFonts() takes Ref protectedThis { *this } at entry so the loader (and its Document) are kept alive for the duration of the potentially-reentrant load, even if script destroys the document mid-iteration.

The restored invariant is that the font loader cannot be freed while it is running font loads that may re-enter through script. The layout test loads a cached font and removes the loading iframes on their onload, reproducing the destroy-during-load re-entrancy under ASan.

Key insight
DocumentFontLoader had no lifetime protection, so script running during loadPendingFonts() could destroy the document and free the loader mid-iteration; ref-counting it through its Document and protecting this fixes the UAF.

Attack Path

  1. Cache a font and load it Declare an @font-face used by the page so font loading is triggered.
  2. Run script during load Use subframes (or other resources) whose onload handlers run while loadPendingFonts() is iterating, e.g. iframes that remove themselves on load.
  3. Destroy the document mid-load The re-entrant script tears down a Document, freeing its DocumentFontLoader while loadPendingFonts() is still on the stack.
  4. Use-after-free loadPendingFonts() continues to use the freed loader/document, corrupting memory in the WebContent process.

Impact Assessment

A use-after-free of a Document’s font loader when script destroys the document during font loading, reachable from ordinary @font-face usage plus self-removing subframes, in the WebContent process. Attacker control over the re-entrancy timing makes it a groomable UAF; the observable is an ASan crash. Confined to WebContent; rated medium.

Changed Functions

FunctionChangeNotes
if
LayoutTests/http/tests/security/document-cached-font-loading.html
 modified

Files Changed

  • Source/WebCore/dom/Document.cpp
  • Source/WebCore/dom/DocumentFontLoader.cpp
  • Source/WebCore/dom/DocumentFontLoader.h
  • LayoutTests/http/tests/security/document-cached-font-loading-expected.txt
  • LayoutTests/http/tests/security/document-cached-font-loading.html
  • LayoutTests/http/tests/security/resources/document-cached-font-loading-helper.html

Audit Directions

  • Other unprotected per-Document helpers
    grep WebCore/dom for unique_ptr members whose methods run resource loads/callbacks without a Ref protectedThis guard.
  • loadPending*/callback loops
    Audit loaders that iterate pending work and invoke handlers able to run script or destroy the owner.
  • self-destroying iframe re-entrancy
    Review paths reachable during subframe onload that can synchronously destroy the parent document.

Patch

view commit
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index 994791cc207b..086344234d18 100644
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -1017,7 +1017,7 @@ DocumentFontLoader& Document::ensureFontLoader()
 {
     ASSERT(m_constructionDidFinish);
     ASSERT(!m_fontLoader);
-    m_fontLoader = makeUnique<DocumentFontLoader>(*this);
+    m_fontLoader = makeUniqueWithoutRefCountedCheck<DocumentFontLoader>(*this);
     return *m_fontLoader;
 }
 
diff --git a/Source/WebCore/dom/DocumentFontLoader.cpp b/Source/WebCore/dom/DocumentFontLoader.cpp
index be8629ab5969..929caa15eef6 100644
--- a/Source/WebCore/dom/DocumentFontLoader.cpp
+++ b/Source/WebCore/dom/DocumentFontLoader.cpp
@@ -99,6 +99,7 @@ void DocumentFontLoader::loadPendingFonts()
 
 void DocumentFontLoader::fontLoadingTimerFired()
 {
+    Ref protectedThis { *this };
     loadPendingFonts();
 
     // FIXME: Use SubresourceLoader instead.
diff --git a/Source/WebCore/dom/DocumentFontLoader.h b/Source/WebCore/dom/DocumentFontLoader.h
index 7dba4f427ff6..dee268564819 100644
--- a/Source/WebCore/dom/DocumentFontLoader.h
+++ b/Source/WebCore/dom/DocumentFontLoader.h
@@ -43,6 +43,9 @@ public:
     DocumentFontLoader(Document&);
     ~DocumentFontLoader();
 
+    void ref() const { m_document->ref(); }
+    void deref() const { m_document->deref(); }
+
     CachedFont* cachedFont(URL&&, bool, bool, LoadedFromOpaqueSource);
     void beginLoadingFontSoon(CachedFont&);
 
diff --git a/LayoutTests/http/tests/security/document-cached-font-loading-expected.txt b/LayoutTests/http/tests/security/document-cached-font-loading-expected.txt
new file mode 100644
index 000000000000..654ddf7f17ef
--- /dev/null
+++ b/LayoutTests/http/tests/security/document-cached-font-loading-expected.txt
@@ -0,0 +1 @@
+This test passes if it does not crash.
diff --git a/LayoutTests/http/tests/security/document-cached-font-loading.html b/LayoutTests/http/tests/security/document-cached-font-loading.html
new file mode 100644
index 000000000000..38a0812044ea
--- /dev/null
+++ b/LayoutTests/http/tests/security/document-cached-font-loading.html
@@ -0,0 +1,22 @@
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+}
+</script>
+<style>
+/* cache the font */
+@font-face {
+    font-family: 'uaf';
+    src: url(a.ttf);
+}
+body {
+    font-family: 'uaf';
+}
+</style>
+</head>
+<body>
+    This test passes if it does not crash.
+    <iframe id="a" onload="document.getElementById('a').remove();" src="../resources/document-cached-font-loading-helper.html"></iframe>
+    <iframe id="b" onload="document.getElementById('b').remove();" src="../resources/document-cached-font-loading-helper.html"></iframe>
+</body>
diff --git a/LayoutTests/http/tests/security/resources/document-cached-font-loading-helper.html b/LayoutTests/http/tests/security/resources/document-cached-font-loading-helper.html
new file mode 100644
index 000000000000..a5fdf917f5eb
--- /dev/null
+++ b/LayoutTests/http/tests/security/resources/document-cached-font-loading-helper.html
@@ -0,0 +1,12 @@
+<head>
+<style>
+@font-face {
+    font-family: 'uaf';
+    src: url(font.ttf);
+}
+body {
+    font-family: 'uaf';
+}
+</style>
+</head>
+<body>text</body>
diff --git a/LayoutTests/http/tests/security/resources/font.ttf b/LayoutTests/http/tests/security/resources/font.ttf
new file mode 100644
index 000000000000..672bf761fe9e
Binary files /dev/null and b/LayoutTests/http/tests/security/resources/font.ttf differ
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/LayoutTests/http/tests/security/document-cached-font-loading-expected.txt b/LayoutTests/http/tests/security/document-cached-font-loading-expected.txt
new file mode 100644
index 000000000000..654ddf7f17ef
--- /dev/null
+++ b/LayoutTests/http/tests/security/document-cached-font-loading-expected.txt
@@ -0,0 +1 @@
+This test passes if it does not crash.
diff --git a/LayoutTests/http/tests/security/document-cached-font-loading.html b/LayoutTests/http/tests/security/document-cached-font-loading.html
new file mode 100644
index 000000000000..38a0812044ea
--- /dev/null
+++ b/LayoutTests/http/tests/security/document-cached-font-loading.html
@@ -0,0 +1,22 @@
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+}
+</script>
+<style>
+/* cache the font */
+@font-face {
+    font-family: 'uaf';
+    src: url(a.ttf);
+}
+body {
+    font-family: 'uaf';
+}
+</style>
+</head>
+<body>
+    This test passes if it does not crash.
+    <iframe id="a" onload="document.getElementById('a').remove();" src="../resources/document-cached-font-loading-helper.html"></iframe>
+    <iframe id="b" onload="document.getElementById('b').remove();" src="../resources/document-cached-font-loading-helper.html"></iframe>
+</body>
diff --git a/LayoutTests/http/tests/security/resources/document-cached-font-loading-helper.html b/LayoutTests/http/tests/security/resources/document-cached-font-loading-helper.html
new file mode 100644
index 000000000000..a5fdf917f5eb
--- /dev/null
+++ b/LayoutTests/http/tests/security/resources/document-cached-font-loading-helper.html
@@ -0,0 +1,12 @@
+<head>
+<style>
+@font-face {
+    font-family: 'uaf';
+    src: url(font.ttf);
+}
+body {
+    font-family: 'uaf';
+}
+</style>
+</head>
+<body>text</body>
diff --git a/LayoutTests/http/tests/security/resources/font.ttf b/LayoutTests/http/tests/security/resources/font.ttf
new file mode 100644
index 000000000000..672bf761fe9e
Binary files /dev/null and b/LayoutTests/http/tests/security/resources/font.ttf differ
Loading diff…

Original Bug Report

The reporter's bug is still restricted on the tracker.
On This Page