Verify that the notification response contains sensible data

chrome/browser/notifications/notification_platform_bridge_mac.mm

Index: chrome/browser/notifications/notification_platform_bridge_mac.mm
diff --git a/chrome/browser/notifications/notification_platform_bridge_mac.mm b/chrome/browser/notifications/notification_platform_bridge_mac.mm
index 8ceb560269967a3172a769c7e94251788b716b55..5e0f65e8a03882ca0b0b86c43cf0d71e269ec267 100644
--- a/chrome/browser/notifications/notification_platform_bridge_mac.mm
+++ b/chrome/browser/notifications/notification_platform_bridge_mac.mm
@@ -184,6 +184,63 @@ 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
@@ -191,6 +248,8 @@ bool NotificationPlatformBridgeMac::SupportsNotificationCenter() const {
        didActivateNotification:(NSUserNotification*)notification {
   NSDictionary* response =
       [NotificationResponseBuilder buildDictionary:notification];
+  if (!NotificationPlatformBridgeMac::VerifyNotificationData(response))
+    return;
 
   NSNumber* buttonIndex =
       [response objectForKey:notification_constants::kNotificationButtonIndex];
@@ -199,8 +258,8 @@ bool NotificationPlatformBridgeMac::SupportsNotificationCenter() const {
 
   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;