backport_986051.patch 8.3 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218
  1. From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
  2. From: Cheng Zhao <[email protected]>
  3. Date: Thu, 4 Oct 2018 14:57:02 -0700
  4. Subject: fix: guard against missing CommandLineAPIScope
  5. [986051] [Medium] [CVE-2020-6518]: Security: Use-after-free of CommandLineAPIScope object
  6. Backport https://chromium.googlesource.com/v8/v8.git/+/3b60af8669916f3b019745f19144392f6b4f6b12
  7. diff --git a/src/inspector/v8-console.cc b/src/inspector/v8-console.cc
  8. index f4d0ffa0550a941e318ec595a74403e2e78d5396..da93cbedae708c48149ef8ae3354f5c7b872ef0b 100644
  9. --- a/src/inspector/v8-console.cc
  10. +++ b/src/inspector/v8-console.cc
  11. @@ -783,15 +783,11 @@ static bool isCommandLineAPIGetter(const String16& name) {
  12. void V8Console::CommandLineAPIScope::accessorGetterCallback(
  13. v8::Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
  14. - CommandLineAPIScope* scope = static_cast<CommandLineAPIScope*>(
  15. - info.Data().As<v8::External>()->Value());
  16. - DCHECK(scope);
  17. -
  18. + CommandLineAPIScope* scope = *static_cast<CommandLineAPIScope**>(
  19. + info.Data().As<v8::ArrayBuffer>()->GetBackingStore()->Data());
  20. v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
  21. - if (scope->m_cleanup) {
  22. - bool removed = info.Holder()->Delete(context, name).FromMaybe(false);
  23. - DCHECK(removed);
  24. - USE(removed);
  25. + if (scope == nullptr) {
  26. + USE(info.Holder()->Delete(context, name).FromMaybe(false));
  27. return;
  28. }
  29. v8::Local<v8::Object> commandLineAPI = scope->m_commandLineAPI;
  30. @@ -815,16 +811,14 @@ void V8Console::CommandLineAPIScope::accessorGetterCallback(
  31. void V8Console::CommandLineAPIScope::accessorSetterCallback(
  32. v8::Local<v8::Name> name, v8::Local<v8::Value> value,
  33. const v8::PropertyCallbackInfo<void>& info) {
  34. - CommandLineAPIScope* scope = static_cast<CommandLineAPIScope*>(
  35. - info.Data().As<v8::External>()->Value());
  36. + CommandLineAPIScope* scope = *static_cast<CommandLineAPIScope**>(
  37. + info.Data().As<v8::ArrayBuffer>()->GetBackingStore()->Data());
  38. + if (scope == nullptr) return;
  39. v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
  40. if (!info.Holder()->Delete(context, name).FromMaybe(false)) return;
  41. if (!info.Holder()->CreateDataProperty(context, name, value).FromMaybe(false))
  42. return;
  43. - bool removed =
  44. - scope->m_installedMethods->Delete(context, name).FromMaybe(false);
  45. - DCHECK(removed);
  46. - USE(removed);
  47. + USE(scope->m_installedMethods->Delete(context, name).FromMaybe(false));
  48. }
  49. V8Console::CommandLineAPIScope::CommandLineAPIScope(
  50. @@ -833,14 +827,15 @@ V8Console::CommandLineAPIScope::CommandLineAPIScope(
  51. : m_context(context),
  52. m_commandLineAPI(commandLineAPI),
  53. m_global(global),
  54. - m_installedMethods(v8::Set::New(context->GetIsolate())),
  55. - m_cleanup(false) {
  56. + m_installedMethods(v8::Set::New(context->GetIsolate())) {
  57. v8::MicrotasksScope microtasksScope(context->GetIsolate(),
  58. v8::MicrotasksScope::kDoNotRunMicrotasks);
  59. v8::Local<v8::Array> names;
  60. if (!m_commandLineAPI->GetOwnPropertyNames(context).ToLocal(&names)) return;
  61. - v8::Local<v8::External> externalThis =
  62. - v8::External::New(context->GetIsolate(), this);
  63. + m_thisReference =
  64. + v8::ArrayBuffer::New(context->GetIsolate(), sizeof(CommandLineAPIScope*));
  65. + *static_cast<CommandLineAPIScope**>(
  66. + m_thisReference->GetBackingStore()->Data()) = this;
  67. for (uint32_t i = 0; i < names->Length(); ++i) {
  68. v8::Local<v8::Value> name;
  69. if (!names->Get(context, i).ToLocal(&name) || !name->IsName()) continue;
  70. @@ -851,7 +846,7 @@ V8Console::CommandLineAPIScope::CommandLineAPIScope(
  71. ->SetAccessor(context, v8::Local<v8::Name>::Cast(name),
  72. CommandLineAPIScope::accessorGetterCallback,
  73. CommandLineAPIScope::accessorSetterCallback,
  74. - externalThis, v8::DEFAULT, v8::DontEnum,
  75. + m_thisReference, v8::DEFAULT, v8::DontEnum,
  76. v8::SideEffectType::kHasNoSideEffect)
  77. .FromMaybe(false)) {
  78. bool removed = m_installedMethods->Delete(context, name).FromMaybe(false);
  79. @@ -865,7 +860,8 @@ V8Console::CommandLineAPIScope::CommandLineAPIScope(
  80. V8Console::CommandLineAPIScope::~CommandLineAPIScope() {
  81. v8::MicrotasksScope microtasksScope(m_context->GetIsolate(),
  82. v8::MicrotasksScope::kDoNotRunMicrotasks);
  83. - m_cleanup = true;
  84. + *static_cast<CommandLineAPIScope**>(
  85. + m_thisReference->GetBackingStore()->Data()) = nullptr;
  86. v8::Local<v8::Array> names = m_installedMethods->AsArray();
  87. for (uint32_t i = 0; i < names->Length(); ++i) {
  88. v8::Local<v8::Value> name;
  89. diff --git a/src/inspector/v8-console.h b/src/inspector/v8-console.h
  90. index 4d38c51a2a28d6871ab00b21bde0dfb2c0605357..5875164595f78992d50c23fe90884269178ab986 100644
  91. --- a/src/inspector/v8-console.h
  92. +++ b/src/inspector/v8-console.h
  93. @@ -42,7 +42,7 @@ class V8Console : public v8::debug::ConsoleDelegate {
  94. v8::Local<v8::Object> m_commandLineAPI;
  95. v8::Local<v8::Object> m_global;
  96. v8::Local<v8::Set> m_installedMethods;
  97. - bool m_cleanup;
  98. + v8::Local<v8::ArrayBuffer> m_thisReference;
  99. DISALLOW_COPY_AND_ASSIGN(CommandLineAPIScope);
  100. };
  101. diff --git a/test/inspector/runtime/regress-986051-expected.txt b/test/inspector/runtime/regress-986051-expected.txt
  102. new file mode 100644
  103. index 0000000000000000000000000000000000000000..ad2f3d8209532a03acf39974faca6c36adf4b78c
  104. --- /dev/null
  105. +++ b/test/inspector/runtime/regress-986051-expected.txt
  106. @@ -0,0 +1,76 @@
  107. +Regression test for 986051
  108. +Regression test
  109. +{
  110. + id : <messageId>
  111. + result : {
  112. + result : {
  113. + description : 1
  114. + type : number
  115. + value : 1
  116. + }
  117. + }
  118. +}
  119. +{
  120. + id : <messageId>
  121. + result : {
  122. + exceptionDetails : {
  123. + columnNumber : 1
  124. + exception : {
  125. + className : ReferenceError
  126. + description : ReferenceError: $0 is not defined at <anonymous>:1:1
  127. + objectId : <objectId>
  128. + subtype : error
  129. + type : object
  130. + }
  131. + exceptionId : <exceptionId>
  132. + lineNumber : 1
  133. + scriptId : <scriptId>
  134. + stackTrace : {
  135. + callFrames : [
  136. + [0] : {
  137. + columnNumber : 0
  138. + functionName :
  139. + lineNumber : 0
  140. + scriptId : <scriptId>
  141. + url :
  142. + }
  143. + ]
  144. + }
  145. + text : Uncaught
  146. + }
  147. + result : {
  148. + className : ReferenceError
  149. + description : ReferenceError: $0 is not defined at <anonymous>:1:1
  150. + objectId : <objectId>
  151. + subtype : error
  152. + type : object
  153. + }
  154. + }
  155. +}
  156. +{
  157. + id : <messageId>
  158. + result : {
  159. + result : {
  160. + className : global
  161. + description : global
  162. + objectId : <objectId>
  163. + type : object
  164. + }
  165. + }
  166. +}
  167. +{
  168. + id : <messageId>
  169. + result : {
  170. + result : {
  171. + type : undefined
  172. + }
  173. + }
  174. +}
  175. +{
  176. + id : <messageId>
  177. + result : {
  178. + result : {
  179. + type : undefined
  180. + }
  181. + }
  182. +}
  183. diff --git a/test/inspector/runtime/regress-986051.js b/test/inspector/runtime/regress-986051.js
  184. new file mode 100644
  185. index 0000000000000000000000000000000000000000..7c6842a36cf23185178ec99d97a488e88ca30910
  186. --- /dev/null
  187. +++ b/test/inspector/runtime/regress-986051.js
  188. @@ -0,0 +1,25 @@
  189. +// Copyright 2020 the V8 project authors. All rights reserved.
  190. +// Use of this source code is governed by a BSD-style license that can be
  191. +// found in the LICENSE file.
  192. +
  193. +let {Protocol} = InspectorTest.start(
  194. + "Regression test for 986051");
  195. +
  196. +Protocol.Runtime.enable();
  197. +(async function() {
  198. + InspectorTest.log("Regression test");
  199. + evaluateRepl('1', true);
  200. + evaluateRepl('$0', false);
  201. + evaluateRepl('Object.defineProperty(globalThis, "$0", {configurable: false});', true);
  202. + evaluateRepl('$0', true);
  203. + evaluateRepl('$0', false);
  204. + InspectorTest.completeTest();
  205. +})();
  206. +
  207. +async function evaluateRepl(expression, includeCommandLineAPI) {
  208. + InspectorTest.logMessage(await Protocol.Runtime.evaluate({
  209. + expression,
  210. + includeCommandLineAPI,
  211. + replMode: true,
  212. + }));
  213. +}