Title: [263570] trunk/Source/WebKit
Revision
263570
Author
cdu...@apple.com
Date
2020-06-26 10:37:25 -0700 (Fri, 26 Jun 2020)

Log Message

[iOS] Network process is crashing when launching TJMaxx app due to invalid NetworkProcess::DestroySession IPC message
https://bugs.webkit.org/show_bug.cgi?id=213625
<rdar://problem/64737890>

Reviewed by Alex Christensen.

The app is calling [WKWebsiteDataStore init] despite the method being marked as unavailable in
WKWebsiteDataStore.h. As a result, they end up with a WKWebsiteDataStore object whose internal
_websiteDataStore is bad because its constructor was never called. When [WKWebsiteDataStore dealloc]
gets called later own, it calls the ~WebsiteDataStore() destructor for _websiteDataStore but its
m_sessionID is 0 because we never called the constructor. This causes us to send a
NetworkProcess::DestroySession IPC with a sessionID that is 0, which is not valid so the
NetworkProcess crashes.

To address the issue, we now provide an implementation of [WKWebsiteDataStore init] which raises an
exception, behind a linked-on-after check. To keep the app working, [WKWebsiteDataStore init] returns
a new ephemeral data store until rebuilt with the new SDK.

* UIProcess/API/Cocoa/WKWebsiteDataStore.h:
Mark "new" as unavailable, otherwise [WKWebsiteDataStore new] builds.

* UIProcess/API/Cocoa/WKWebsiteDataStore.mm:
(-[WKWebsiteDataStore init]):
Raise an exception with latest SDK, a new ephemeral data store otherwise.

* UIProcess/Cocoa/VersionChecks.h:
Add linked-on-after check.

* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::~WebsiteDataStore):
Add a release assertion to make sure that m_sessionID is always valid when the destructor is called.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (263569 => 263570)


--- trunk/Source/WebKit/ChangeLog	2020-06-26 17:18:41 UTC (rev 263569)
+++ trunk/Source/WebKit/ChangeLog	2020-06-26 17:37:25 UTC (rev 263570)
@@ -1,3 +1,37 @@
+2020-06-26  Chris Dumez  <cdu...@apple.com>
+
+        [iOS] Network process is crashing when launching TJMaxx app due to invalid NetworkProcess::DestroySession IPC message
+        https://bugs.webkit.org/show_bug.cgi?id=213625
+        <rdar://problem/64737890>
+
+        Reviewed by Alex Christensen.
+
+        The app is calling [WKWebsiteDataStore init] despite the method being marked as unavailable in
+        WKWebsiteDataStore.h. As a result, they end up with a WKWebsiteDataStore object whose internal
+        _websiteDataStore is bad because its constructor was never called. When [WKWebsiteDataStore dealloc]
+        gets called later own, it calls the ~WebsiteDataStore() destructor for _websiteDataStore but its
+        m_sessionID is 0 because we never called the constructor. This causes us to send a
+        NetworkProcess::DestroySession IPC with a sessionID that is 0, which is not valid so the
+        NetworkProcess crashes.
+
+        To address the issue, we now provide an implementation of [WKWebsiteDataStore init] which raises an
+        exception, behind a linked-on-after check. To keep the app working, [WKWebsiteDataStore init] returns
+        a new ephemeral data store until rebuilt with the new SDK.
+
+        * UIProcess/API/Cocoa/WKWebsiteDataStore.h:
+        Mark "new" as unavailable, otherwise [WKWebsiteDataStore new] builds.
+
+        * UIProcess/API/Cocoa/WKWebsiteDataStore.mm:
+        (-[WKWebsiteDataStore init]):
+        Raise an exception with latest SDK, a new ephemeral data store otherwise.
+
+        * UIProcess/Cocoa/VersionChecks.h:
+        Add linked-on-after check.
+
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::~WebsiteDataStore):
+        Add a release assertion to make sure that m_sessionID is always valid when the destructor is called.
+
 2020-06-26  Stephan Szabo  <stephan.sz...@sony.com>
 
         [WinCairo] Cannot build without resource load statistics

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.h (263569 => 263570)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.h	2020-06-26 17:18:41 UTC (rev 263569)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.h	2020-06-26 17:37:25 UTC (rev 263570)
@@ -47,6 +47,7 @@
 */
 + (WKWebsiteDataStore *)nonPersistentDataStore;
 
