Do not walk prototype chain of restricted object when displaying it in

devtools

Bug: chromium:1213374
Change-Id: Ie064873e8a3998aad01120022e39e93dba0cb729
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3041424
Commit-Queue: Danil Somsikov <dsv@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75874}

src/debug/debug-interface.h

@@ -614,7 +614,7 @@ struct PropertyDescriptor {
   v8::Local<v8::Value> set;
 };
 
-class PropertyIterator {
+class V8_EXPORT_PRIVATE PropertyIterator {
  public:
   // Creating a PropertyIterator can potentially throw an exception.
   // The returned std::unique_ptr is empty iff that happens.

src/debug/debug-property-iterator.cc

@@ -21,10 +21,9 @@ std::unique_ptr<DebugPropertyIterator> DebugPropertyIterator::Create(
       new DebugPropertyIterator(isolate, receiver));
 
   if (receiver->IsJSProxy()) {
-    iterator->is_own_ = false;
-    iterator->prototype_iterator_.AdvanceIgnoringProxies();
+    iterator->AdvanceToPrototype();
   }
-  if (iterator->prototype_iterator_.IsAtEnd()) return iterator;
+  if (iterator->Done()) return iterator;
 
   if (!iterator->FillKeysForCurrentPrototypeAndStage()) return nullptr;
   if (iterator->should_move_to_next_stage() && !iterator->AdvanceInternal()) {
@@ -40,8 +39,14 @@ DebugPropertyIterator::DebugPropertyIterator(Isolate* isolate,
       prototype_iterator_(isolate, receiver, kStartAtReceiver,
                           PrototypeIterator::END_AT_NULL) {}
 
-bool DebugPropertyIterator::Done() const {
-  return prototype_iterator_.IsAtEnd();
+bool DebugPropertyIterator::Done() const { return is_done_; }
+
+void DebugPropertyIterator::AdvanceToPrototype() {
+  stage_ = kExoticIndices;
+  is_own_ = false;
+  if (!prototype_iterator_.HasAccess()) is_done_ = true;
+  prototype_iterator_.AdvanceIgnoringProxies();
+  if (prototype_iterator_.IsAtEnd()) is_done_ = true;
 }
 
 bool DebugPropertyIterator::AdvanceInternal() {
@@ -56,9 +61,7 @@ bool DebugPropertyIterator::AdvanceInternal() {
         stage_ = Stage::kAllProperties;
         break;
       case Stage::kAllProperties:
-        stage_ = kExoticIndices;
-        is_own_ = false;
-        prototype_iterator_.AdvanceIgnoringProxies();
+        AdvanceToPrototype();
         break;
     }
     if (!FillKeysForCurrentPrototypeAndStage()) return false;
@@ -145,7 +148,7 @@ bool DebugPropertyIterator::FillKeysForCurrentPrototypeAndStage() {
   current_key_index_ = 0;
   exotic_length_ = 0;
   keys_ = Handle<FixedArray>::null();
-  if (prototype_iterator_.IsAtEnd()) return true;
+  if (is_done_) return true;
   Handle<JSReceiver> receiver =
       PrototypeIterator::GetCurrent<JSReceiver>(prototype_iterator_);
   bool has_exotic_indices = receiver->IsJSTypedArray();
@@ -169,7 +172,7 @@ bool DebugPropertyIterator::FillKeysForCurrentPrototypeAndStage() {
 }
 
 bool DebugPropertyIterator::should_move_to_next_stage() const {
-  if (prototype_iterator_.IsAtEnd()) return false;
+  if (is_done_) return false;
   if (stage_ == kExoticIndices) return current_key_index_ >= exotic_length_;
   return keys_.is_null() ||
          current_key_index_ >= static_cast<size_t>(keys_->length());

src/debug/debug-property-iterator.h

@@ -45,6 +45,7 @@ class DebugPropertyIterator final : public debug::PropertyIterator {
   bool should_move_to_next_stage() const;
   void CalculateNativeAccessorFlags();
   Handle<Name> raw_name() const;
+  void AdvanceToPrototype();
   V8_WARN_UNUSED_RESULT bool AdvanceInternal();
 
   Isolate* isolate_;
@@ -59,6 +60,7 @@ class DebugPropertyIterator final : public debug::PropertyIterator {
   bool calculated_native_accessor_flags_ = false;
   int native_accessor_flags_ = 0;
   bool is_own_ = true;
+  bool is_done_ = false;
 };
 }  // namespace internal
 }  // namespace v8

src/runtime/runtime-debug.cc

@@ -194,13 +194,17 @@ MaybeHandle<JSArray> Runtime::GetInternalProperties(Isolate* isolate,
                                                     Handle<Object> object) {
   auto result = ArrayList::New(isolate, 8 * 2);
   if (object->IsJSObject()) {
-    PrototypeIterator iter(isolate, Handle<JSObject>::cast(object));
-    Handle<Object> prototype = PrototypeIterator::GetCurrent(iter);
-    if (!prototype->IsNull(isolate)) {
-      result = ArrayList::Add(
-          isolate, result,
-          isolate->factory()->NewStringFromStaticChars("[[Prototype]]"),
-          prototype);
+    PrototypeIterator iter(isolate, Handle<JSObject>::cast(object),
+                           kStartAtReceiver);
+    if (iter.HasAccess()) {
+      iter.Advance();
+      Handle<Object> prototype = PrototypeIterator::GetCurrent(iter);
+      if (!prototype->IsNull(isolate)) {
+        result = ArrayList::Add(
+            isolate, result,
+            isolate->factory()->NewStringFromStaticChars("[[Prototype]]"),
+            prototype);
+      }
     }
   }
   if (object->IsJSBoundFunction()) {

src/runtime/runtime.h

@@ -801,8 +801,8 @@ class Runtime : public AllStatic {
   V8_WARN_UNUSED_RESULT static MaybeHandle<Object> HasProperty(
       Isolate* isolate, Handle<Object> object, Handle<Object> key);
 
-  V8_WARN_UNUSED_RESULT static MaybeHandle<JSArray> GetInternalProperties(
-      Isolate* isolate, Handle<Object>);
+  V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT static MaybeHandle<JSArray>
+  GetInternalProperties(Isolate* isolate, Handle<Object>);
 
   V8_WARN_UNUSED_RESULT static MaybeHandle<Object> ThrowIteratorError(
       Isolate* isolate, Handle<Object> object);

test/unittests/BUILD.gn

@@ -300,6 +300,7 @@ v8_source_set("unittests_sources") {
     "compiler/value-numbering-reducer-unittest.cc",
     "compiler/zone-stats-unittest.cc",
     "date/date-cache-unittest.cc",
+    "debug/debug-property-iterator-unittest.cc",
     "diagnostics/eh-frame-iterator-unittest.cc",
     "diagnostics/eh-frame-writer-unittest.cc",
     "execution/microtask-queue-unittest.cc",
@@ -369,6 +370,7 @@ v8_source_set("unittests_sources") {
     "regress/regress-crbug-1056054-unittest.cc",
     "regress/regress-crbug-938251-unittest.cc",
     "run-all-unittests.cc",
+    "runtime/runtime-debug-unittest.cc",
     "strings/char-predicates-unittest.cc",
     "strings/unicode-unittest.cc",
     "tasks/background-compile-task-unittest.cc",

test/unittests/debug/debug-property-iterator-unittest.cc

+// Copyright 2021 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.
+
+#include "include/v8.h"
+#include "src/api/api.h"
+#include "src/objects/objects-inl.h"
+#include "test/unittests/test-utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace v8 {
+namespace debug {
+namespace {
+
+using DebugPropertyIteratorTest = TestWithContext;
+
+TEST_F(DebugPropertyIteratorTest, WalksPrototypeChain) {
+  TryCatch try_catch(isolate());
+
+  Local<Object> object = Object::New(isolate());
+
+  ASSERT_TRUE(object
+                  ->CreateDataProperty(
+                      context(),
+                      String::NewFromUtf8Literal(isolate(), "own_property"),
+                      Number::New(isolate(), 42))
+                  .FromMaybe(false));
+
+  Local<Object> prototype = Object::New(isolate());
+  ASSERT_TRUE(object->SetPrototype(context(), prototype).FromMaybe(false));
+  ASSERT_TRUE(prototype
+                  ->CreateDataProperty(context(),
+                                       String::NewFromUtf8Literal(
+                                           isolate(), "prototype_property"),
+                                       Number::New(isolate(), 21))
+                  .FromMaybe(false));
+
+  auto iterator = PropertyIterator::Create(context(), object);
+  ASSERT_NE(iterator, nullptr);
+  ASSERT_FALSE(iterator->Done());
+  EXPECT_TRUE(iterator->is_own());
+  char name_buffer[100];
+  iterator->name().As<v8::String>()->WriteUtf8(isolate(), name_buffer);
+  EXPECT_EQ("own_property", std::string(name_buffer));
+  ASSERT_TRUE(iterator->Advance().FromMaybe(false));
+
+  ASSERT_FALSE(iterator->Done());
+  EXPECT_TRUE(iterator->is_own());
+  iterator->name().As<v8::String>()->WriteUtf8(isolate(), name_buffer);
+  EXPECT_EQ("own_property", std::string(name_buffer));
+  ASSERT_TRUE(iterator->Advance().FromMaybe(false));
+
+  ASSERT_FALSE(iterator->Done());
+  EXPECT_FALSE(iterator->is_own());
+  iterator->name().As<v8::String>()->WriteUtf8(isolate(), name_buffer);
+  EXPECT_EQ("prototype_property", std::string(name_buffer));
+  ASSERT_TRUE(iterator->Advance().FromMaybe(false));
+
+  ASSERT_FALSE(iterator->Done());
+}
+
+bool may_access = true;
+
+bool AccessCheck(Local<Context> accessing_context,
+                 Local<Object> accessed_object, Local<Value> data) {
+  return may_access;
+}
+
+TEST_F(DebugPropertyIteratorTest, DoestWalksPrototypeChainIfInaccesible) {
+  TryCatch try_catch(isolate());
+
+  Local<ObjectTemplate> object_template = ObjectTemplate::New(isolate());
+  object_template->SetAccessCheckCallback(AccessCheck);
+
+  Local<Object> object =
+      object_template->NewInstance(context()).ToLocalChecked();
+  ASSERT_TRUE(object
+                  ->CreateDataProperty(
+                      context(),
+                      String::NewFromUtf8Literal(isolate(), "own_property"),
+                      Number::New(isolate(), 42))
+                  .FromMaybe(false));
+
+  auto iterator = PropertyIterator::Create(context(), object);
+  may_access = false;
+  ASSERT_NE(iterator, nullptr);
+  ASSERT_FALSE(iterator->Done());
+  EXPECT_TRUE(iterator->is_own());
+  char name_buffer[100];
+  iterator->name().As<v8::String>()->WriteUtf8(isolate(), name_buffer);
+  EXPECT_EQ("own_property", std::string(name_buffer));
+  ASSERT_TRUE(iterator->Advance().FromMaybe(false));
+
+  ASSERT_TRUE(iterator->Done());
+}
+
+}  // namespace
+}  // namespace debug
+}  // namespace v8

test/unittests/runtime/runtime-debug-unittest.cc

+// Copyright 2021 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.
+
+#include "include/v8.h"
+#include "src/api/api.h"
+#include "src/objects/objects-inl.h"
+#include "src/runtime/runtime.h"
+#include "test/unittests/test-utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace v8 {
+namespace internal {
+namespace {
+
+using RuntimeTest = TestWithContext;
+
+TEST_F(RuntimeTest, ReturnsPrototype) {
+  TryCatch try_catch(isolate());
+
+  Local<v8::Object> object = v8::Object::New(isolate());
+  Handle<JSArray> i_result =
+      Runtime::GetInternalProperties(i_isolate(), Utils::OpenHandle(*object))
+          .ToHandleChecked();
+  Local<Array> result = Utils::ToLocal(i_result);
+  EXPECT_GE(result->Length(), 1u);
+
+  char name_buffer[100];
+  result->Get(context(), 0)
+      .ToLocalChecked()
+      .As<v8::String>()
+      ->WriteUtf8(isolate(), name_buffer);
+  EXPECT_EQ("[[Prototype]]", std::string(name_buffer));
+}
+
+bool AccessCheck(Local<v8::Context> accessing_context,
+                 Local<v8::Object> accessed_object, Local<Value> data) {
+  return false;
+}
+
+TEST_F(RuntimeTest, DoesNotReturnPrototypeWhenInacessible) {
+  TryCatch try_catch(isolate());
+
+  Local<ObjectTemplate> object_template = ObjectTemplate::New(isolate());
+  object_template->SetAccessCheckCallback(AccessCheck);
+
+  Local<v8::Object> object =
+      object_template->NewInstance(context()).ToLocalChecked();
+  Handle<JSArray> i_result =
+      Runtime::GetInternalProperties(i_isolate(), Utils::OpenHandle(*object))
+          .ToHandleChecked();
+  Local<Array> result = Utils::ToLocal(i_result);
+  EXPECT_EQ(0u, result->Length());
+}
+
+}  // namespace
+}  // namespace internal
+}  // namespace v8