Verify that the notification response contains sensible data

chrome/browser/notifications/notification_platform_bridge_mac.mm

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/notifications/notification_platform_bridge_mac.h"
 
 #include <utility>
 
 #include "base/mac/foundation_util.h"
 #include "base/mac/mac_util.h"
@@ skipping 166 matching lines @@
           objectForKey:notification_constants::kNotificationId]));
     }
   }
   return true;
 }
 
 bool NotificationPlatformBridgeMac::SupportsNotificationCenter() const {
   return true;
 }
 
+// static
+bool NotificationPlatformBridgeMac::VerifyNotificationData(
+    NSDictionary* response) {
+  if (![response
+          objectForKey:notification_constants::kNotificationButtonIndex] ||
+      ![response objectForKey:notification_constants::kNotificationOperation] ||
+      ![response objectForKey:notification_constants::kNotificationId] ||
+      ![response objectForKey:notification_constants::kNotificationProfileId]) {

[Peter Beverloo, 2016/07/01 13:31:43]

+kNotificationIncognito

[Miguel Garcia, 2016/07/04 15:18:56]

Done.

+    LOG(ERROR) << "Missing required key";

[Robert Sesek, 2016/07/01 15:35:57]

Should we ship this log information, or should it be DLOG/VLOG ?

[Miguel Garcia, 2016/07/04 15:18:56]

Yeah I think we should ship it, it's a pretty bad issue if we find these kind of
problems so better ship as much information as possible.

+    return false;
+  }
+
+  NSNumber* buttonIndex =

[Robert Sesek, 2016/07/01 15:35:57]

naming: under_scores since this is in C++

[Miguel Garcia, 2016/07/04 15:18:56]

Done.

+      [response objectForKey:notification_constants::kNotificationButtonIndex];
+  NSNumber* operation =
+      [response objectForKey:notification_constants::kNotificationOperation];
+  NSString* notificationId =
+      [response objectForKey:notification_constants::kNotificationId];
+  NSString* profileId =
+      [response objectForKey:notification_constants::kNotificationProfileId];
+
+  if (buttonIndex.intValue < -1 ||
+      buttonIndex.intValue >=
+          static_cast<int>(blink::kWebNotificationMaxActions)) {
+    LOG(ERROR) << "Invalid number of buttons supplied " << buttonIndex.intValue;
+    return false;
+  }
+
+  if (operation.unsignedIntValue > NotificationCommon::OPERATION_MAX) {
+    LOG(ERROR) << operation.unsignedIntValue
+               << " does not correspond to a valid operation.";
+    return false;
+  }
+
+  if (notificationId.length <= 0) {
+    LOG(ERROR) << "NotificationId not provided";

[Peter Beverloo, 2016/07/01 13:31:43]

"not provided" -> "is empty" perhaps, since we now verify that it's included in
the dictionary? Dito on line 227

[Miguel Garcia, 2016/07/04 15:18:56]

Done.

+    return false;
+  }
+
+  if (profileId.length <= 0) {
+    LOG(ERROR) << "ProfileId not provided";
+    return false;
+  }
+
+  // Origin is not actually required but if it's there it should be a valid one.
+  NSString* origin =
+      [response objectForKey:notification_constants::kNotificationOrigin];
+  if (origin) {
+    std::string notificationOrigin = base::SysNSStringToUTF8(origin);
+    GURL url(notificationOrigin);
+    if (!url.is_valid())
+      return false;
+  }
+
+  return true;
+}
+
 // /////////////////////////////////////////////////////////////////////////////
 
 @implementation NotificationCenterDelegate
 - (void)userNotificationCenter:(NSUserNotificationCenter*)center
        didActivateNotification:(NSUserNotification*)notification {
   NSDictionary* response =
       [NotificationResponseBuilder buildDictionary:notification];
+  if (!NotificationPlatformBridgeMac::VerifyNotificationData(response))
+    return;
 
   NSNumber* buttonIndex =
       [response objectForKey:notification_constants::kNotificationButtonIndex];
   NSNumber* operation =
       [response objectForKey:notification_constants::kNotificationOperation];
 
   std::string notificationOrigin = base::SysNSStringToUTF8(
       [response objectForKey:notification_constants::kNotificationOrigin]);
-  NSString* notificationId = [notification.userInfo
-      objectForKey:notification_constants::kNotificationId];
+  NSString* notificationId =
+      [response objectForKey:notification_constants::kNotificationId];
   std::string persistentNotificationId =
       base::SysNSStringToUTF8(notificationId);
   int64_t persistentId;
   if (!base::StringToInt64(persistentNotificationId, &persistentId)) {
     LOG(ERROR) << "Unable to convert notification ID: "
                << persistentNotificationId << " to integer.";
     return;
   }
   std::string profileId = base::SysNSStringToUTF8(
       [response objectForKey:notification_constants::kNotificationProfileId]);
   NSNumber* isIncognito =
       [response objectForKey:notification_constants::kNotificationIncognito];
 
   GURL origin(notificationOrigin);
 
   PlatformNotificationServiceImpl::GetInstance()
       ->ProcessPersistentNotificationOperation(
           static_cast<NotificationCommon::Operation>(operation.intValue),
           profileId, [isIncognito boolValue], origin, persistentId,
           buttonIndex.intValue);
 }
 
 - (BOOL)userNotificationCenter:(NSUserNotificationCenter*)center
      shouldPresentNotification:(NSUserNotification*)nsNotification {
   // Always display notifications, regardless of whether the app is foreground.
   return YES;
 }
 
 @end