Possible crash under NetworkSocketStream::didFailSocketStream()
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Jul 2017 21:45:40 +0000 (21:45 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Jul 2017 21:45:40 +0000 (21:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174526
<rdar://problem/32831441>

Reviewed by Brent Fulgham.

Source/WebCore:

Call m_client.didFailSocketStream() asynchronously in the constructor as our
caller (the client) is also being initialized at this point.

* platform/network/cf/SocketStreamHandleImplCFNet.cpp:
(WebCore::SocketStreamHandleImpl::SocketStreamHandleImpl):

Source/WebKit:

For robustness, initialize the SocketStreamHandleImpl after the other
data members. We are passing |this| to the SocketStreamHandleImpl when
constructing it so it is unsafe to have uninitialized data members
at this point.

* NetworkProcess/NetworkSocketStream.cpp:
(WebKit::NetworkSocketStream::NetworkSocketStream):
* NetworkProcess/NetworkSocketStream.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219525 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/network/cf/SocketStreamHandleImplCFNet.cpp
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkSocketStream.cpp
Source/WebKit/NetworkProcess/NetworkSocketStream.h

index e1dac7c..77a920b 100644 (file)
@@ -1,3 +1,17 @@
+2017-07-14  Chris Dumez  <cdumez@apple.com>
+
+        Possible crash under NetworkSocketStream::didFailSocketStream()
+        https://bugs.webkit.org/show_bug.cgi?id=174526
+        <rdar://problem/32831441>
+
+        Reviewed by Brent Fulgham.
+
+        Call m_client.didFailSocketStream() asynchronously in the constructor as our
+        caller (the client) is also being initialized at this point.
+
+        * platform/network/cf/SocketStreamHandleImplCFNet.cpp:
+        (WebCore::SocketStreamHandleImpl::SocketStreamHandleImpl):
+
 2017-07-14  Youenn Fablet  <youenn@apple.com>
 
         WebRTC: silence data not sent for disabled audio track
index 4176800..a6304b1 100644 (file)
@@ -86,7 +86,10 @@ SocketStreamHandleImpl::SocketStreamHandleImpl(const URL& url, SocketStreamHandl
     if (url.protocolIs("ws")
         && !sessionID.isEphemeral()
         && _CFNetworkIsKnownHSTSHostWithSession(m_httpsURL.get(), nullptr)) {
-        m_client.didFailSocketStream(*this, SocketStreamError(0, m_url.string(), "WebSocket connection failed because it violates HTTP Strict Transport Security."));
+        // Call this asynchronously because the socket stream is not fully constructed at this point.
+        callOnMainThread([this, protectedThis = makeRef(*this)] {
+            m_client.didFailSocketStream(*this, SocketStreamError(0, m_url.string(), "WebSocket connection failed because it violates HTTP Strict Transport Security."));
+        });
         return;
     }
 #endif
index 8ab05ab..925405a 100644 (file)
@@ -1,3 +1,20 @@
+2017-07-14  Chris Dumez  <cdumez@apple.com>
+
+        Possible crash under NetworkSocketStream::didFailSocketStream()
+        https://bugs.webkit.org/show_bug.cgi?id=174526
+        <rdar://problem/32831441>
+
+        Reviewed by Brent Fulgham.
+
+        For robustness, initialize the SocketStreamHandleImpl after the other
+        data members. We are passing |this| to the SocketStreamHandleImpl when
+        constructing it so it is unsafe to have uninitialized data members
+        at this point.
+
+        * NetworkProcess/NetworkSocketStream.cpp:
+        (WebKit::NetworkSocketStream::NetworkSocketStream):
+        * NetworkProcess/NetworkSocketStream.h:
+
 2017-07-14  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [CMake] Unclear distinction between WebKitHelpers and WebKitMacros
index bdede90..33170da 100644 (file)
@@ -41,9 +41,9 @@ Ref<NetworkSocketStream> NetworkSocketStream::create(WebCore::URL&& url, WebCore
 }
 
 NetworkSocketStream::NetworkSocketStream(URL&& url, SessionID sessionID, const String& credentialPartition, uint64_t identifier, IPC::Connection& connection, SourceApplicationAuditToken&& auditData)
-    : m_impl(SocketStreamHandleImpl::create(url, *this, sessionID, credentialPartition, WTFMove(auditData)))
-    , m_identifier(identifier)
+    : m_identifier(identifier)
     , m_connection(connection)
+    , m_impl(SocketStreamHandleImpl::create(url, *this, sessionID, credentialPartition, WTFMove(auditData)))
 {
 }
 
index 1cdfa7b..7dc3d0c 100644 (file)
@@ -67,9 +67,10 @@ private:
     uint64_t messageSenderDestinationID() final;
 
     NetworkSocketStream(WebCore::URL&&, WebCore::SessionID, const String& credentialPartition, uint64_t, IPC::Connection&, WebCore::SourceApplicationAuditToken&&);
-    Ref<WebCore::SocketStreamHandleImpl> m_impl;
+
     uint64_t m_identifier;
     IPC::Connection& m_connection;
+    Ref<WebCore::SocketStreamHandleImpl> m_impl;
 };
 
 } // namespace WebKit