- Revision
- 287814
- Author
- wenson_hs...@apple.com
- Date
- 2022-01-08 14:12:16 -0800 (Sat, 08 Jan 2022)
Log Message
Null pointer crash when calling into `-[WebView close]` in `-webView:didCommitLoadForFrame:`
https://bugs.webkit.org/show_bug.cgi?id=234994
rdar://86845512
Reviewed by Chris Dumez and Geoff Garen.
Source/WebCore:
Add a null check in `FrameLoader::dispatchDidCommitLoad()` before calling into any methods on the frame's Page.
Since we called into the client layer to `dispatchDidCommitLoad()` immediately prior to this, a legacy WebKit
client may have already caused the page to be destroyed (e.g. by closing the web view). Since Frame's pointer
to Page is a WeakPtr, this triggers null derefs when attempting to call either `didCommitLoad()` or
`remoteInspectorInformationDidChange()`.
Test: WebKitLegacy.CloseWhileCommittingLoad
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::dispatchDidCommitLoad):
Tools:
Add a new API test to exercise the fix.
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKitCocoa/CloseWhileCommittingLoad.mm: Added.
(-[CloseWhileCommittingLoadDelegate webView:didCommitLoadForFrame:]):
(TestWebKitAPI::TEST):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (287813 => 287814)
--- trunk/Source/WebCore/ChangeLog 2022-01-08 18:26:13 UTC (rev 287813)
+++ trunk/Source/WebCore/ChangeLog 2022-01-08 22:12:16 UTC (rev 287814)
@@ -1,3 +1,22 @@
+2022-01-08 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Null pointer crash when calling into `-[WebView close]` in `-webView:didCommitLoadForFrame:`
+ https://bugs.webkit.org/show_bug.cgi?id=234994
+ rdar://86845512
+
+ Reviewed by Chris Dumez and Geoff Garen.
+
+ Add a null check in `FrameLoader::dispatchDidCommitLoad()` before calling into any methods on the frame's Page.
+ Since we called into the client layer to `dispatchDidCommitLoad()` immediately prior to this, a legacy WebKit
+ client may have already caused the page to be destroyed (e.g. by closing the web view). Since Frame's pointer
+ to Page is a WeakPtr, this triggers null derefs when attempting to call either `didCommitLoad()` or
+ `remoteInspectorInformationDidChange()`.
+
+ Test: WebKitLegacy.CloseWhileCommittingLoad
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::dispatchDidCommitLoad):
+
2022-01-08 Gabriel Nava Marino <gnavamar...@apple.com>
null ptr deref in WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (287813 => 287814)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2022-01-08 18:26:13 UTC (rev 287813)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2022-01-08 22:12:16 UTC (rev 287814)
@@ -4039,14 +4039,14 @@
m_client->dispatchDidCommitLoad(initialHasInsecureContent, initialUsedLegacyTLS);
- if (m_frame.isMainFrame())
- m_frame.page()->didCommitLoad();
+ if (auto* page = m_frame.page(); page && m_frame.isMainFrame())
+ page->didCommitLoad();
InspectorInstrumentation::didCommitLoad(m_frame, m_documentLoader.get());
#if ENABLE(REMOTE_INSPECTOR)
- if (m_frame.isMainFrame())
- m_frame.page()->remoteInspectorInformationDidChange();
+ if (auto* page = m_frame.page(); page && m_frame.isMainFrame())
+ page->remoteInspectorInformationDidChange();
#endif
}
Modified: trunk/Tools/ChangeLog (287813 => 287814)
--- trunk/Tools/ChangeLog 2022-01-08 18:26:13 UTC (rev 287813)
+++ trunk/Tools/ChangeLog 2022-01-08 22:12:16 UTC (rev 287814)
@@ -1,3 +1,18 @@
+2022-01-08 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Null pointer crash when calling into `-[WebView close]` in `-webView:didCommitLoadForFrame:`
+ https://bugs.webkit.org/show_bug.cgi?id=234994
+ rdar://86845512
+
+ Reviewed by Chris Dumez and Geoff Garen.
+
+ Add a new API test to exercise the fix.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKitCocoa/CloseWhileCommittingLoad.mm: Added.
+ (-[CloseWhileCommittingLoadDelegate webView:didCommitLoadForFrame:]):
+ (TestWebKitAPI::TEST):
+
2022-01-08 Tyler Wilcock <tyle...@apple.com>
AX: AccessibilityObject::setFocused(true) should make the webpage focused, and make web content the first responder
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (287813 => 287814)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2022-01-08 18:26:13 UTC (rev 287813)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2022-01-08 22:12:16 UTC (rev 287814)
@@ -1029,6 +1029,7 @@
F42F081227449892007E0D90 /* multiple-images.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F42F0811274497C8007E0D90 /* multiple-images.html */; };
F434CA1A22E65BCA005DDB26 /* ScrollToRevealSelection.mm in Sources */ = {isa = PBXBuildFile; fileRef = F434CA1922E65BCA005DDB26 /* ScrollToRevealSelection.mm */; };
F4352F9F26D403DE00E605E4 /* editable-responsive-body.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F4352F9E26D4037000E605E4 /* editable-responsive-body.html */; };
+ F43CAB1D278A326500C8D0A2 /* CloseWhileCommittingLoad.mm in Sources */ = {isa = PBXBuildFile; fileRef = F43CAB1C278A326500C8D0A2 /* CloseWhileCommittingLoad.mm */; };
F43E3BBF20DADA1E00A4E7ED /* WKScrollViewTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F43E3BBE20DADA1E00A4E7ED /* WKScrollViewTests.mm */; };
F43E3BC120DADBC500A4E7ED /* fixed-nav-bar.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F43E3BC020DADB8000A4E7ED /* fixed-nav-bar.html */; };
F442851D2140DF2900CCDA22 /* NSFontPanelTesting.mm in Sources */ = {isa = PBXBuildFile; fileRef = F442851C2140DF2900CCDA22 /* NSFontPanelTesting.mm */; };
@@ -2980,6 +2981,7 @@
F434CA1922E65BCA005DDB26 /* ScrollToRevealSelection.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = ScrollToRevealSelection.mm; sourceTree = "<group>"; };
F4352F9E26D4037000E605E4 /* editable-responsive-body.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "editable-responsive-body.html"; sourceTree = "<group>"; };
F43C3823278133190099ABCE /* NSResponderTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = NSResponderTests.mm; sourceTree = "<group>"; };
+ F43CAB1C278A326500C8D0A2 /* CloseWhileCommittingLoad.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CloseWhileCommittingLoad.mm; sourceTree = "<group>"; };
F43E3BBE20DADA1E00A4E7ED /* WKScrollViewTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = WKScrollViewTests.mm; sourceTree = "<group>"; };
F43E3BC020DADB8000A4E7ED /* fixed-nav-bar.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "fixed-nav-bar.html"; sourceTree = "<group>"; };
F442851B2140DF2900CCDA22 /* NSFontPanelTesting.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = NSFontPanelTesting.h; sourceTree = "<group>"; };
@@ -3079,6 +3081,7 @@
F4D5E4E71F0C5D27008C1A49 /* dragstart-clear-selection.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "dragstart-clear-selection.html"; sourceTree = "<group>"; };
F4D65DA71F5E46C0009D8C27 /* selected-text-image-link-and-editable.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "selected-text-image-link-and-editable.html"; sourceTree = "<group>"; };
F4D9818E2196B911008230FC /* editable-nested-lists.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "editable-nested-lists.html"; sourceTree = "<group>"; };
+ F4DBE5DC278906A300642BC0 /* CloseWhileCommittingLoad.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CloseWhileCommittingLoad.mm; sourceTree = "<group>"; };
F4DEF6EC1E9B4D950048EF61 /* image-in-link-and-input.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "image-in-link-and-input.html"; sourceTree = "<group>"; };
F4E0A28E211E5D5B00AF7C7F /* DragAndDropTestsMac.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = DragAndDropTestsMac.mm; sourceTree = "<group>"; };
F4E0A295211FC5A300AF7C7F /* selected-text-and-textarea.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "selected-text-and-textarea.html"; sourceTree = "<group>"; };
@@ -3395,6 +3398,7 @@
F422A3E8235ACA9B00CF00CA /* ClipboardTests.mm */,
CDF92236216D186400647AA7 /* CloseWebViewAfterEnterFullscreen.mm */,
CDF0B789216D484300421ECC /* CloseWebViewDuringEnterFullscreen.mm */,
+ F4DBE5DC278906A300642BC0 /* CloseWhileCommittingLoad.mm */,
1AAD19F51C7CE20300831E47 /* Coding.mm */,
7C3DB8E21D12129B00AE8CC3 /* CommandBackForward.mm */,
5C4A84941F7EEFD400ACFC54 /* Configuration.mm */,
@@ -4780,6 +4784,7 @@
26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */,
93CFA8681CEBCFED000565A8 /* CandidateTests.mm */,
290A9BB51735DE8A00D71BBC /* CloseNewWindowInNavigationPolicyDelegate.mm */,
+ F43CAB1C278A326500C8D0A2 /* CloseWhileCommittingLoad.mm */,
A1146A8A1D2D704F000FE710 /* ContentFiltering.mm */,
5142B2701517C88B00C32B19 /* ContextMenuCanCopyURL.mm */,
37FB72951DB2E82F00E41BE4 /* ContextMenuDefaultItemsHaveTags.mm */,
@@ -5442,6 +5447,7 @@
7CCE7EE61A411AE600447C4C /* CloseFromWithinCreatePage.cpp in Sources */,
7CCE7EB71A411A7E00447C4C /* CloseNewWindowInNavigationPolicyDelegate.mm in Sources */,
7CCE7EE51A411AE600447C4C /* CloseThenTerminate.cpp in Sources */,
+ F43CAB1D278A326500C8D0A2 /* CloseWhileCommittingLoad.mm in Sources */,
6B306106218A372900F5A802 /* ClosingWebView.mm in Sources */,
7C3965061CDD74F90094DBB8 /* ColorTests.cpp in Sources */,
1C9EB8411E380DA1005C6442 /* ComplexTextController.cpp in Sources */,
Added: trunk/Tools/TestWebKitAPI/Tests/mac/CloseWhileCommittingLoad.mm (0 => 287814)
--- trunk/Tools/TestWebKitAPI/Tests/mac/CloseWhileCommittingLoad.mm (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/mac/CloseWhileCommittingLoad.mm 2022-01-08 22:12:16 UTC (rev 287814)
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2022 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#import "config.h"
+#import "PlatformUtilities.h"
+#import <WebKit/WebFrameLoadDelegate.h>
+#import <WebKit/WebViewPrivate.h>
+#import <wtf/RetainPtr.h>
+
+static bool didCloseWhileCommittingLoad;
+
+@interface CloseWhileCommittingLoadDelegate : NSObject <WebFrameLoadDelegate>
+@end
+
+@implementation CloseWhileCommittingLoadDelegate
+
+- (void)webView:(WebView *)sender didCommitLoadForFrame:(WebFrame *)frame
+{
+ [sender close];
+
+ didCloseWhileCommittingLoad = true;
+}
+
+@end
+
+namespace TestWebKitAPI {
+
+TEST(WebKitLegacy, CloseWhileCommittingLoad)
+{
+ auto webView = adoptNS([WebView new]);
+ auto delegate = adoptNS([CloseWhileCommittingLoadDelegate new]);
+ [webView setFrameLoadDelegate:delegate.get()];
+
+ didCloseWhileCommittingLoad = false;
+ [[webView mainFrame] loadRequest:[NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]]];
+
+ Util::run(&didCloseWhileCommittingLoad);
+}
+
+} // namespace TestWebKitAPI
