Skip to content

Commit

Permalink
Load SafeBuiltins in a closure instead of global namespace
Browse files Browse the repository at this point in the history
  • Loading branch information
darkdh committed Aug 24, 2022
1 parent 046005a commit 0e1262a
Show file tree
Hide file tree
Showing 10 changed files with 204 additions and 44 deletions.
16 changes: 11 additions & 5 deletions browser/brave_wallet/solana_provider_renderer_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,14 @@ class SolanaProviderRendererTest : public InProcessBrowserTest {
ASSERT_TRUE(content::WaitForLoadStop(web_contents(browser)));
}

void LoadInternalScriptForCreatingTestData() {
constexpr char BypassSafeBuiltins[] = "$Object = Object";
// Bypass loading safe builtins because we only use this script to create
// test data.
ASSERT_TRUE(ExecJs(web_contents(browser()), BypassSafeBuiltins));
ASSERT_TRUE(ExecJs(web_contents(browser()), *g_provider_internal_script));
}

protected:
net::EmbeddedTestServer https_server_;
TestBraveContentBrowserClient test_content_browser_client_;
Expand Down Expand Up @@ -775,7 +783,7 @@ IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, Disconnect) {
}

IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, SignTransaction) {
ASSERT_TRUE(ExecJs(web_contents(browser()), *g_provider_internal_script));
LoadInternalScriptForCreatingTestData();
const std::string serialized_tx_str = VectorToArrayString(kSerializedTx);
const std::string tx =
base::StrCat({"(window._brave_solana.createTransaction(new Uint8Array([",
Expand Down Expand Up @@ -821,7 +829,7 @@ IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, SignTransaction) {
}

IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, SignAllTransactions) {
ASSERT_TRUE(ExecJs(web_contents(browser()), *g_provider_internal_script));
LoadInternalScriptForCreatingTestData();
const std::string serialized_tx_str = VectorToArrayString(kSerializedTx);
const std::string txs = base::StrCat(
{"([window._brave_solana.createTransaction(new Uint8Array([",
Expand Down Expand Up @@ -883,7 +891,7 @@ IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, SignAllTransactions) {
}

IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, SignAndSendTransaction) {
ASSERT_TRUE(ExecJs(web_contents(browser()), *g_provider_internal_script));
LoadInternalScriptForCreatingTestData();
const std::string serialized_tx_str = VectorToArrayString(kSerializedTx);
const std::string send_options =
R"({"maxRetries": 9007199254740991,
Expand Down Expand Up @@ -1276,7 +1284,6 @@ IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, SafeBuiltins) {
auto result = EvalJs(web_contents(browser()), R"(
async function test() {
Object.defineProperties = ()=>{}
$Object.defineProperties = ()=>{}
await window.braveSolana.connect()
window._brave_solana.createPublickey = ()=>'0x00'
window.domAutomationController.send(
Expand All @@ -1292,7 +1299,6 @@ IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, SafeBuiltins) {
auto result2 = EvalJs(web_contents(browser()), R"(
async function test() {
Object.freeze = ()=>{}
$Object.freeze = ()=>{}
await window.braveSolana.connect()
window._brave_solana.abc = 123
if (window._brave_solana.abc === 123)
Expand Down
3 changes: 2 additions & 1 deletion components/brave_wallet/renderer/js_ethereum_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ static base::NoDestructor<std::string> g_provider_script("");
constexpr char kEthereum[] = "ethereum";
constexpr char kEmit[] = "emit";
constexpr char kIsBraveWallet[] = "isBraveWallet";
constexpr char kEthereumProviderScript[] = "ethereum_provider.js";

} // namespace

Expand Down Expand Up @@ -579,7 +580,7 @@ v8::Local<v8::Promise> JSEthereumProvider::IsUnlocked() {
void JSEthereumProvider::InjectInitScript(bool is_main_world) {
blink::WebLocalFrame* web_frame = render_frame_->GetWebFrame();
if (is_main_world) {
ExecuteScript(web_frame, *g_provider_script);
ExecuteScript(web_frame, *g_provider_script, kEthereumProviderScript);
}
}

Expand Down
10 changes: 7 additions & 3 deletions components/brave_wallet/renderer/js_solana_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ constexpr char kSolana[] = "solana";
constexpr char kSignature[] = "signature";
constexpr char kSignatures[] = "signatures";
constexpr char kToString[] = "toString";
constexpr char kSolanaProviderSript[] = "solana_provider.js";
constexpr char kSolanaProviderInternalSript[] = "solana_provider_internal.js";

} // namespace

Expand Down Expand Up @@ -164,7 +166,7 @@ void JSSolanaProvider::Install(bool allow_overwrite_window_solana,
}

blink::WebLocalFrame* web_frame = render_frame->GetWebFrame();
ExecuteScript(web_frame, *g_provider_script);
ExecuteScript(web_frame, *g_provider_script, kSolanaProviderSript);
}

gin::ObjectTemplateBuilder JSSolanaProvider::GetObjectTemplateBuilder(
Expand Down Expand Up @@ -874,7 +876,8 @@ v8::Local<v8::Value> JSSolanaProvider::CreatePublicKey(
v8::Local<v8::Context> context,
const std::string& base58_str) {
// Internal object for CreatePublicKey and CreateTransaction
ExecuteScript(render_frame()->GetWebFrame(), *g_provider_internal_script);
ExecuteScript(render_frame()->GetWebFrame(), *g_provider_internal_script,
kSolanaProviderInternalSript);
const base::Value public_key_value(base58_str);
std::vector<v8::Local<v8::Value>> args;
args.push_back(v8_value_converter_->ToV8Value(public_key_value, context));
Expand All @@ -889,7 +892,8 @@ v8::Local<v8::Value> JSSolanaProvider::CreateTransaction(
v8::Local<v8::Context> context,
const std::vector<uint8_t> serialized_tx) {
// Internal object for CreatePublicKey and CreateTransaction
ExecuteScript(render_frame()->GetWebFrame(), *g_provider_internal_script);
ExecuteScript(render_frame()->GetWebFrame(), *g_provider_internal_script,
kSolanaProviderInternalSript);
const base::Value serialized_tx_value(serialized_tx);
std::vector<v8::Local<v8::Value>> args;
args.push_back(v8_value_converter_->ToV8Value(serialized_tx_value, context));
Expand Down
32 changes: 5 additions & 27 deletions components/brave_wallet/renderer/v8_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,15 @@

#include <utility>

#include "brave/components/safe_builtins/renderer/safe_builtins.h"
#include "brave/components/safe_builtins/renderer/safe_builtins_helpers.h"
#include "gin/converter.h"
#include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/public/web/web_script_source.h"
#include "v8/include/v8-function.h"
#include "v8/include/v8-microtask-queue.h"
#include "v8/include/v8-object.h"

namespace brave_wallet {

namespace {

void FreezeAndSetGlobally(const base::StringPiece& name,
v8::Local<v8::Context> context,
v8::Local<v8::Value> value) {
v8::Local<v8::Object> object = v8::Local<v8::Object>::Cast(value);
CHECK(value->IsObject()) << name;

object->SetIntegrityLevel(context, v8::IntegrityLevel::kFrozen);
SetProviderNonWritable(context, context->Global(), object,
gin::StringToV8(context->GetIsolate(), name), false);
}

} // namespace

v8::MaybeLocal<v8::Value> GetProperty(v8::Local<v8::Context> context,
v8::Local<v8::Value> object,
const base::StringPiece& name) {
Expand Down Expand Up @@ -100,12 +84,13 @@ v8::MaybeLocal<v8::Value> CallMethodOfObject(
static_cast<int>(args.size()), args.data());
}

void ExecuteScript(blink::WebLocalFrame* web_frame, const std::string script) {
void ExecuteScript(blink::WebLocalFrame* web_frame,
const std::string& script,
const std::string& name) {
if (web_frame->IsProvisional())
return;

web_frame->ExecuteScript(
blink::WebScriptSource(blink::WebString::FromUTF8(script)));
brave::LoadScriptWithSafeBuiltins(web_frame, script, name);
}

void SetProviderNonWritable(v8::Local<v8::Context> context,
Expand All @@ -131,11 +116,4 @@ void SetOwnPropertyNonWritable(v8::Local<v8::Context> context,
provider_object->DefineProperty(context, property_name, desc).Check();
}

void InitSafeBuiltinsProtection(v8::Local<v8::Context> context) {
brave::SafeBuiltins safe_builtins(context);
// Object
v8::Local<v8::Value> object = safe_builtins.GetObjekt();
FreezeAndSetGlobally("$Object", context, object);
}

} // namespace brave_wallet
7 changes: 3 additions & 4 deletions components/brave_wallet/renderer/v8_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ v8::MaybeLocal<v8::Value> CallMethodOfObject(
const base::StringPiece& method_name,
std::vector<v8::Local<v8::Value>>&& args);

void ExecuteScript(blink::WebLocalFrame* web_frame, const std::string script);
void ExecuteScript(blink::WebLocalFrame* web_frame,
const std::string& script,
const std::string& name);

// By default we allow extensions to overwrite the window.[provider] object
// but if the user goes into settings and explicitly selects to use Brave Wallet
Expand All @@ -55,9 +57,6 @@ void SetOwnPropertyNonWritable(v8::Local<v8::Context> context,
v8::Local<v8::Object> provider_object,
v8::Local<v8::String> property_name);

// Prevents built-in prototypes from being overwritten.
void InitSafeBuiltinsProtection(v8::Local<v8::Context> context);

} // namespace brave_wallet

#endif // BRAVE_COMPONENTS_BRAVE_WALLET_RENDERER_V8_HELPER_H_
4 changes: 4 additions & 0 deletions components/safe_builtins/renderer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@ source_set("renderer") {
sources = [
"safe_builtins.cc",
"safe_builtins.h",
"safe_builtins_helpers.cc",
"safe_builtins_helpers.h",
]

deps = [
"//base",
"//gin",
"//third_party/blink/public:blink",
"//v8",
]
}
2 changes: 2 additions & 0 deletions components/safe_builtins/renderer/DEPS
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
include_rules = [
"+gin",
"+third_party/blink/public",
"+v8/include",
]
144 changes: 144 additions & 0 deletions components/safe_builtins/renderer/safe_builtins_helpers.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
/* Copyright (c) 2022 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/components/safe_builtins/renderer/safe_builtins_helpers.h"

#include "brave/components/safe_builtins/renderer/safe_builtins.h"
#include "gin/converter.h"
#include "third_party/blink/public/web/web_console_message.h"
#include "third_party/blink/public/web/web_local_frame.h"
#include "v8/include/v8-exception.h"
#include "v8/include/v8-function.h"
#include "v8/include/v8-local-handle.h"
#include "v8/include/v8-microtask-queue.h"
#include "v8/include/v8-object.h"

namespace brave {

namespace {

std::string CreateExceptionString(v8::Local<v8::Context> context,
const v8::TryCatch& try_catch) {
v8::Local<v8::Message> message(try_catch.Message());
if (message.IsEmpty()) {
return "try_catch has no message";
}

std::string resource_name = "<unknown resource>";
if (!message->GetScriptOrigin().ResourceName().IsEmpty()) {
v8::String::Utf8Value resource_name_v8(
context->GetIsolate(), message->GetScriptOrigin().ResourceName());
resource_name.assign(*resource_name_v8, resource_name_v8.length());
}

std::string error_message = "<no error message>";
if (!message->Get().IsEmpty()) {
v8::String::Utf8Value error_message_v8(context->GetIsolate(),
message->Get());
error_message.assign(*error_message_v8, error_message_v8.length());
}

int line_number = 0;
auto maybe = message->GetLineNumber(context);
line_number = maybe.IsJust() ? maybe.FromJust() : 0;

return base::StringPrintf("%s:%d: %s", resource_name.c_str(), line_number,
error_message.c_str());
}

v8::Local<v8::String> WrapSource(v8::Isolate* isolate,
v8::Local<v8::String> source) {
v8::EscapableHandleScope handle_scope(isolate);
v8::Local<v8::String> left = gin::StringToV8(isolate,
"(function($Object) {"
"'use strict';");
v8::Local<v8::String> right = gin::StringToV8(isolate, "\n})");
return handle_scope.Escape(v8::Local<v8::String>(v8::String::Concat(
isolate, left, v8::String::Concat(isolate, source, right))));
}

v8::Local<v8::Value> RunScript(v8::Local<v8::Context> context,
v8::Local<v8::String> name,
v8::Local<v8::String> code) {
v8::EscapableHandleScope handle_scope(context->GetIsolate());
v8::Context::Scope context_scope(context);

v8::MicrotasksScope microtasks(context->GetIsolate(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::TryCatch try_catch(context->GetIsolate());
try_catch.SetCaptureMessage(true);
v8::ScriptOrigin origin(context->GetIsolate(), name);
v8::ScriptCompiler::Source script_source(code, origin);
v8::Local<v8::Script> script;
if (!v8::ScriptCompiler::Compile(
context, &script_source, v8::ScriptCompiler::kNoCompileOptions,
v8::ScriptCompiler::NoCacheReason::kNoCacheBecauseInlineScript)
.ToLocal(&script)) {
blink::WebConsoleMessage::LogWebConsoleMessage(
context, blink::WebConsoleMessage(
blink::mojom::ConsoleMessageLevel::kError,
blink::WebString::FromUTF8(
CreateExceptionString(context, try_catch))));
return v8::Undefined(context->GetIsolate());
}

v8::Local<v8::Value> result;
if (!script->Run(context).ToLocal(&result)) {
blink::WebConsoleMessage::LogWebConsoleMessage(
context, blink::WebConsoleMessage(
blink::mojom::ConsoleMessageLevel::kError,
blink::WebString::FromUTF8(
CreateExceptionString(context, try_catch))));
return v8::Undefined(context->GetIsolate());
}

return handle_scope.Escape(result);
}

void SafeCallFunction(blink::WebLocalFrame* web_frame,
v8::Local<v8::Context> context,
const v8::Local<v8::Function>& function,
int argc,
v8::Local<v8::Value> argv[]) {
v8::HandleScope handle_scope(context->GetIsolate());
v8::Context::Scope scope(context);
v8::MicrotasksScope microtasks(context->GetIsolate(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::Local<v8::Object> global = context->Global();
if (web_frame) {
web_frame->RequestExecuteV8Function(context, function, global, argc, argv,
{});
}
}

} // namespace

void LoadScriptWithSafeBuiltins(blink::WebLocalFrame* web_frame,
const std::string& script,
const std::string& name) {
v8::Local<v8::Context> context = web_frame->MainWorldScriptContext();
v8::Local<v8::String> wrapped_source(WrapSource(
context->GetIsolate(), gin::StringToV8(context->GetIsolate(), script)));
// Wrapping script in function(...) {...}
v8::Local<v8::Value> func_as_value = RunScript(
context, gin::StringToV8(context->GetIsolate(), name), wrapped_source);
if (func_as_value.IsEmpty() || func_as_value->IsUndefined()) {
std::string message =
base::StringPrintf("Bad source for require(\"%s\")", name.c_str());
web_frame->AddMessageToConsole(
blink::WebConsoleMessage(blink::mojom::ConsoleMessageLevel::kError,
blink::WebString::FromUTF8(message)));
return;
}

v8::Local<v8::Function> func = v8::Local<v8::Function>::Cast(func_as_value);
SafeBuiltins safe_builtins(context);
// These must match the argument order in WrapSource.
v8::Local<v8::Value> args[] = {safe_builtins.GetObjekt()};

SafeCallFunction(web_frame, context, func, std::size(args), args);
}

} // namespace brave
26 changes: 26 additions & 0 deletions components/safe_builtins/renderer/safe_builtins_helpers.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* Copyright (c) 2022 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_COMPONENTS_SAFE_BUILTINS_RENDERER_SAFE_BUILTINS_HELPERS_H_
#define BRAVE_COMPONENTS_SAFE_BUILTINS_RENDERER_SAFE_BUILTINS_HELPERS_H_

#include <string>

namespace blink {
class WebLocalFrame;
} // namespace blink

namespace brave {

// Load script in a closure that will use safe builtin types to prevent
// prototype pollution attack. When a new type is added, we need to update
// WrapSource and args for SafeCallFunction.
void LoadScriptWithSafeBuiltins(blink::WebLocalFrame* web_frame,
const std::string& script,
const std::string& name);

} // namespace brave

#endif // BRAVE_COMPONENTS_SAFE_BUILTINS_RENDERER_SAFE_BUILTINS_HELPERS_H_
Loading

0 comments on commit 0e1262a

Please sign in to comment.