Skip to content

Commit

Permalink
chore: cherry-pick fix from chromium issue 986051 (#24614)
Browse files Browse the repository at this point in the history
  • Loading branch information
@zcbenz
zcbenz committed Jul 18, 2020
1 parent 61ae10a commit 3fad4f2
Show file tree
Hide file tree
Showing 2 changed files with 219 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/v8/.patches
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ do_not_export_private_v8_symbols_on_windows.patch
revert_cleanup_switch_offset_of_to_offsetof_where_possible.patch
fix_build_deprecated_attirbute_for_older_msvc_versions.patch
backport_1084820.patch
backport_986051.patch
218 changes: 218 additions & 0 deletions patches/v8/backport_986051.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Thu, 4 Oct 2018 14:57:02 -0700
Subject: fix: guard against missing CommandLineAPIScope

[986051] [Medium] [CVE-2020-6518]: Security: Use-after-free of CommandLineAPIScope object
Backport https://chromium.googlesource.com/v8/v8.git/+/3b60af8669916f3b019745f19144392f6b4f6b12

diff --git a/src/inspector/v8-console.cc b/src/inspector/v8-console.cc
index ec7709f8c690e17d873ce6b2499603b9de635e14..4fd33e346ae90e547997f1f604cb3963d1ba8e89 100644
--- a/src/inspector/v8-console.cc
+++ b/src/inspector/v8-console.cc
@@ -783,15 +783,11 @@ static bool isCommandLineAPIGetter(const String16& name) {

void V8Console::CommandLineAPIScope::accessorGetterCallback(
v8::Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
- CommandLineAPIScope* scope = static_cast<CommandLineAPIScope*>(
- info.Data().As<v8::External>()->Value());
- DCHECK(scope);
-
+ CommandLineAPIScope* scope = *static_cast<CommandLineAPIScope**>(
+ info.Data().As<v8::ArrayBuffer>()->GetBackingStore()->Data());
v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
- if (scope->m_cleanup) {
- bool removed = info.Holder()->Delete(context, name).FromMaybe(false);
- DCHECK(removed);
- USE(removed);
+ if (scope == nullptr) {
+ USE(info.Holder()->Delete(context, name).FromMaybe(false));
return;
}
v8::Local<v8::Object> commandLineAPI = scope->m_commandLineAPI;
@@ -815,16 +811,14 @@ void V8Console::CommandLineAPIScope::accessorGetterCallback(
void V8Console::CommandLineAPIScope::accessorSetterCallback(
v8::Local<v8::Name> name, v8::Local<v8::Value> value,
const v8::PropertyCallbackInfo<void>& info) {
- CommandLineAPIScope* scope = static_cast<CommandLineAPIScope*>(
- info.Data().As<v8::External>()->Value());
+ CommandLineAPIScope* scope = *static_cast<CommandLineAPIScope**>(
+ info.Data().As<v8::ArrayBuffer>()->GetBackingStore()->Data());
+ if (scope == nullptr) return;
v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
if (!info.Holder()->Delete(context, name).FromMaybe(false)) return;
if (!info.Holder()->CreateDataProperty(context, name, value).FromMaybe(false))
return;
- bool removed =
- scope->m_installedMethods->Delete(context, name).FromMaybe(false);
- DCHECK(removed);
- USE(removed);
+ USE(scope->m_installedMethods->Delete(context, name).FromMaybe(false));
}

V8Console::CommandLineAPIScope::CommandLineAPIScope(
@@ -833,14 +827,15 @@ V8Console::CommandLineAPIScope::CommandLineAPIScope(
: m_context(context),
m_commandLineAPI(commandLineAPI),
m_global(global),
- m_installedMethods(v8::Set::New(context->GetIsolate())),
- m_cleanup(false) {
+ m_installedMethods(v8::Set::New(context->GetIsolate())) {
v8::MicrotasksScope microtasksScope(context->GetIsolate(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
v8::Local<v8::Array> names;
if (!m_commandLineAPI->GetOwnPropertyNames(context).ToLocal(&names)) return;
- v8::Local<v8::External> externalThis =
- v8::External::New(context->GetIsolate(), this);
+ m_thisReference =
+ v8::ArrayBuffer::New(context->GetIsolate(), sizeof(CommandLineAPIScope*));
+ *static_cast<CommandLineAPIScope**>(
+ m_thisReference->GetBackingStore()->Data()) = this;
for (uint32_t i = 0; i < names->Length(); ++i) {
v8::Local<v8::Value> name;
if (!names->Get(context, i).ToLocal(&name) || !name->IsName()) continue;
@@ -851,7 +846,7 @@ V8Console::CommandLineAPIScope::CommandLineAPIScope(
->SetAccessor(context, v8::Local<v8::Name>::Cast(name),
CommandLineAPIScope::accessorGetterCallback,
CommandLineAPIScope::accessorSetterCallback,
- externalThis, v8::DEFAULT, v8::DontEnum,
+ m_thisReference, v8::DEFAULT, v8::DontEnum,
v8::SideEffectType::kHasNoSideEffect)
.FromMaybe(false)) {
bool removed = m_installedMethods->Delete(context, name).FromMaybe(false);
@@ -865,7 +860,8 @@ V8Console::CommandLineAPIScope::CommandLineAPIScope(
V8Console::CommandLineAPIScope::~CommandLineAPIScope() {
v8::MicrotasksScope microtasksScope(m_context->GetIsolate(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
- m_cleanup = true;
+ *static_cast<CommandLineAPIScope**>(
+ m_thisReference->GetBackingStore()->Data()) = nullptr;
v8::Local<v8::Array> names = m_installedMethods->AsArray();
for (uint32_t i = 0; i < names->Length(); ++i) {
v8::Local<v8::Value> name;
diff --git a/src/inspector/v8-console.h b/src/inspector/v8-console.h
index 4d38c51a2a28d6871ab00b21bde0dfb2c0605357..5875164595f78992d50c23fe90884269178ab986 100644
--- a/src/inspector/v8-console.h
+++ b/src/inspector/v8-console.h
@@ -42,7 +42,7 @@ class V8Console : public v8::debug::ConsoleDelegate {
v8::Local<v8::Object> m_commandLineAPI;
v8::Local<v8::Object> m_global;
v8::Local<v8::Set> m_installedMethods;
- bool m_cleanup;
+ v8::Local<v8::ArrayBuffer> m_thisReference;

DISALLOW_COPY_AND_ASSIGN(CommandLineAPIScope);
};
diff --git a/test/inspector/runtime/regress-986051-expected.txt b/test/inspector/runtime/regress-986051-expected.txt
new file mode 100644
index 0000000000000000000000000000000000000000..ad2f3d8209532a03acf39974faca6c36adf4b78c
--- /dev/null
+++ b/test/inspector/runtime/regress-986051-expected.txt
@@ -0,0 +1,76 @@
+Regression test for 986051
+Regression test
+{
+ id : <messageId>
+ result : {
+ result : {
+ description : 1
+ type : number
+ value : 1
+ }
+ }
+}
+{
+ id : <messageId>
+ result : {
+ exceptionDetails : {
+ columnNumber : 1
+ exception : {
+ className : ReferenceError
+ description : ReferenceError: $0 is not defined at <anonymous>:1:1
+ objectId : <objectId>
+ subtype : error
+ type : object
+ }
+ exceptionId : <exceptionId>
+ lineNumber : 1
+ scriptId : <scriptId>
+ stackTrace : {
+ callFrames : [
+ [0] : {
+ columnNumber : 0
+ functionName :
+ lineNumber : 0
+ scriptId : <scriptId>
+ url :
+ }
+ ]
+ }
+ text : Uncaught
+ }
+ result : {
+ className : ReferenceError
+ description : ReferenceError: $0 is not defined at <anonymous>:1:1
+ objectId : <objectId>
+ subtype : error
+ type : object
+ }
+ }
+}
+{
+ id : <messageId>
+ result : {
+ result : {
+ className : global
+ description : global
+ objectId : <objectId>
+ type : object
+ }
+ }
+}
+{
+ id : <messageId>
+ result : {
+ result : {
+ type : undefined
+ }
+ }
+}
+{
+ id : <messageId>
+ result : {
+ result : {
+ type : undefined
+ }
+ }
+}
diff --git a/test/inspector/runtime/regress-986051.js b/test/inspector/runtime/regress-986051.js
new file mode 100644
index 0000000000000000000000000000000000000000..7c6842a36cf23185178ec99d97a488e88ca30910
--- /dev/null
+++ b/test/inspector/runtime/regress-986051.js
@@ -0,0 +1,25 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let {Protocol} = InspectorTest.start(
+ "Regression test for 986051");
+
+Protocol.Runtime.enable();
+(async function() {
+ InspectorTest.log("Regression test");
+ evaluateRepl('1', true);
+ evaluateRepl('$0', false);
+ evaluateRepl('Object.defineProperty(globalThis, "$0", {configurable: false});', true);
+ evaluateRepl('$0', true);
+ evaluateRepl('$0', false);
+ InspectorTest.completeTest();
+})();
+
+async function evaluateRepl(expression, includeCommandLineAPI) {
+ InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+ expression,
+ includeCommandLineAPI,
+ replMode: true,
+ }));
+}

0 comments on commit 3fad4f2

Please sign in to comment.