Correct nullptr dereference in DocumentLoader::responseReceived #47079

brentfulgham · 2025-06-23T21:44:15Z

`97054b3`

Correct nullptr dereference in DocumentLoader::responseReceived
https://bugs.webkit.org/show_bug.cgi?id=294863
<rdar://problem/153030488>

Reviewed by Charlie Wolfe.

Telemetry from recent betas indicate nullptr crashes in
`DocumentLoader::responseReceived`. This patch corrects this error.

* Source/WebCore/loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::responseReceived): Correct nullptr frame
checks. Also a Drive-by-fix to add missing call to the completion handler.
(WebCore::DocumentLoader::responseReceived): Correct nullptr checks.

Canonical link: https://commits.webkit.org/296582@main

8aa6908

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2025-06-23T21:46:48Z

EWS run on previous version of this PR (hash 5217f45)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

achristensen07 · 2025-06-24T00:05:56Z

Source/WebCore/loader/DocumentLoader.cpp

@@ -1000,14 +1001,17 @@ void DocumentLoader::responseReceived(ResourceResponse&& response, CompletionHan
    if (m_substituteData.isValid() || !platformStrategies()->loaderStrategy()->havePerformedSecurityChecks(response)) {
        auto url = response.url();
        RefPtr frame = m_frame.get();
-        ContentSecurityPolicy contentSecurityPolicy(URL { url }, this, frame->document());
+        ReportingClient* reportingClient = nullptr;


This seems like a step in the wrong direction. Why not just use a RefPtr<Document>?

We don’t want to take ownership of the Document, so perhaps a WeakPtr. After looking at this yesterday, I think this argument to CSP was hacked in a bit poorly, and would need a larger patch to get this right.

I took the approach of matching the logic in the other overload of this method, and intend to follow up with a patch that addresses the ugly bare pointer used for this argument in current code.

webkit-ews-buildbot · 2025-06-24T09:07:09Z

Failed mac-AS-debug-wk2 checks. Please resolve failures and re-apply safe-merge-queue label.

Rejecting #47079 from merge queue.

webkit-ews-buildbot · 2025-06-24T09:07:12Z

Safe-Merge-Queue: Build #61260.

cdumez · 2025-06-24T16:16:49Z

Source/WebCore/loader/DocumentLoader.cpp

@@ -1035,12 +1039,15 @@ void DocumentLoader::responseReceived(ResourceResponse&& response, CompletionHan
    m_response = WTFMove(response);

    if (m_identifierForLoadWithoutResourceLoader) {
+        RefPtr protectedFrameLoader { frameLoader() };


We do not call local variable protectedXXX anymore. This is pre-safer CPP style. This should be:

RefPtr frameLoader = this->frameLoader();

OK, will fix.

cdumez · 2025-06-24T16:19:33Z

Source/WebCore/loader/DocumentLoader.cpp

@@ -1053,18 +1060,24 @@ void DocumentLoader::responseReceived(ResourceResponse&& response, CompletionHan
        return;
    }

+    RefPtr protectedFrame { m_frame.get() };


Why do we need this? Why can't we reuse the frame local variable in this function?

It's not in scope?

Oh, it's possible. It's hard to read diffs sometimes :)

Since we're moving it, we should name it frame, not protectedFrame.

webkit-early-warning-system · 2025-06-24T17:56:19Z

EWS run on current version of this PR (hash 8aa6908)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-ews-buildbot · 2025-06-24T20:50:02Z

Safe-Merge-Queue: Build #61307.

https://bugs.webkit.org/show_bug.cgi?id=294863 <rdar://problem/153030488> Reviewed by Charlie Wolfe. Telemetry from recent betas indicate nullptr crashes in `DocumentLoader::responseReceived`. This patch corrects this error. * Source/WebCore/loader/DocumentLoader.cpp: (WebCore::DocumentLoader::responseReceived): Correct nullptr frame checks. Also a Drive-by-fix to add missing call to the completion handler. (WebCore::DocumentLoader::responseReceived): Correct nullptr checks. Canonical link: https://commits.webkit.org/296582@main

webkit-commit-queue · 2025-06-24T21:04:37Z

Committed 296582@main (97054b3): https://commits.webkit.org/296582@main

Reviewed commits have been landed. Closing PR #47079 and removing active labels.

brentfulgham requested a review from cdumez as a code owner June 23, 2025 21:44

brentfulgham self-assigned this Jun 23, 2025

brentfulgham added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Jun 23, 2025

brentfulgham requested a review from charliewolfe June 23, 2025 22:04

charliewolfe approved these changes Jun 23, 2025

View reviewed changes

brentfulgham added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jun 23, 2025

achristensen07 requested changes Jun 24, 2025

View reviewed changes

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 24, 2025

webkit-ews-buildbot removed the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jun 24, 2025

cdumez reviewed Jun 24, 2025

View reviewed changes

brentfulgham removed the merging-blocked Applied to prevent a change from being merged label Jun 24, 2025

brentfulgham force-pushed the eng/Correct-nullptr-dereference-in-DocumentLoader-responseReceived branch from 5217f45 to 8aa6908 Compare June 24, 2025 17:56

brentfulgham added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jun 24, 2025

webkit-ews-buildbot added merge-queue Applied to send a pull request to merge-queue and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Jun 24, 2025

webkit-commit-queue force-pushed the eng/Correct-nullptr-dereference-in-DocumentLoader-responseReceived branch from 8aa6908 to 97054b3 Compare June 24, 2025 21:04

webkit-commit-queue merged commit 97054b3 into WebKit:main Jun 24, 2025

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Correct nullptr dereference in DocumentLoader::responseReceived #47079

Correct nullptr dereference in DocumentLoader::responseReceived #47079

Uh oh!

brentfulgham commented Jun 23, 2025 •

edited by webkit-early-warning-system

Loading

Uh oh!

webkit-early-warning-system commented Jun 23, 2025 •

edited

Loading

Uh oh!

achristensen07 Jun 24, 2025 •

edited

Loading

Uh oh!

brentfulgham Jun 24, 2025

Uh oh!

webkit-ews-buildbot commented Jun 24, 2025

Uh oh!

webkit-ews-buildbot commented Jun 24, 2025

Uh oh!

cdumez Jun 24, 2025

Uh oh!

brentfulgham Jun 24, 2025

Uh oh!

cdumez Jun 24, 2025

Uh oh!

charliewolfe Jun 24, 2025

Uh oh!

cdumez Jun 24, 2025

Uh oh!

cdumez Jun 24, 2025

Uh oh!

brentfulgham Jun 24, 2025

Uh oh!

webkit-early-warning-system commented Jun 24, 2025 •

edited

Loading

Uh oh!

webkit-ews-buildbot commented Jun 24, 2025

Uh oh!

webkit-commit-queue commented Jun 24, 2025

Uh oh!

Uh oh!

Correct nullptr dereference in DocumentLoader::responseReceived #47079

Correct nullptr dereference in DocumentLoader::responseReceived #47079

Uh oh!

Conversation

brentfulgham commented Jun 23, 2025 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

achristensen07 Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webkit-ews-buildbot commented Jun 24, 2025

Uh oh!

webkit-ews-buildbot commented Jun 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-ews-buildbot commented Jun 24, 2025

Uh oh!

webkit-commit-queue commented Jun 24, 2025

Uh oh!

Uh oh!

brentfulgham commented Jun 23, 2025 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Jun 23, 2025 •

edited

Loading

achristensen07 Jun 24, 2025 •

edited

Loading

webkit-early-warning-system commented Jun 24, 2025 •

edited

Loading