Skip to content

Commit

Permalink
[Extensions] Do not load scripts that exceed a maximum size
Browse files Browse the repository at this point in the history
This CL enforces maximum sizes for individual content scripts and all
scripts in an extension. The maximum sizes are:
 - 500 MB per script
 - 1 GB for all content scripts (manifest and dynamic) for a single
   extension

Any file in a script entry that would exceed one of these limits will
not be loaded and will generate an install warning for the extension
if it's a manifest script.

For dynamic scripts, the registerContentScripts function should return
an error and be a no-op if a limit is exceeded. This will be tackled in
a follow up.

Bug: 1379187
Change-Id: I2de4f123f85783350b10415c5cee022355f47f1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4095461
Commit-Queue: Kelvin Jiang <kelvinjiang@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084757}
  • Loading branch information
Celsius273 authored and Chromium LUCI CQ committed Dec 18, 2022
1 parent a793cc9 commit d17176a
Show file tree
Hide file tree
Showing 14 changed files with 315 additions and 21 deletions.
5 changes: 3 additions & 2 deletions chrome/browser/extensions/api/scripting/scripting_api.cc
Expand Up @@ -501,8 +501,9 @@ ValidateContentScriptsResult ValidateParsedScriptsOnFileThread(
// Validate that claimed script resources actually exist, and are UTF-8
// encoded.
std::string error;
bool are_script_files_valid =
script_parsing::ValidateFileSources(*scripts, symlink_policy, &error);
std::vector<InstallWarning> warnings;
bool are_script_files_valid = script_parsing::ValidateFileSources(
*scripts, symlink_policy, &error, &warnings);

return std::make_pair(std::move(scripts), are_script_files_valid
? absl::nullopt
Expand Down
34 changes: 34 additions & 0 deletions chrome/browser/extensions/content_script_apitest.cc
Expand Up @@ -45,9 +45,12 @@
#include "extensions/browser/browsertest_util.h"
#include "extensions/browser/content_script_tracker.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/common/api/content_scripts.h"
#include "extensions/common/extension.h"
#include "extensions/common/extension_features.h"
#include "extensions/common/identifiability_metrics.h"
#include "extensions/common/utils/content_script_utils.h"
#include "extensions/strings/grit/extensions_strings.h"
#include "extensions/test/extension_test_message_listener.h"
#include "extensions/test/result_catcher.h"
#include "extensions/test/test_extension_dir.h"
Expand All @@ -58,6 +61,7 @@
#include "services/network/public/cpp/features.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/common/privacy_budget/identifiable_surface.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/page_transition_types.h"
#include "url/gurl.h"

Expand Down Expand Up @@ -406,6 +410,36 @@ IN_PROC_BROWSER_TEST_F(ContentScriptApiTest, FetchExemptFromCSP) {
EXPECT_EQ("Failed to fetch", listener.message());
}

// Test that content scripts that exceed the individual script size limit or the
// total extensions script limit will not be loaded/injected, and will generate
// an install warning.
IN_PROC_BROWSER_TEST_F(ContentScriptApiTest, LargeScriptFilesNotLoaded) {
auto single_scripts_limit_reset =
script_parsing::CreateScopedMaxScriptLengthForTesting(800u);
auto extension_scripts_limit_reset =
script_parsing::CreateScopedMaxScriptsLengthPerExtensionForTesting(1000u);
ASSERT_TRUE(StartEmbeddedTestServer());

ResultCatcher result_catcher;
const Extension* extension =
LoadExtension(test_data_dir_.AppendASCII("content_scripts/large_scripts"),
{.ignore_manifest_warnings = true});
ASSERT_TRUE(extension);
EXPECT_TRUE(result_catcher.GetNextResult()) << result_catcher.message();

std::vector<InstallWarning> expected_warnings;
expected_warnings.emplace_back(
l10n_util::GetStringFUTF8(IDS_EXTENSION_CONTENT_SCRIPT_FILE_TOO_LARGE,
u"big.js"),
api::content_scripts::ManifestKeys::kContentScripts);
expected_warnings.emplace_back(
l10n_util::GetStringFUTF8(IDS_EXTENSION_CONTENT_SCRIPT_FILE_TOO_LARGE,
u"inject_element_2.js"),
api::content_scripts::ManifestKeys::kContentScripts);

EXPECT_EQ(extension->install_warnings(), expected_warnings);
}

class ContentScriptCssInjectionTest : public ExtensionApiTest {
protected:
// TODO(rdevlin.cronin): Make a testing switch that looks like FeatureSwitch,
Expand Down
@@ -0,0 +1,23 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

var div = document.createElement('div');
div.id = 'BIG';
document.body.appendChild(div);
/*
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
PADDING OUT THE FILE TO BE OVER 800 BYTES
*/
@@ -0,0 +1,5 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

document.title = 'I CHANGED TITLE!!!';
@@ -0,0 +1,7 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

var div = document.createElement('div');
div.id = 'injected';
document.body.appendChild(div);
@@ -0,0 +1,20 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

var div = document.createElement('div');
div.id = 'injected_2';
document.body.appendChild(div);
/*
PADDING OUT THE FILE TO BE OVER 700 BYTES
PADDING OUT THE FILE TO BE OVER 700 BYTES
PADDING OUT THE FILE TO BE OVER 700 BYTES
PADDING OUT THE FILE TO BE OVER 700 BYTES
PADDING OUT THE FILE TO BE OVER 700 BYTES
PADDING OUT THE FILE TO BE OVER 700 BYTES
PADDING OUT THE FILE TO BE OVER 700 BYTES
PADDING OUT THE FILE TO BE OVER 700 BYTES
PADDING OUT THE FILE TO BE OVER 700 BYTES
PADDING OUT THE FILE TO BE OVER 700 BYTES
PADDING OUT THE FILE TO BE OVER 700 BYTES
*/
@@ -0,0 +1,29 @@
{
"name": "Large content scripts",
"version": "1.0",
"manifest_version": 3,
"description": "Content scripts that surpass the size limit are not loaded.",
"background": {
"service_worker": "worker.js",
"type": "module"
},
"permissions": ["scripting", "tabs"],
"host_permissions": ["*://example.com/*"],
"content_scripts": [
{
"matches": ["<all_urls>"],
"js": ["big.js"],
"run_at": "document_end"
},
{
"matches": ["<all_urls>"],
"js": ["inject_element.js", "change_title.js"],
"run_at": "document_end"
},
{
"matches": ["<all_urls>"],
"js": ["inject_element_2.js"],
"run_at": "document_end"
}
]
}
@@ -0,0 +1,40 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import {openTab} from '/_test_resources/test_util/tabs_util.js';

function getInjectedElementIds() {
let childIds = [];
for (const child of document.body.children)
childIds.push(child.id);
return childIds.sort();
};

chrome.test.runTests([
async function checkContentScriptInjectionResults() {
async function getTitleForTab(tabId) {
let results = await chrome.scripting.executeScript(
{target: {tabId}, func: () => document.title});
chrome.test.assertEq(1, results.length);
return results[0].result;
};

const config = await chrome.test.getConfig();
const url = `http://example.com:${config.testServer.port}/simple.html`;
const tab = await openTab(url);
const title = await getTitleForTab(tab.id);
chrome.test.assertEq('I CHANGED TITLE!!!', title);

let results = await chrome.scripting.executeScript(
{target: {tabId: tab.id}, func: getInjectedElementIds});

// Only inject_element_1.js and change_title.js should be loaded/injected as
// big.js exceeds the individual script size limit, and loading
// inject_element_2.js would exceed the extension's total script size limit.
chrome.test.assertEq(1, results.length);
chrome.test.assertEq(['injected'], results[0].result);

chrome.test.succeed();
},
]);
56 changes: 49 additions & 7 deletions extensions/browser/extension_user_script_loader.cc
Expand Up @@ -6,6 +6,7 @@

#include <stddef.h>

#include <algorithm>
#include <functional>
#include <map>
#include <memory>
Expand Down Expand Up @@ -111,7 +112,8 @@ struct VerifyContentInfo {
// couldn't be read.
std::tuple<absl::optional<std::string>, ReadScriptContentSource>
ReadScriptContent(UserScript::File* script_file,
const absl::optional<int>& script_resource_id) {
const absl::optional<int>& script_resource_id,
size_t& remaining_length) {
const base::FilePath& path = ExtensionResource::GetFilePath(
script_file->extension_root(), script_file->relative_path(),
ExtensionResource::SYMLINKS_MUST_RESOLVE_WITHIN_ROOT);
Expand All @@ -127,11 +129,20 @@ ReadScriptContent(UserScript::File* script_file,
return {absl::nullopt, ReadScriptContentSource::kFile};
}

size_t max_script_length =
std::min(remaining_length, script_parsing::GetMaxScriptLength());
std::string content;
if (!base::ReadFileToString(path, &content)) {
LOG(WARNING) << "Failed to load user script file: " << path.value();
if (!base::ReadFileToStringWithMaxSize(path, &content, max_script_length)) {
if (content.empty()) {
LOG(WARNING) << "Failed to load user script file: " << path.value();
} else {
LOG(WARNING) << "Failed to load user script file, maximum size exceeded: "
<< path.value();
}
return {absl::nullopt, ReadScriptContentSource::kFile};
}

remaining_length -= content.size();
return {std::move(content), ReadScriptContentSource::kFile};
}

Expand Down Expand Up @@ -202,9 +213,11 @@ void LoadScriptContent(const mojom::HostID& host_id,
UserScript::File* script_file,
const absl::optional<int>& script_resource_id,
const SubstitutionMap* localization_messages,
const scoped_refptr<ContentVerifier>& verifier) {
const scoped_refptr<ContentVerifier>& verifier,
size_t& remaining_length) {
DCHECK(script_file);
auto [content, source] = ReadScriptContent(script_file, script_resource_id);
auto [content, source] =
ReadScriptContent(script_file, script_resource_id, remaining_length);

bool needs_content_verification = source == ReadScriptContentSource::kFile;
if (needs_content_verification && verifier.get()) {
Expand Down Expand Up @@ -265,6 +278,23 @@ void FillScriptFileResourceIds(const UserScript::FileList& script_files,
}
}

// Returns the total length of scripts that were previously loaded (i.e. not
// present in `added_script_ids`).
size_t GetTotalLoadedScriptsLength(
UserScriptList* user_scripts,
const std::set<std::string>& added_script_ids) {
size_t total_length = 0u;
for (const std::unique_ptr<UserScript>& script : *user_scripts) {
if (added_script_ids.count(script->id()) == 0) {
for (const auto& js_script : script->js_scripts())
total_length += js_script->GetContent().length();
for (const auto& js_script : script->css_scripts())
total_length += js_script->GetContent().length();
}
}
return total_length;
}

void LoadUserScripts(
UserScriptList* user_scripts,
ScriptResourceIds script_resource_ids,
Expand All @@ -277,6 +307,17 @@ void LoadUserScripts(
size_t manifest_script_length = 0u;
size_t dynamic_script_length = 0u;

// Calculate the remaining storage allocated for scripts for this extension by
// subtracting the length of all loaded scripts from the extension's max
// scripts length. Note that subtraction is only done if the result will be
// positive (to avoid unsigned wraparound).
size_t loaded_length =
GetTotalLoadedScriptsLength(user_scripts, added_script_ids);
size_t remaining_length =
loaded_length >= script_parsing::GetMaxScriptsLengthPerExtension()
? 0u
: script_parsing::GetMaxScriptsLengthPerExtension() - loaded_length;

for (const std::unique_ptr<UserScript>& script : *user_scripts) {
size_t script_files_length = 0u;

Expand All @@ -287,7 +328,7 @@ void LoadUserScripts(
if (script_file->GetContent().empty()) {
LoadScriptContent(script->host_id(), script_file.get(),
script_resource_ids[script_file.get()], nullptr,
verifier);
verifier, remaining_length);
}

script_files_length += script_file->GetContent().length();
Expand All @@ -303,7 +344,8 @@ void LoadUserScripts(
if (script_file->GetContent().empty()) {
LoadScriptContent(script->host_id(), script_file.get(),
script_resource_ids[script_file.get()],
localization_messages.get(), verifier);
localization_messages.get(), verifier,
remaining_length);
}

script_files_length += script_file->GetContent().length();
Expand Down
Expand Up @@ -256,7 +256,7 @@ bool ContentScriptsHandler::Validate(
// and are UTF-8 encoded.
return script_parsing::ValidateFileSources(
ContentScriptsInfo::GetContentScripts(extension),
script_parsing::GetSymlinkPolicy(extension), error);
script_parsing::GetSymlinkPolicy(extension), error, warnings);
}

} // namespace extensions

0 comments on commit d17176a

Please sign in to comment.