Commit 7b7146be authored by Anita Woodruff's avatar Anita Woodruff Committed by Commit Bot

[Notifications] Check max data on the renderer-side

- The legacy IPC pathway was checking the max data
size *before* the IPC call, and rejecting if it was
exceeded.

- This patch reinstates the check for the new mojo
pathway, so that we don't risk crashing the renderer
(thanks to verification in struct traits) if max data
is exceeded.

- This involved moving the max data constant to mojo
so it can be accessed from both blink and content.

R=peter@chromium.org

Bug: 798742,796991
Change-Id: I14ae782c9265411da1374beb7ae7312cb265b07e
Reviewed-on: https://chromium-review.googlesource.com/1011602Reviewed-by: default avatarDmitry Gozman <dgozman@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551146}
parent 25647eed
......@@ -35,7 +35,7 @@ bool ValidateActions(
bool ValidateData(const std::vector<char>& data) {
return data.size() <=
content::PlatformNotificationData::kMaximumDeveloperDataSize;
blink::mojom::NotificationData::kMaximumDeveloperDataSize;
}
bool ValidateImage(const SkBitmap& image) {
......
......@@ -56,10 +56,6 @@ struct CONTENT_EXPORT PlatformNotificationData {
PlatformNotificationData(const PlatformNotificationData& other);
~PlatformNotificationData();
// The maximum size of developer-provided data to be stored in the |data|
// property of this structure.
static const size_t kMaximumDeveloperDataSize = 1024 * 1024;
enum Direction {
DIRECTION_LEFT_TO_RIGHT,
DIRECTION_RIGHT_TO_LEFT,
......
......@@ -16,6 +16,7 @@
#include "content/renderer/notifications/notification_data_conversions.h"
#include "content/renderer/notifications/notification_dispatcher.h"
#include "content/renderer/service_worker/web_service_worker_registration_impl.h"
#include "third_party/blink/public/platform/modules/notifications/notification.mojom.h"
#include "third_party/blink/public/platform/url_conversion.h"
#include "third_party/blink/public/platform/web_security_origin.h"
#include "url/origin.h"
......@@ -101,7 +102,8 @@ void NotificationManager::ShowPersistent(
UMA_HISTOGRAM_COUNTS_1000("Notifications.AuthorDataSize", author_data_size);
if (author_data_size > PlatformNotificationData::kMaximumDeveloperDataSize) {
if (author_data_size >
blink::mojom::NotificationData::kMaximumDeveloperDataSize) {
callbacks->OnError();
return;
}
......
......@@ -2195,7 +2195,6 @@ crbug.com/738493 http/tests/performance-timing/longtask-v2/longtask-executescrip
# Expected failures during incremental implementation of mojo notification.
crbug.com/796991 virtual/mojo-notifications/http/tests/notifications/serviceworkerregistration-document-close.html [ Skip ]
crbug.com/796991 virtual/mojo-notifications/http/tests/notifications/serviceworkerregistration-document-data-invalid.html [ Skip ]
crbug.com/796991 virtual/mojo-notifications/http/tests/notifications/serviceworkerregistration-get-close.html [ Skip ]
crbug.com/713587 external/wpt/css/css-ui/caret-color-006.html [ Skip ]
......
......@@ -41,6 +41,10 @@ struct NotificationAction {
// Structure representing the information associated with a Web Notification.
struct NotificationData {
// The maximum size of developer-provided data to be stored in the |data|
// property of this structure, in bytes.
const uint64 kMaximumDeveloperDataSize = 1048576; // 1024 * 1024 (= 1 MB)
// Title to be displayed with the Web Notification.
mojo_base.mojom.String16 title;
......@@ -94,8 +98,7 @@ struct NotificationData {
// Developer-provided data associated with the notification, in the form of
// a serialized string. Must not exceed the number of bytes specified by
// |kMaximumDeveloperDataBytes| in PlatformNotificationData.
// TODO(https://crbug.com/798742): Move |kMaximumDeveloperDataBytes| here.
// |kMaximumDeveloperDataSize|.
array<int8>? data;
// Actions that should be shown as buttons on the notification.
......
......@@ -137,10 +137,26 @@ void NotificationManager::DisplayPersistentNotification(
DCHECK_EQ(notification_data.actions.size(),
notification_resources->action_icons.size());
// Verify that the author-provided payload size does not exceed our limit.
// This is an implementation-defined limit to prevent abuse of notification
// data as a storage mechanism. A UMA histogram records the requested sizes,
// which enables us to track how much data authors are attempting to store.
//
// If the size exceeds this limit, reject the showNotification() promise. This
// is outside of the boundaries set by the specification, but it gives authors
// an indication that something has gone wrong.
size_t author_data_size = notification_data.data.size();
DEFINE_THREAD_SAFE_STATIC_LOCAL(
CustomCountHistogram, notification_data_size_histogram,
("Notifications.AuthorDataSize", 1, 1000, 50));
notification_data_size_histogram.Count(notification_data.data.size());
notification_data_size_histogram.Count(author_data_size);
if (author_data_size >
mojom::blink::NotificationData::kMaximumDeveloperDataSize) {
resolver->Reject();
return;
}
GetNotificationService()->DisplayPersistentNotification(
service_worker_registration->RegistrationId(), notification_data,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment