Do not leak private property names to proxy traps and interceptors.

R=rossberg
BUG=

Review URL: https://codereview.chromium.org/1573143002

Cr-Commit-Position: refs/heads/master@{#33212}

src/lookup.cc

@@ -610,6 +610,7 @@ LookupIterator::State LookupIterator::LookupInHolder(Map* const map,
   switch (state_) {
     case NOT_FOUND:
       if (map->IsJSProxyMap()) {
+        // Do not leak private property names.
         if (!name_.is_null() && name_->IsPrivate()) return NOT_FOUND;
         return JSPROXY;
       }
@@ -625,6 +626,8 @@ LookupIterator::State LookupIterator::LookupInHolder(Map* const map,
       }
       if (check_interceptor() && HasInterceptor(map) &&
           !SkipInterceptor(JSObject::cast(holder))) {
+        // Do not leak private property names.
+        if (!name_.is_null() && name_->IsPrivate()) return NOT_FOUND;
         return INTERCEPTOR;
       }
     // Fall through.

src/messages.h

@@ -257,6 +257,7 @@ class CallSite {
   T(ProxyPreventExtensionsExtensible,                                          \
     "'preventExtensions' on proxy: trap returned truish but the proxy target " \
     "is extensible")                                                           \
+  T(ProxyPrivate, "Cannot pass private property name to proxy trap")           \
   T(ProxyRevoked, "Cannot perform '%' on a proxy that has been revoked")       \
   T(ProxySetFrozenData,                                                        \
     "'set' on proxy: trap returned truish for property '%' which exists in "   \

src/objects.cc

@@ -836,6 +836,7 @@ MaybeHandle<Object> JSProxy::GetProperty(Isolate* isolate,
         Object);
   }
 
+  DCHECK(!name->IsPrivate());
   STACK_CHECK(MaybeHandle<Object>());
   Handle<Name> trap_name = isolate->factory()->get_string();
   // 1. Assert: IsPropertyKey(P) is true.
@@ -4017,6 +4018,7 @@ Maybe<bool> JSObject::SetPropertyWithInterceptor(LookupIterator* it,
     result = args.Call(setter, index, v8::Utils::ToLocal(value));
   } else {
     Handle<Name> name = it->name();
+    DCHECK(!name->IsPrivate());
 
     if (name->IsSymbol() && !interceptor->can_intercept_symbols()) {
       return Just(false);
@@ -4157,6 +4159,10 @@ Maybe<bool> Object::SetProperty(LookupIterator* it, Handle<Object> value,
   if (found) return result;
   ShouldThrow should_throw =
       is_sloppy(language_mode) ? DONT_THROW : THROW_ON_ERROR;
+  if (it->GetReceiver()->IsJSProxy() && it->GetName()->IsPrivate()) {
+    RETURN_FAILURE(it->isolate(), should_throw,
+                   NewTypeError(MessageTemplate::kProxyPrivate));
+  }
   return AddDataProperty(it, value, NONE, should_throw, store_mode);
 }
 
@@ -4172,6 +4178,10 @@ Maybe<bool> Object::SetSuperProperty(LookupIterator* it, Handle<Object> value,
   Maybe<bool> result =
       SetPropertyInternal(it, value, language_mode, store_mode, &found);
   if (found) return result;
+  if (it->GetReceiver()->IsJSProxy() && it->GetName()->IsPrivate()) {
+    RETURN_FAILURE(isolate, should_throw,
+                   NewTypeError(MessageTemplate::kProxyPrivate));
+  }
 
   // The property either doesn't exist on the holder or exists there as a data
   // property.
@@ -4833,6 +4843,7 @@ void JSProxy::Revoke(Handle<JSProxy> proxy) {
 
 Maybe<bool> JSProxy::HasProperty(Isolate* isolate, Handle<JSProxy> proxy,
                                  Handle<Name> name) {
+  DCHECK(!name->IsPrivate());
   STACK_CHECK(Nothing<bool>());
   // 1. (Assert)
   // 2. Let handler be the value of the [[ProxyHandler]] internal slot of O.
@@ -4857,6 +4868,8 @@ Maybe<bool> JSProxy::HasProperty(Isolate* isolate, Handle<JSProxy> proxy,
     // 7a. Return target.[[HasProperty]](P).
     return JSReceiver::HasProperty(target, name);
   }
+  // Do not leak private property names.
+  if (name->IsPrivate()) return Just(false);
   // 8. Let booleanTrapResult be ToBoolean(? Call(trap, handler, «target, P»)).
   Handle<Object> trap_result_obj;
   Handle<Object> args[] = {target, name};
@@ -4900,6 +4913,7 @@ Maybe<bool> JSProxy::HasProperty(Isolate* isolate, Handle<JSProxy> proxy,
 Maybe<bool> JSProxy::SetProperty(Handle<JSProxy> proxy, Handle<Name> name,
                                  Handle<Object> value, Handle<Object> receiver,
                                  LanguageMode language_mode) {
+  DCHECK(!name->IsPrivate());
   Isolate* isolate = proxy->GetIsolate();
   STACK_CHECK(Nothing<bool>());
   Factory* factory = isolate->factory();
@@ -4968,6 +4982,7 @@ Maybe<bool> JSProxy::SetProperty(Handle<JSProxy> proxy, Handle<Name> name,
 Maybe<bool> JSProxy::DeletePropertyOrElement(Handle<JSProxy> proxy,
                                              Handle<Name> name,
                                              LanguageMode language_mode) {
+  DCHECK(!name->IsPrivate());
   ShouldThrow should_throw =
       is_sloppy(language_mode) ? DONT_THROW : THROW_ON_ERROR;
   Isolate* isolate = proxy->GetIsolate();
@@ -5388,6 +5403,7 @@ Maybe<PropertyAttributes> JSObject::GetPropertyAttributesWithInterceptor(
       result = args.Call(query, index);
     } else {
       Handle<Name> name = it->name();
+      DCHECK(!name->IsPrivate());
       v8::GenericNamedPropertyQueryCallback query =
           v8::ToCData<v8::GenericNamedPropertyQueryCallback>(
               interceptor->query());
@@ -5413,7 +5429,7 @@ Maybe<PropertyAttributes> JSObject::GetPropertyAttributesWithInterceptor(
       result = args.Call(getter, index);
     } else {
       Handle<Name> name = it->name();
-
+      DCHECK(!name->IsPrivate());
       v8::GenericNamedPropertyGetterCallback getter =
           v8::ToCData<v8::GenericNamedPropertyGetterCallback>(
               interceptor->getter());
@@ -6157,6 +6173,7 @@ Maybe<bool> JSObject::DeletePropertyWithInterceptor(LookupIterator* it) {
     return Nothing<bool>();
   } else {
     Handle<Name> name = it->name();
+    DCHECK(!name->IsPrivate());
     v8::GenericNamedPropertyDeleterCallback deleter =
         v8::ToCData<v8::GenericNamedPropertyDeleterCallback>(
             interceptor->deleter());
@@ -6212,6 +6229,11 @@ Maybe<bool> JSReceiver::DeleteProperty(LookupIterator* it,
                                             it->GetName(), language_mode);
   }
 
+  if (it->GetReceiver()->IsJSProxy()) {
+    DCHECK(it->state() == LookupIterator::NOT_FOUND);
+    DCHECK(it->GetName()->IsPrivate());
+    return Just(true);
+  }
   Handle<JSObject> receiver = Handle<JSObject>::cast(it->GetReceiver());
 
   bool is_observed =
@@ -7088,6 +7110,11 @@ Maybe<bool> JSProxy::DefineOwnProperty(Isolate* isolate, Handle<JSProxy> proxy,
       key->IsName()
           ? Handle<Name>::cast(key)
           : Handle<Name>::cast(isolate->factory()->NumberToString(key));
+  // Do not leak private property names.
+  if (property_name->IsPrivate()) {
+    RETURN_FAILURE(isolate, should_throw,
+                   NewTypeError(MessageTemplate::kProxyPrivate));
+  }
   Handle<Object> trap_result_obj;
   Handle<Object> args[] = {target, property_name, desc_obj};
   ASSIGN_RETURN_ON_EXCEPTION_VALUE(
@@ -7232,7 +7259,9 @@ Maybe<bool> JSProxy::GetOwnPropertyDescriptor(Isolate* isolate,
                                               Handle<JSProxy> proxy,
                                               Handle<Name> name,
                                               PropertyDescriptor* desc) {
+  DCHECK(!name->IsPrivate());
   STACK_CHECK(Nothing<bool>());
+
   Handle<String> trap_name =
       isolate->factory()->getOwnPropertyDescriptor_string();
   // 1. (Assert)
@@ -9979,7 +10008,7 @@ Handle<DescriptorArray> DescriptorArray::CopyUpToAddAttributes(
       Name* key = desc->GetKey(i);
       PropertyDetails details = desc->GetDetails(i);
       // Bulk attribute changes never affect private properties.
-      if (!key->IsSymbol() || !Symbol::cast(key)->is_private()) {
+      if (!key->IsPrivate()) {
         int mask = DONT_DELETE | DONT_ENUM;
         // READ_ONLY is an invalid attribute for JS setters/getters.
         if (details.type() != ACCESSOR_CONSTANT || !value->IsAccessorPair()) {
@@ -16144,6 +16173,7 @@ MaybeHandle<Object> JSObject::GetPropertyWithInterceptor(LookupIterator* it,
     result = args.Call(getter, index);
   } else {
     Handle<Name> name = it->name();
+    DCHECK(!name->IsPrivate());
 
     if (name->IsSymbol() && !interceptor->can_intercept_symbols()) {
       return isolate->factory()->undefined_value();

test/mjsunit/harmony/private-symbols.js

+// Copyright 2016 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.
+
+// Flags: --harmony-proxies --harmony-reflect --allow-natives-syntax
+
+
+var symbol = %CreatePrivateSymbol("private");
+
+
+// Private symbols must never be listed.
+
+var object = {};
+object[symbol] = 42;
+for (var key of Object.keys(object)) assertUnreachable();
+for (var key of Object.getOwnPropertySymbols(object)) assertUnreachable();
+for (var key of Object.getOwnPropertyNames(object)) assertUnreachable();
+for (var key of Reflect.ownKeys(object)) assertUnreachable();
+for (var key of Reflect.enumerate(object)) assertUnreachable();
+for (var key in object) assertUnreachable();
+
+var object2 = {__proto__: object};
+for (var key of Object.keys(object2)) assertUnreachable();
+for (var key of Object.getOwnPropertySymbols(object2)) assertUnreachable();
+for (var key of Object.getOwnPropertyNames(object2)) assertUnreachable();
+for (var key of Reflect.ownKeys(object2)) assertUnreachable();
+for (var key of Reflect.enumerate(object2)) assertUnreachable();
+for (var key in object2) assertUnreachable();
+
+
+// Private symbols must never leak to proxy traps.
+
+var proxy = new Proxy({}, new Proxy({}, {get() {return () => {throw 666}}}));
+var object = {__proto__: proxy};
+
+// [[Set]]
+assertEquals(42, proxy[symbol] = 42);
+assertThrows(function() { "use strict"; proxy[symbol] = 42 }, TypeError);
+assertEquals(false, Reflect.set(proxy, symbol, 42));
+assertEquals(42, object[symbol] = 42);
+assertEquals(43, (function() {"use strict"; return object[symbol] = 43})());
+assertEquals(true, Reflect.set(object, symbol, 44));
+
+// [[DefineOwnProperty]]
+assertEquals(false, Reflect.defineProperty(proxy, symbol, {}));
+assertThrows(() => Object.defineProperty(proxy, symbol, {}), TypeError);
+assertEquals(true, Reflect.defineProperty(object, symbol, {}));
+assertEquals(object, Object.defineProperty(object, symbol, {}));
+
+// [[Delete]]
+assertEquals(true, delete proxy[symbol]);
+assertEquals(true, (function() {"use strict"; return delete proxy[symbol]})());
+assertEquals(true, Reflect.deleteProperty(proxy, symbol));
+assertEquals(true, delete object[symbol]);
+assertEquals(true, (function() {"use strict"; return delete object[symbol]})());
+assertEquals(true, Reflect.deleteProperty(object, symbol));
+
+// [[GetOwnPropertyDescriptor]]
+assertEquals(undefined, Object.getOwnPropertyDescriptor(proxy, symbol));
+assertEquals(undefined, Reflect.getOwnPropertyDescriptor(proxy, symbol));
+assertFalse(Object.prototype.hasOwnProperty.call(proxy, symbol));
+assertEquals(undefined, Object.getOwnPropertyDescriptor(object, symbol));
+assertEquals(undefined, Reflect.getOwnPropertyDescriptor(object, symbol));
+assertFalse(Object.prototype.hasOwnProperty.call(object, symbol));
+
+// [[Has]]
+assertFalse(symbol in proxy);
+assertFalse(Reflect.has(proxy, symbol));
+assertFalse(symbol in object);
+assertFalse(Reflect.has(object, symbol));
+
+// [[Get]]
+assertEquals(undefined, proxy[symbol]);
+assertEquals(undefined, Reflect.get(proxy, symbol));
+assertEquals(undefined, Reflect.get(proxy, symbol, 42));
+assertEquals(undefined, object[symbol]);
+assertEquals(undefined, Reflect.get(object, symbol));
+assertEquals(undefined, Reflect.get(object, symbol, 42));