Changeset 268796 in webkit

Timestamp:

Oct 21, 2020, 9:01:31 AM (6 years ago)

Author:

commit-queue@webkit.org

Message:

Don't crash when deallocating WKWebView during TLS handshake
​https://bugs.webkit.org/show_bug.cgi?id=218025
<rdar://problem/70225969>

Patch by Alex Christensen <​achristensen@webkit.org> on 2020-10-21
Reviewed by Tim Horton.

Source/WebKit:

NetworkProcessProxy::didReceiveAuthenticationChallenge would sometimes dereference an unchecked
Optional<SecurityOriginData> which would result in a null dereference crash. Also, sometimes
Connection::initializeSendSource would assert because it was trying to set up a cancel handler for
a send port that had not been successfully set up yet. I added a test that reproduces both of these
issues most of the time.

• Platform/IPC/cocoa/ConnectionCocoa.mm:

(IPC::Connection::initializeSendSource):

• UIProcess/Network/NetworkProcessProxy.cpp:

(WebKit::NetworkProcessProxy::didReceiveAuthenticationChallenge):

Tools:

• TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:

(TEST):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/Platform/IPC/cocoa/ConnectionCocoa.mm (modified) (1 diff)
• Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-10-21  Alex Christensen  <achristensen@webkit.org>
+
+        Don't crash when deallocating WKWebView during TLS handshake
+        https://bugs.webkit.org/show_bug.cgi?id=218025
+        <rdar://problem/70225969>
+
+        Reviewed by Tim Horton.
+
+        NetworkProcessProxy::didReceiveAuthenticationChallenge would sometimes dereference an unchecked
+        Optional<SecurityOriginData> which would result in a null dereference crash.  Also, sometimes
+        Connection::initializeSendSource would assert because it was trying to set up a cancel handler for
+        a send port that had not been successfully set up yet.  I added a test that reproduces both of these
+        issues most of the time.
+
+        * Platform/IPC/cocoa/ConnectionCocoa.mm:
+        (IPC::Connection::initializeSendSource):
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::didReceiveAuthenticationChallenge):
+
 2020-10-21  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebKit/Platform/IPC/cocoa/ConnectionCocoa.mm

@@ -395 +395 @@
     });
 
-    ASSERT(MACH_PORT_VALID(m_sendPort));
-    mach_port_t sendPort = m_sendPort;
-    dispatch_source_set_cancel_handler(m_sendSource, ^{
-        // Release our send right.
-        deallocateSendRightSafely(sendPort);
-    });
+    if (MACH_PORT_VALID(m_sendPort)) {
+        mach_port_t sendPort = m_sendPort;
+        dispatch_source_set_cancel_handler(m_sendSource, ^{
+            // Release our send right.
+            deallocateSendRightSafely(sendPort);
+        });
+    }
 }
 

trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp

@@ -433 +433 @@
     }
 
+    if (!topOrigin) {
+        authenticationChallenge->listener().completeChallenge(AuthenticationChallengeDisposition::RejectProtectionSpaceAndContinue);
+        return;
+    }
+
     WebPageProxy::forMostVisibleWebPageIfAny(sessionID, *topOrigin, [this, weakThis = makeWeakPtr(this), sessionID, authenticationChallenge = WTFMove(authenticationChallenge), negotiatedLegacyTLS](auto* page) mutable {
         if (!weakThis)

trunk/Tools/ChangeLog

@@ +1 @@
+2020-10-21  Alex Christensen  <achristensen@webkit.org>
+
+        Don't crash when deallocating WKWebView during TLS handshake
+        https://bugs.webkit.org/show_bug.cgi?id=218025
+        <rdar://problem/70225969>
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm:
+        (TEST):
+
 2020-10-20  Sam Weinig  <weinig@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Challenge.mm

@@ -239 +239 @@
 
     Util::run(&navigationFinished);
+}
+
+TEST(Challenge, DeallocateDuringChallenge)
+{
+    using namespace TestWebKitAPI;
+    HTTPServer server({{ "/", { "hi" }}}, HTTPServer::Protocol::Https);
+
+    auto delegate = [[TestNavigationDelegate new] autorelease];
+    delegate.didReceiveAuthenticationChallenge = ^(WKWebView *, NSURLAuthenticationChallenge *challenge, void (^completionHandler)(NSURLSessionAuthChallengeDisposition, NSURLCredential *)) {
+        completionHandler(NSURLSessionAuthChallengeUseCredential, [NSURLCredential credentialForTrust:challenge.protectionSpace.serverTrust]);
+    };
+
+    @autoreleasepool {
+        Vector<RetainPtr<WKWebView>> views;
+        for (size_t i = 0; i < 100; i++)
+            views.append(adoptNS([WKWebView new]));
+        for (auto& view : views) {
+            [view setNavigationDelegate:delegate];
+            [view loadRequest:server.request()];
+        }
+        Util::spinRunLoop(10);
+    }
+    Util::spinRunLoop(1000);
 }