[Backport] CVE-2020-6518: Use after free in developer tools

Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/v8/v8/+/2153215:
guard against missing CommandLineAPIScope

Fixed: chromium:986051
Change-Id: I01ef94fe43ac5c8734890706a6dccd01e008bfec
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67204}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

4 files changed, 118 insertions, 21 deletions

diff --git a/chromium/v8/src/inspector/v8-console.cc b/chromium/v8/src/inspector/v8-console.cc
index f4d0ffa0550..da93cbedae7 100644
--- a/chromium/v8/src/inspector/v8-console.cc
+++ b/chromium/v8/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/chromium/v8/src/inspector/v8-console.h b/chromium/v8/src/inspector/v8-console.h
index 4d38c51a2a2..5875164595f 100644
--- a/chromium/v8/src/inspector/v8-console.h
+++ b/chromium/v8/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/chromium/v8/test/inspector/runtime/regress-986051-expected.txt b/chromium/v8/test/inspector/runtime/regress-986051-expected.txt
new file mode 100644
index 00000000000..ad2f3d82095
--- /dev/null
+++ b/chromium/v8/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/chromium/v8/test/inspector/runtime/regress-986051.js b/chromium/v8/test/inspector/runtime/regress-986051.js
new file mode 100644
index 00000000000..7c6842a36cf
--- /dev/null
+++ b/chromium/v8/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,
+  }));
+}