+- (instancetype)new NS_UNAVAILABLE;
 - (instancetype)init NS_UNAVAILABLE;
 
 /*! @abstract Whether the data store is persistent or not. */

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm (263569 => 263570)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm	2020-06-26 17:18:41 UTC (rev 263569)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm	2020-06-26 17:37:25 UTC (rev 263570)
@@ -30,6 +30,7 @@
 #import "AuthenticationChallengeDispositionCocoa.h"
 #import "CompletionHandlerCallChecker.h"
 #import "ShouldGrandfatherStatistics.h"
+#import "VersionChecks.h"
 #import "WKHTTPCookieStoreInternal.h"
 #import "WKNSArray.h"
 #import "WKNSURLAuthenticationChallenge.h"
@@ -115,6 +116,20 @@
     return wrapper(WebKit::WebsiteDataStore::createNonPersistent());
 }
 
+- (instancetype)init
+{
+    if (WebKit::linkedOnOrAfter(WebKit::SDKVersion::FirstWithWKWebsiteDataStoreInitReturningNil))
+        [NSException raise:NSGenericException format:@"Calling [WKWebsiteDataStore init] is not supported."];
+    
+    if (!(self = [super init]))
+        return nil;
+
+    RELEASE_LOG_ERROR(Storage, "Application is calling [WKWebsiteDataStore init], which is not supported");
+    API::Object::constructInWrapper<WebKit::WebsiteDataStore>(self, WebKit::WebsiteDataStoreConfiguration::create(WebKit::IsPersistent::No), PAL::SessionID::generateEphemeralSessionID());
+
+    return self;
+}
+
 - (void)dealloc
 {
     _websiteDataStore->WebKit::WebsiteDataStore::~WebsiteDataStore();

Modified: trunk/Source/WebKit/UIProcess/Cocoa/VersionChecks.h (263569 => 263570)


--- trunk/Source/WebKit/UIProcess/Cocoa/VersionChecks.h	2020-06-26 17:18:41 UTC (rev 263569)
+++ trunk/Source/WebKit/UIProcess/Cocoa/VersionChecks.h	2020-06-26 17:37:25 UTC (rev 263570)
@@ -94,6 +94,7 @@
     FirstWithSessionCleanupByDefault = DYLD_IOS_VERSION_FIRST_WITH_SESSION_CLEANUP_BY_DEFAULT,
     FirstThatSendsNativeMouseEvents = DYLD_IOS_VERSION_13_4,
     FirstWithInitializeWebKit2MainThreadAssertion = DYLD_IOS_VERSION_14_0,
+    FirstWithWKWebsiteDataStoreInitReturningNil = DYLD_IOS_VERSION_14_0,
 #elif PLATFORM(MAC)
     FirstWithNetworkCache = DYLD_MACOSX_VERSION_10_11,
     FirstWithExceptionsForDuplicateCompletionHandlerCalls = DYLD_MACOSX_VERSION_10_13,
@@ -107,6 +108,7 @@
     FirstThatRestrictsBaseURLSchemes = DYLD_MACOSX_VERSION_10_15_4,
     FirstWithSessionCleanupByDefault = DYLD_MACOS_VERSION_FIRST_WITH_SESSION_CLEANUP_BY_DEFAULT,
     FirstWithInitializeWebKit2MainThreadAssertion = DYLD_MACOSX_VERSION_10_16,
+    FirstWithWKWebsiteDataStoreInitReturningNil = DYLD_MACOSX_VERSION_10_16,
 #endif
 };
 

Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (263569 => 263570)


--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2020-06-26 17:18:41 UTC (rev 263569)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp	2020-06-26 17:37:25 UTC (rev 263570)
@@ -124,6 +124,7 @@
 WebsiteDataStore::~WebsiteDataStore()
 {
     ASSERT(RunLoop::isMain());
+    RELEASE_ASSERT(m_sessionID.isValid());
 
     platformDestroy();
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to