- Revision
- 246939
- Author
- you...@apple.com
- Date
- 2019-06-28 14:53:42 -0700 (Fri, 28 Jun 2019)
Log Message
Protect NetworkProcess::m_swServers from bad session IDs
https://bugs.webkit.org/show_bug.cgi?id=199298
<rdar://problem/51859081>
Reviewed by Chris Dumez.
Protect NetworkProcess from receiving bad session IDs in service worker code path by checking for session ID validity whenever interacting with the map.
One of the check is done in WebProcess in which case, if the session ID is bad, the SW connection to NetworkProcess will not be made.
For bad session IDs, this will in that case trigger timing out of service worker operations.
For get/clear data, exit early in case of bad session ID.
Made some refactoring to remove swOriginStoreForSession method.
In the one call site where it is used, the store should already be created so we reuse existingSWOriginStoreForSession.
Added a bunch of additional ASSERTs.
* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::destroySession):
(WebKit::NetworkProcess::fetchWebsiteData):
(WebKit::NetworkProcess::deleteWebsiteData):
(WebKit::NetworkProcess::deleteWebsiteDataForRegistrableDomains):
(WebKit::NetworkProcess::registrableDomainsWithWebsiteData):
(WebKit::NetworkProcess::actualPrepareToSuspend):
(WebKit::NetworkProcess::swServerForSession):
(WebKit::NetworkProcess::existingSWOriginStoreForSession const):
(WebKit::NetworkProcess::registerSWServerConnection):
* NetworkProcess/NetworkProcess.h:
* WebProcess/Network/NetworkProcessConnection.cpp:
(WebKit::NetworkProcessConnection::initializeSWClientConnection):
* WebProcess/Storage/WebSWClientConnection.cpp:
(WebKit::WebSWClientConnection::WebSWClientConnection):
(WebKit::WebSWClientConnection::initializeConnectionIfNeeded):
(WebKit::WebSWClientConnection::ensureConnectionAndSend):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (246938 => 246939)
--- trunk/Source/WebKit/ChangeLog 2019-06-28 21:39:36 UTC (rev 246938)
+++ trunk/Source/WebKit/ChangeLog 2019-06-28 21:53:42 UTC (rev 246939)
@@ -1,3 +1,40 @@
+2019-06-28 Youenn Fablet <you...@apple.com>
+
+ Protect NetworkProcess::m_swServers from bad session IDs
+ https://bugs.webkit.org/show_bug.cgi?id=199298
+ <rdar://problem/51859081>
+
+ Reviewed by Chris Dumez.
+
+ Protect NetworkProcess from receiving bad session IDs in service worker code path by checking for session ID validity whenever interacting with the map.
+ One of the check is done in WebProcess in which case, if the session ID is bad, the SW connection to NetworkProcess will not be made.
+ For bad session IDs, this will in that case trigger timing out of service worker operations.
+
+ For get/clear data, exit early in case of bad session ID.
+
+ Made some refactoring to remove swOriginStoreForSession method.
+ In the one call site where it is used, the store should already be created so we reuse existingSWOriginStoreForSession.
+
+ Added a bunch of additional ASSERTs.
+
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::destroySession):
+ (WebKit::NetworkProcess::fetchWebsiteData):
+ (WebKit::NetworkProcess::deleteWebsiteData):
+ (WebKit::NetworkProcess::deleteWebsiteDataForRegistrableDomains):
+ (WebKit::NetworkProcess::registrableDomainsWithWebsiteData):
+ (WebKit::NetworkProcess::actualPrepareToSuspend):
+ (WebKit::NetworkProcess::swServerForSession):
+ (WebKit::NetworkProcess::existingSWOriginStoreForSession const):
+ (WebKit::NetworkProcess::registerSWServerConnection):
+ * NetworkProcess/NetworkProcess.h:
+ * WebProcess/Network/NetworkProcessConnection.cpp:
+ (WebKit::NetworkProcessConnection::initializeSWClientConnection):
+ * WebProcess/Storage/WebSWClientConnection.cpp:
+ (WebKit::WebSWClientConnection::WebSWClientConnection):
+ (WebKit::WebSWClientConnection::initializeConnectionIfNeeded):
+ (WebKit::WebSWClientConnection::ensureConnectionAndSend):
+
2019-06-28 Timothy Hatcher <timo...@apple.com>
Rename effectiveAppearanceIsInactive and useInactiveAppearance to better match UIUserInterfaceLevel.
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp (246938 => 246939)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2019-06-28 21:39:36 UTC (rev 246938)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2019-06-28 21:53:42 UTC (rev 246939)
@@ -577,6 +577,10 @@
void NetworkProcess::destroySession(const PAL::SessionID& sessionID)
{
+ ASSERT(sessionID.isValid());
+ if (!sessionID.isValid())
+ return;
+
ASSERT(sessionID != PAL::SessionID::defaultSessionID());
if (auto session = m_networkSessions.take(sessionID))
@@ -1250,6 +1254,10 @@
void NetworkProcess::fetchWebsiteData(PAL::SessionID sessionID, OptionSet<WebsiteDataType> websiteDataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, uint64_t callbackID)
{
+ ASSERT(sessionID.isValid());
+ if (!sessionID.isValid())
+ return;
+
struct CallbackAggregator final : public ThreadSafeRefCounted<CallbackAggregator> {
explicit CallbackAggregator(Function<void (WebsiteData)>&& completionHandler)
: m_completionHandler(WTFMove(completionHandler))
@@ -1344,6 +1352,10 @@
void NetworkProcess::deleteWebsiteData(PAL::SessionID sessionID, OptionSet<WebsiteDataType> websiteDataTypes, WallTime modifiedSince, uint64_t callbackID)
{
+ ASSERT(sessionID.isValid());
+ if (!sessionID.isValid())
+ return;
+
#if PLATFORM(COCOA)
if (websiteDataTypes.contains(WebsiteDataType::HSTSCache)) {
if (auto* networkStorageSession = storageSession(sessionID))
@@ -1551,6 +1563,10 @@
void NetworkProcess::deleteWebsiteDataForRegistrableDomains(PAL::SessionID sessionID, OptionSet<WebsiteDataType> websiteDataTypes, HashMap<RegistrableDomain, WebsiteDataToRemove>&& domains, bool shouldNotifyPage, CompletionHandler<void(const HashSet<RegistrableDomain>&)>&& completionHandler)
{
+ ASSERT(sessionID.isValid());
+ if (!sessionID.isValid())
+ return;
+
OptionSet<WebsiteDataFetchOption> fetchOptions = WebsiteDataFetchOption::DoNotCreateProcesses;
struct CallbackAggregator final : public ThreadSafeRefCounted<CallbackAggregator> {
@@ -1749,6 +1765,10 @@
void NetworkProcess::registrableDomainsWithWebsiteData(PAL::SessionID sessionID, OptionSet<WebsiteDataType> websiteDataTypes, bool shouldNotifyPage, CompletionHandler<void(HashSet<RegistrableDomain>&&)>&& completionHandler)
{
+ ASSERT(sessionID.isValid());
+ if (!sessionID.isValid())
+ return;
+
OptionSet<WebsiteDataFetchOption> fetchOptions = WebsiteDataFetchOption::DoNotCreateProcesses;
struct CallbackAggregator final : public ThreadSafeRefCounted<CallbackAggregator> {
@@ -2035,8 +2055,10 @@
connection->cleanupForSuspension([delayedTaskCounter] { });
#if ENABLE(SERVICE_WORKER)
- for (auto& server : m_swServers.values())
+ for (auto& server : m_swServers.values()) {
+ ASSERT(m_swServers.get(server->sessionID()) == server.get());
server->startSuspension([delayedTaskCounter] { });
+ }
#endif
for (auto& session : m_networkSessions)
@@ -2367,7 +2389,7 @@
SWServer& NetworkProcess::swServerForSession(PAL::SessionID sessionID)
{
ASSERT(sessionID.isValid());
-
+
auto result = m_swServers.ensure(sessionID, [&] {
auto path = m_swDatabasePaths.get(sessionID);
// There should already be a registered path for this PAL::SessionID.
@@ -2383,13 +2405,12 @@
return *result.iterator->value;
}
-WebSWOriginStore& NetworkProcess::swOriginStoreForSession(PAL::SessionID sessionID)
+WebSWOriginStore* NetworkProcess::existingSWOriginStoreForSession(PAL::SessionID sessionID) const
{
- return static_cast<WebSWOriginStore&>(swServerForSession(sessionID).originStore());
-}
+ ASSERT(sessionID.isValid());
+ if (!sessionID.isValid())
+ return nullptr;
-WebSWOriginStore* NetworkProcess::existingSWOriginStoreForSession(PAL::SessionID sessionID) const
-{
auto* swServer = m_swServers.get(sessionID);
if (!swServer)
return nullptr;
@@ -2430,7 +2451,10 @@
ASSERT(parentProcessHasServiceWorkerEntitlement());
ASSERT(!m_swServerConnections.contains(connection.identifier()));
m_swServerConnections.add(connection.identifier(), &connection);
- swOriginStoreForSession(connection.sessionID()).registerSWServerConnection(connection);
+ auto* store = existingSWOriginStoreForSession(connection.sessionID());
+ ASSERT(store);
+ if (store)
+ store->registerSWServerConnection(connection);
}
void NetworkProcess::unregisterSWServerConnection(WebSWServerConnection& connection)
Modified: trunk/Source/WebKit/NetworkProcess/NetworkProcess.h (246938 => 246939)
--- trunk/Source/WebKit/NetworkProcess/NetworkProcess.h 2019-06-28 21:39:36 UTC (rev 246938)
+++ trunk/Source/WebKit/NetworkProcess/NetworkProcess.h 2019-06-28 21:53:42 UTC (rev 246939)
@@ -463,7 +463,6 @@
void disableServiceWorkerProcessTerminationDelay();
- WebSWOriginStore& swOriginStoreForSession(PAL::SessionID);
WebSWOriginStore* existingSWOriginStoreForSession(PAL::SessionID) const;
bool needsServerToContextConnectionForRegistrableDomain(const WebCore::RegistrableDomain&) const;
Modified: trunk/Source/WebKit/WebProcess/Network/NetworkProcessConnection.cpp (246938 => 246939)
--- trunk/Source/WebKit/WebProcess/Network/NetworkProcessConnection.cpp 2019-06-28 21:39:36 UTC (rev 246938)
+++ trunk/Source/WebKit/WebProcess/Network/NetworkProcessConnection.cpp 2019-06-28 21:53:42 UTC (rev 246939)
@@ -279,6 +279,7 @@
SWServerConnectionIdentifier NetworkProcessConnection::initializeSWClientConnection(WebSWClientConnection& connection)
{
+ ASSERT(connection.sessionID().isValid());
SWServerConnectionIdentifier identifier;
bool result = m_connection->sendSync(Messages::NetworkConnectionToWebProcess::EstablishSWServerConnection(connection.sessionID()), Messages::NetworkConnectionToWebProcess::EstablishSWServerConnection::Reply(identifier), 0);
ASSERT_UNUSED(result, result);
Modified: trunk/Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp (246938 => 246939)
--- trunk/Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp 2019-06-28 21:39:36 UTC (rev 246938)
+++ trunk/Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp 2019-06-28 21:53:42 UTC (rev 246939)
@@ -56,7 +56,6 @@
: m_sessionID(sessionID)
, m_swOriginTable(makeUniqueRef<WebSWOriginTable>())
{
- ASSERT(sessionID.isValid());
initializeConnectionIfNeeded();
}
@@ -68,6 +67,10 @@
void WebSWClientConnection::initializeConnectionIfNeeded()
{
+ ASSERT(m_sessionID.isValid());
+ if (!m_sessionID.isValid())
+ return;
+
if (m_connection)
return;
@@ -83,7 +86,8 @@
void WebSWClientConnection::ensureConnectionAndSend(const U& message)
{
initializeConnectionIfNeeded();
- send(message);
+ if (m_connection)
+ send(message);
}
void WebSWClientConnection::scheduleJobInServer(const ServiceWorkerJobData& jobData)
