[webkit-changes] [292152] trunk/Source/WebKit

Thu, 31 Mar 2022 04:18:18 -0700

Title: [292152] trunk/Source/WebKit
Revision
292152
Author
n...@apple.com
Date
2022-03-31 04:17:14 -0700 (Thu, 31 Mar 2022)

Log Message

Fix null string crashes in PushService
https://bugs.webkit.org/show_bug.cgi?id=238585

Reviewed by Darin Adler.

We've seen a few webpushd crashes due to some PushService routines being passed null string
arguments. Guard against this with isEmpty checks.

This also shouldn't be possible--we shouldn't be sending these requests to webpushd in the
first place. Add some logs to help us catch what's going on.

* webpushd/PushService.mm:
(WebPushD::PushService::getSubscription):
(WebPushD::PushService::subscribe):
(WebPushD::PushService::unsubscribe):
(WebPushD::PushService::incrementSilentPushCount):
(WebPushD::PushService::removeRecordsImpl):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (292151 => 292152)


--- trunk/Source/WebKit/ChangeLog	2022-03-31 10:36:53 UTC (rev 292151)
+++ trunk/Source/WebKit/ChangeLog	2022-03-31 11:17:14 UTC (rev 292152)
@@ -1,3 +1,23 @@
+2022-03-31  Ben Nham  <n...@apple.com>
+
+        Fix null string crashes in PushService
+        https://bugs.webkit.org/show_bug.cgi?id=238585
+
+        Reviewed by Darin Adler.
+
+        We've seen a few webpushd crashes due to some PushService routines being passed null string
+        arguments. Guard against this with isEmpty checks.
+
+        This also shouldn't be possible--we shouldn't be sending these requests to webpushd in the
+        first place. Add some logs to help us catch what's going on.
+
+        * webpushd/PushService.mm:
+        (WebPushD::PushService::getSubscription):
+        (WebPushD::PushService::subscribe):
+        (WebPushD::PushService::unsubscribe):
+        (WebPushD::PushService::incrementSilentPushCount):
+        (WebPushD::PushService::removeRecordsImpl):
+
 2022-03-31  Diego Pino Garcia  <dp...@igalia.com>
 
         [WPE] Unreviewed, fix non-unified build after r290343

Modified: trunk/Source/WebKit/webpushd/PushService.mm (292151 => 292152)


--- trunk/Source/WebKit/webpushd/PushService.mm	2022-03-31 10:36:53 UTC (rev 292151)
+++ trunk/Source/WebKit/webpushd/PushService.mm	2022-03-31 11:17:14 UTC (rev 292152)
@@ -438,6 +438,12 @@
 
 void PushService::getSubscription(const String& bundleIdentifier, const String& scope, CompletionHandler<void(const Expected<std::optional<WebCore::PushSubscriptionData>, WebCore::ExceptionData>&)>&& completionHandler)
 {
+    if (bundleIdentifier.isEmpty() || scope.isEmpty()) {
+        RELEASE_LOG_ERROR(Push, "Ignoring getSubscription request with bundleIdentifier (empty = %d) and scope (empty = %d)", bundleIdentifier.isEmpty(), scope.isEmpty());
+        completionHandler(makeUnexpected(WebCore::ExceptionData { WebCore::AbortError, "Invalid sender"_s }));
+        return;
+    }
+
     enqueuePushServiceRequest(m_getSubscriptionRequests, makeUnique<GetSubscriptionRequest>(*this, bundleIdentifier, scope, WTFMove(completionHandler)));
 }
 
@@ -448,6 +454,12 @@
 
 void PushService::subscribe(const String& bundleIdentifier, const String& scope, const Vector<uint8_t>& vapidPublicKey, CompletionHandler<void(const Expected<WebCore::PushSubscriptionData, WebCore::ExceptionData>&)>&& completionHandler)
 {
+    if (bundleIdentifier.isEmpty() || scope.isEmpty()) {
+        RELEASE_LOG_ERROR(Push, "Ignoring subscribe request with bundleIdentifier (empty = %d) and scope (empty = %d)", bundleIdentifier.isEmpty(), scope.isEmpty());
+        completionHandler(makeUnexpected(WebCore::ExceptionData { WebCore::AbortError, "Invalid sender"_s }));
+        return;
+    }
+
     enqueuePushServiceRequest(m_subscribeRequests, makeUnique<SubscribeRequest>(*this, bundleIdentifier, scope, vapidPublicKey, WTFMove(completionHandler)));
 }
 
@@ -458,6 +470,12 @@
 
 void PushService::unsubscribe(const String& bundleIdentifier, const String& scope, std::optional<PushSubscriptionIdentifier> subscriptionIdentifier, CompletionHandler<void(const Expected<bool, WebCore::ExceptionData>&)>&& completionHandler)
 {
+    if (bundleIdentifier.isEmpty() || scope.isEmpty()) {
+        RELEASE_LOG_ERROR(Push, "Ignoring unsubscribe request with bundleIdentifier (empty = %d) and scope (empty = %d)", bundleIdentifier.isEmpty(), scope.isEmpty());
+        completionHandler(makeUnexpected(WebCore::ExceptionData { WebCore::AbortError, "Invalid sender"_s }));
+        return;
+    }
+
     enqueuePushServiceRequest(m_unsubscribeRequests, makeUnique<UnsubscribeRequest>(*this, bundleIdentifier, scope, subscriptionIdentifier, WTFMove(completionHandler)));
 }
 
@@ -468,6 +486,12 @@
 
 void PushService::incrementSilentPushCount(const String& bundleIdentifier, const String& securityOrigin, CompletionHandler<void(unsigned)>&& handler)
 {
+    if (bundleIdentifier.isEmpty() || securityOrigin.isEmpty()) {
+        RELEASE_LOG_ERROR(Push, "Ignoring removeRecordsImpl request with bundleIdentifier (empty = %d) and securityOrigin (empty = %d)", bundleIdentifier.isEmpty(), securityOrigin.isEmpty());
+        handler(0);
+        return;
+    }
+
     m_database->incrementSilentPushCount(bundleIdentifier, securityOrigin, [this, bundleIdentifier, securityOrigin, handler = WTFMove(handler)](unsigned silentPushCount) mutable {
         if (silentPushCount < WebKit::WebPushD::maxSilentPushCount) {
             handler(silentPushCount);
@@ -494,6 +518,12 @@
 
 void PushService::removeRecordsImpl(const String& bundleIdentifier, const std::optional<String>& securityOrigin, CompletionHandler<void(unsigned)>&& handler)
 {
+    if (bundleIdentifier.isEmpty() || (securityOrigin && securityOrigin->isEmpty())) {
+        RELEASE_LOG_ERROR(Push, "Ignoring removeRecordsImpl request with bundleIdentifier (empty = %d) and securityOrigin (empty = %d)", bundleIdentifier.isEmpty(), securityOrigin && securityOrigin->isEmpty());
+        handler(0);
+        return;
+    }
+
     auto removedRecordsHandler = [this, bundleIdentifier, securityOrigin, handler = WTFMove(handler)](Vector<RemovedPushRecord>&& removedRecords) mutable {
         for (auto& record : removedRecords) {
             m_connection->unsubscribe(record.topic, record.serverVAPIDPublicKey, [topic = record.topic](bool unsubscribed, NSError* error) {

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to