Diff
Modified: branches/safari-608-branch/Source/WebKit/ChangeLog (252053 => 252054)
--- branches/safari-608-branch/Source/WebKit/ChangeLog 2019-11-05 16:40:22 UTC (rev 252053)
+++ branches/safari-608-branch/Source/WebKit/ChangeLog 2019-11-05 16:40:27 UTC (rev 252054)
@@ -1,5 +1,39 @@
2019-11-05 Alan Coon <[email protected]>
+ Apply patch. rdar://problem/56903274
+
+ 2019-11-05 Jiewen Tan <[email protected]>
+
+ [WebAuthn] Guard against unexpected -[_WKWebAuthenticationPanel cancel]
+ https://bugs.webkit.org/show_bug.cgi?id=203830
+ <rdar://problem/56797134>
+
+ Reviewed by Brent Fulgham .
+
+ -[_WKWebAuthenticationPanel cancel] was only expected to be called on behalf of an
+ explicit user cancel from the UI. However, clients may call it in different other
+ unexpected scenarios as well. We should guard against that.
+
+ To do so, two counter ways are implemented:
+ 1) AuthenticatorManager::cancelRequest is changed to invoke the pending request if there
+ is no GlobalFrameID. This case can only be reached via calling -[_WKWebAuthenticationPanel cancel]
+ before AuthenticatorManager::cancelRequest.
+ 2) WebAuthenticationPanelClient::updatePanel and WebAuthenticationPanelClient::dismissPanel
+ will call delegate methods in the next run loop to prevent -[_WKWebAuthenticationPanel cancel]
+ being called in the delegates.
+
+ Covered by new API tests.
+
+ * UIProcess/WebAuthentication/AuthenticatorManager.cpp:
+ (WebKit::AuthenticatorManager::cancelRequest):
+ (WebKit::AuthenticatorManager::createService const):
+ (WebKit::AuthenticatorManager::invokePendingCompletionHandler):
+ * UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm:
+ (WebKit::WebAuthenticationPanelClient::updatePanel const):
+ (WebKit::WebAuthenticationPanelClient::dismissPanel const):
+
+2019-11-05 Alan Coon <[email protected]>
+
Cherry-pick r248121. rdar://problem/56903580
Crash under WebProcessProxy::didBecomeUnresponsive()
Modified: branches/safari-608-branch/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp (252053 => 252054)
--- branches/safari-608-branch/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp 2019-11-05 16:40:22 UTC (rev 252053)
+++ branches/safari-608-branch/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp 2019-11-05 16:40:27 UTC (rev 252054)
@@ -165,15 +165,16 @@
runPanel();
}
-void AuthenticatorManager::cancelRequest(const WebCore::PageIdentifier& pageID, const Optional<FrameIdentifier>& frameID)
+void AuthenticatorManager::cancelRequest(const PageIdentifier& pageID, const Optional<FrameIdentifier>& frameID)
{
if (!m_pendingCompletionHandler)
return;
- ASSERT(m_pendingRequestData.frameID);
- if (m_pendingRequestData.frameID->pageID != pageID)
- return;
- if (frameID && frameID != m_pendingRequestData.frameID->frameID)
- return;
+ if (auto pendingFrameID = m_pendingRequestData.frameID) {
+ if (pendingFrameID->pageID != pageID)
+ return;
+ if (frameID && frameID != pendingFrameID->frameID)
+ return;
+ }
invokePendingCompletionHandler(ExceptionData { NotAllowedError, "Operation timed out."_s });
clearState();
m_requestTimeOutTimer.stop();
@@ -184,6 +185,7 @@
// "If the user exercises a user agent user-interface option to cancel the process,".
void AuthenticatorManager::cancelRequest(const API::WebAuthenticationPanel& panel)
{
+ RELEASE_ASSERT(RunLoop::isMain());
if (!m_pendingCompletionHandler || m_pendingRequestData.panel.get() != &panel)
return;
resetState();
@@ -257,7 +259,7 @@
panel->client().updatePanel(status);
}
-UniqueRef<AuthenticatorTransportService> AuthenticatorManager::createService(WebCore::AuthenticatorTransport transport, AuthenticatorTransportService::Observer& observer) const
+UniqueRef<AuthenticatorTransportService> AuthenticatorManager::createService(AuthenticatorTransport transport, AuthenticatorTransportService::Observer& observer) const
{
return AuthenticatorTransportService::create(transport, observer);
}
@@ -336,9 +338,9 @@
void AuthenticatorManager::invokePendingCompletionHandler(Respond&& respond)
{
if (auto *panel = m_pendingRequestData.panel.get()) {
- WTF::switchOn(respond, [&](const WebCore::PublicKeyCredentialData&) {
+ WTF::switchOn(respond, [&](const PublicKeyCredentialData&) {
panel->client().dismissPanel(WebAuthenticationResult::Succeeded);
- }, [&](const WebCore::ExceptionData&) {
+ }, [&](const ExceptionData&) {
panel->client().dismissPanel(WebAuthenticationResult::Failed);
});
}
@@ -345,7 +347,6 @@
m_pendingCompletionHandler(WTFMove(respond));
}
-
void AuthenticatorManager::resetState()
{
m_authenticators.clear();
Modified: branches/safari-608-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm (252053 => 252054)
--- branches/safari-608-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm 2019-11-05 16:40:22 UTC (rev 252053)
+++ branches/safari-608-branch/Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticationPanelClient.mm 2019-11-05 16:40:27 UTC (rev 252054)
@@ -30,6 +30,7 @@
#import "WebAuthenticationFlags.h"
#import "_WKWebAuthenticationPanel.h"
+#import <wtf/RunLoop.h>
namespace WebKit {
@@ -58,14 +59,21 @@
void WebAuthenticationPanelClient::updatePanel(WebAuthenticationStatus status) const
{
- if (!m_delegateMethods.panelUpdateWebAuthenticationPanel)
- return;
+ // Call delegates in the next run loop to prevent clients' reentrance that would potentially modify the state
+ // of the current run loop in unexpected ways.
+ RunLoop::main().dispatch([weakThis = makeWeakPtr(*this), this, status] {
+ if (!weakThis)
+ return;
- auto delegate = m_delegate.get();
- if (!delegate)
- return;
+ if (!m_delegateMethods.panelUpdateWebAuthenticationPanel)
+ return;
- [delegate panel:m_panel updateWebAuthenticationPanel:wkWebAuthenticationPanelUpdate(status)];
+ auto delegate = m_delegate.get();
+ if (!delegate)
+ return;
+
+ [delegate panel:m_panel updateWebAuthenticationPanel:wkWebAuthenticationPanelUpdate(status)];
+ });
}
static _WKWebAuthenticationResult wkWebAuthenticationResult(WebAuthenticationResult result)
@@ -82,14 +90,21 @@
void WebAuthenticationPanelClient::dismissPanel(WebAuthenticationResult result) const
{
- if (!m_delegateMethods.panelDismissWebAuthenticationPanelWithResult)
- return;
+ // Call delegates in the next run loop to prevent clients' reentrance that would potentially modify the state
+ // of the current run loop in unexpected ways.
+ RunLoop::main().dispatch([weakThis = makeWeakPtr(*this), this, result] {
+ if (!weakThis)
+ return;
- auto delegate = m_delegate.get();
- if (!delegate)
- return;
+ if (!m_delegateMethods.panelDismissWebAuthenticationPanelWithResult)
+ return;
- [delegate panel:m_panel dismissWebAuthenticationPanelWithResult:wkWebAuthenticationResult(result)];
+ auto delegate = m_delegate.get();
+ if (!delegate)
+ return;
+
+ [delegate panel:m_panel dismissWebAuthenticationPanelWithResult:wkWebAuthenticationResult(result)];
+ });
}
} // namespace WebKit
Modified: branches/safari-608-branch/Tools/ChangeLog (252053 => 252054)
--- branches/safari-608-branch/Tools/ChangeLog 2019-11-05 16:40:22 UTC (rev 252053)
+++ branches/safari-608-branch/Tools/ChangeLog 2019-11-05 16:40:27 UTC (rev 252054)
@@ -1,3 +1,20 @@
+2019-11-05 Alan Coon <[email protected]>
+
+ Apply patch. rdar://problem/56903274
+
+ 2019-11-05 Jiewen Tan <[email protected]>
+
+ [WebAuthn] Guard against unexpected -[_WKWebAuthenticationPanel cancel]
+ https://bugs.webkit.org/show_bug.cgi?id=203830
+ <rdar://problem/56797134>
+
+ Reviewed by Brent Fulgham .
+
+ * TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:
+ (-[TestWebAuthenticationPanelDelegate panel:updateWebAuthenticationPanel:]):
+ (-[TestWebAuthenticationPanelDelegate panel:dismissWebAuthenticationPanelWithResult:]):
+ (TestWebKitAPI::TEST):
+
2019-11-04 Alan Coon <[email protected]>
Apply patch. rdar://problem/56864381
Modified: branches/safari-608-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm (252053 => 252054)
--- branches/safari-608-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm 2019-11-05 16:40:22 UTC (rev 252053)
+++ branches/safari-608-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm 2019-11-05 16:40:27 UTC (rev 252054)
@@ -44,6 +44,7 @@
static bool webAuthenticationPanelSucceded = false;
static bool webAuthenticationPanelUpdateMultipleNFCTagsPresent = false;
static bool webAuthenticationPanelUpdateNoCredentialsFound = false;
+static bool webAuthenticationPanelCancelImmediately = false;
static RetainPtr<_WKWebAuthenticationPanel> gPanel;
@interface TestWebAuthenticationPanelDelegate : NSObject <_WKWebAuthenticationPanelDelegate>
@@ -54,6 +55,9 @@
- (void)panel:(_WKWebAuthenticationPanel *)panel updateWebAuthenticationPanel:(_WKWebAuthenticationPanelUpdate)update
{
ASSERT_NE(panel, nil);
+ if (webAuthenticationPanelCancelImmediately)
+ [panel cancel];
+
if (update == _WKWebAuthenticationPanelUpdateMultipleNFCTagsPresent) {
webAuthenticationPanelUpdateMultipleNFCTagsPresent = true;
return;
@@ -67,6 +71,9 @@
- (void)panel:(_WKWebAuthenticationPanel *)panel dismissWebAuthenticationPanelWithResult:(_WKWebAuthenticationResult)result
{
ASSERT_NE(panel, nil);
+ if (webAuthenticationPanelCancelImmediately)
+ [panel cancel];
+
if (result == _WKWebAuthenticationResultFailed) {
webAuthenticationPanelFailed = true;
return;
@@ -186,6 +193,7 @@
webAuthenticationPanelRan = false;
webAuthenticationPanelFailed = false;
webAuthenticationPanelSucceded = false;
+ webAuthenticationPanelCancelImmediately = false;
gPanel = nullptr;
}
@@ -632,6 +640,59 @@
}
#endif
+TEST(WebAuthenticationPanel, PanelHidCancelReloadNoCrash)
+{
+ reset();
+ RetainPtr<NSURL> testURL = [[NSBundle mainBundle] URLForResource:@"web-authentication-get-assertion-hid-cancel" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+
+ auto *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
+ [[configuration preferences] _setEnabled:YES forExperimentalFeature:webAuthenticationExperimentalFeature()];
+
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSZeroRect configuration:configuration]);
+ auto delegate = adoptNS([[TestWebAuthenticationPanelUIDelegate alloc] init]);
+ [webView setUIDelegate:delegate.get()];
+
+ [webView loadRequest:[NSURLRequest requestWithURL:testURL.get()]];
+ Util::run(&webAuthenticationPanelRan);
+ [gPanel cancel];
+ [webView reload];
+ [webView waitForMessage:@"Operation timed out."];
+}
+
+TEST(WebAuthenticationPanel, PanelHidSuccessCancelNoCrash)
+{
+ reset();
+ RetainPtr<NSURL> testURL = [[NSBundle mainBundle] URLForResource:@"web-authentication-get-assertion-hid" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+
+ auto *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
+ [[configuration preferences] _setEnabled:YES forExperimentalFeature:webAuthenticationExperimentalFeature()];
+
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSZeroRect configuration:configuration]);
+ auto delegate = adoptNS([[TestWebAuthenticationPanelUIDelegate alloc] init]);
+ [webView setUIDelegate:delegate.get()];
+ webAuthenticationPanelCancelImmediately = true;
+
+ [webView loadRequest:[NSURLRequest requestWithURL:testURL.get()]];
+ [webView waitForMessage:@"Succeeded!"];
+}
+
+TEST(WebAuthenticationPanel, PanelHidCtapNoCredentialsFoundCancelNoCrash)
+{
+ reset();
+ RetainPtr<NSURL> testURL = [[NSBundle mainBundle] URLForResource:@"web-authentication-get-assertion-hid-no-credentials" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+
+ auto *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
+ [[configuration preferences] _setEnabled:YES forExperimentalFeature:webAuthenticationExperimentalFeature()];
+
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSZeroRect configuration:configuration]);
+ auto delegate = adoptNS([[TestWebAuthenticationPanelUIDelegate alloc] init]);
+ [webView setUIDelegate:delegate.get()];
+ webAuthenticationPanelCancelImmediately = true;
+
+ [webView loadRequest:[NSURLRequest requestWithURL:testURL.get()]];
+ Util::run(&webAuthenticationPanelUpdateNoCredentialsFound);
+}
+
} // namespace TestWebKitAPI
#endif // ENABLE(WEB_AUTHN)
