Commit a31644c6 authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Extensions] Enforce a maximum message size limit

Enforce a maximum message limit for extensions of a 64MB string (from
the JSON-ified object passed to the API call). We enforce a 128 MB limit
on IPC messages, but even that should be excessively large for our needs.
Also add metrics to see what average messages look like - 64MB is *huge*
(and we have to serialize/deserialize the object multiple times, so
message size can have a pretty big performance impact), so we may be
able to reduce this furter.

Add a test for the new behavior.

Bug: 766713

Change-Id: Ic72b2c6c3ea3ca68a42fcd8061c9e006bc6f1795
Reviewed-on: https://chromium-review.googlesource.com/688674Reviewed-by: default avatarIlya Sherman <isherman@chromium.org>
Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505176}
parent faea21a6
......@@ -1326,6 +1326,12 @@ IN_PROC_BROWSER_TEST_F(MessagingApiTest, MessagingOnUnload) {
EXPECT_EQ(1, message_count);
}
// Tests that messages over a certain size are not sent.
// https://crbug.com/766713.
IN_PROC_BROWSER_TEST_F(MessagingApiTest, LargeMessages) {
ASSERT_TRUE(RunExtensionTest("messaging/large_messages"));
}
} // namespace
}; // namespace extensions
// Copyright 2017 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.
try {
// Passing super-large messages should be prevented by the renderer.
var tooLarge = 1024 * 1024 * 128;
chrome.runtime.sendMessage('a'.repeat(tooLarge));
chrome.test.notifyFail();
} catch (e) {
chrome.test.assertEq("Message length exceeded maximum allowed length.",
e.message);
chrome.test.notifyPass();
}
{
"name": "Pass large messages",
"description": "Pass obscenely large messages",
"background": {"scripts": ["background.js"]},
"manifest_version": 2,
"version": "0.1"
}
......@@ -27,6 +27,7 @@
#include "extensions/renderer/script_context.h"
#include "extensions/renderer/script_context_set.h"
#include "extensions/renderer/v8_helpers.h"
#include "gin/converter.h"
#include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
#include "v8/include/v8.h"
......@@ -109,11 +110,36 @@ void MessagingBindings::PostMessage(
int js_port_id = args[0].As<v8::Int32>()->Value();
auto iter = ports_.find(js_port_id);
if (iter != ports_.end()) {
iter->second->PostExtensionMessage(std::make_unique<Message>(
*v8::String::Utf8Value(args[1]),
blink::WebUserGestureIndicator::IsProcessingUserGesture()));
if (iter == ports_.end())
return;
ExtensionPort& port = *iter->second;
auto message = std::make_unique<Message>(
*v8::String::Utf8Value(args[1]),
blink::WebUserGestureIndicator::IsProcessingUserGesture());
size_t message_length = message->data.length();
// Max bucket at 512 MB - anything over that, and we don't care.
static constexpr int kMaxUmaLength = 1024 * 1024 * 512;
static constexpr int kMinUmaLength = 1;
static constexpr int kBucketCount = 50;
UMA_HISTOGRAM_CUSTOM_COUNTS("Extensions.Messaging.MessageSize",
message_length, kMinUmaLength, kMaxUmaLength,
kBucketCount);
// IPC messages will fail at > 128 MB. Restrict extension messages to 64 MB.
// A 64 MB JSON-ifiable object is scary enough as is.
static constexpr size_t kMaxMessageLength = 1024 * 1024 * 64;
if (message_length > kMaxMessageLength) {
args.GetReturnValue().Set(gin::StringToV8(
args.GetIsolate(), "Message length exceeded maximum allowed length."));
return;
}
port.PostExtensionMessage(std::move(message));
}
void MessagingBindings::CloseChannel(
......
......@@ -35,7 +35,8 @@ class MessagingBindings : public ObjectBackedNativeHandler {
private:
using PortMap = std::map<int, std::unique_ptr<ExtensionPort>>;
// JS Exposed Function: Sends a message along the given channel.
// JS Exposed Function: Sends a message along the given channel. If an error
// occurs, returns the error, else returns nothing.
void PostMessage(const v8::FunctionCallbackInfo<v8::Value>& args);
// JS Exposed Function: Close a port, optionally forcefully (i.e. close the
......
......@@ -116,7 +116,9 @@
console.error('Illegal argument to Port.postMessage');
return;
}
messagingNatives.PostMessage(this.portId_, msg);
var error = messagingNatives.PostMessage(this.portId_, msg);
if (error)
throw new Error(error);
};
// Disconnects the port from the other end.
......
......@@ -22619,6 +22619,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>
<histogram name="Extensions.Messaging.MessageSize" units="bytes">
<owner>redevlin.cronin@chromium.org</owner>
<summary>
The size, in bytes, of a message sent from an extension using one of the
messaging APIs (e.g. chrome.runtime.sendMessage). All message sizes are
logged, but messages over 64 MB in size aren't sent.
</summary>
</histogram>
<histogram name="Extensions.Messaging.SetPortIdTime" units="ms">
<owner>rdevlin.cronin@chromium.org</owner>
<summary>
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