Bug 1582520: Part 2 - Fix RemoteObjectProxy hasOwnProperty hook behavior. r=bzbarsky
authorKris Maglione <maglione.k@gmail.com>
Wed, 25 Sep 2019 17:49:57 +0000
changeset 494943 56d226cfe63c1d66955b70137e54ee6b30c43d66
parent 494942 05dd1a3de4ccff296984df22a5c1bfb578c6a275
child 494944 aa9059b3f9b0550ee66625b8409bc7c3119e3ed1
push id114131
push userdluca@mozilla.com
push dateThu, 26 Sep 2019 09:47:34 +0000
treeherdermozilla-inbound@1dc1a755079a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1582520
milestone71.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1582520: Part 2 - Fix RemoteObjectProxy hasOwnProperty hook behavior. r=bzbarsky Object.hasOwnProperty called on a cross-origin object needs to return true for any property returned by its property enumerator or get hook, and throw a security error for anything else. Ordinary cross-origin objects currently behave correctly, but RemoteObjectProxy objects return false for indexed frame getters, and never throw security exceptions for inaccessible properties. This patch fixes both of those issues by removing the `hasOwn` hook and falling back to the BaseProxyHandler implementation, which defers to the `getOwnPropertyDescriptor` hook. This is slightly more expensive, since it requires reifying property descriptors for every check, but it should be a relatively uncommon operation on cross-origin objects, and should not be particularly expensive for any properties of a RemoteObjectProxy. Differential Revision: https://phabricator.services.mozilla.com/D46734
dom/bindings/RemoteObjectProxy.cpp
dom/bindings/RemoteObjectProxy.h
--- a/dom/bindings/RemoteObjectProxy.cpp
+++ b/dom/bindings/RemoteObjectProxy.cpp
@@ -2,16 +2,17 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "RemoteObjectProxy.h"
 #include "AccessCheck.h"
 #include "jsfriendapi.h"
+#include "xpcprivate.h"
 
 namespace mozilla {
 namespace dom {
 
 bool RemoteObjectProxyBase::getOwnPropertyDescriptor(
     JSContext* aCx, JS::Handle<JSObject*> aProxy, JS::Handle<jsid> aId,
     JS::MutableHandle<JS::PropertyDescriptor> aDesc) const {
   bool ok = CrossOriginGetOwnPropertyHelper(aCx, aProxy, aId, aDesc);
@@ -137,31 +138,16 @@ bool RemoteObjectProxyBase::get(JSContex
 bool RemoteObjectProxyBase::set(JSContext* aCx, JS::Handle<JSObject*> aProxy,
                                 JS::Handle<jsid> aId,
                                 JS::Handle<JS::Value> aValue,
                                 JS::Handle<JS::Value> aReceiver,
                                 JS::ObjectOpResult& aResult) const {
   return CrossOriginSet(aCx, aProxy, aId, aValue, aReceiver, aResult);
 }
 
-bool RemoteObjectProxyBase::hasOwn(JSContext* aCx, JS::Handle<JSObject*> aProxy,
-                                   JS::Handle<jsid> aId, bool* aBp) const {
-  JS::Rooted<JSObject*> holder(aCx);
-  if (!EnsureHolder(aCx, aProxy, &holder) ||
-      !JS_AlreadyHasOwnPropertyById(aCx, holder, aId, aBp)) {
-    return false;
-  }
-
-  if (!*aBp) {
-    *aBp = xpc::IsCrossOriginWhitelistedProp(aCx, aId);
-  }
-
-  return true;
-}
-
 bool RemoteObjectProxyBase::getOwnEnumerablePropertyKeys(
     JSContext* aCx, JS::Handle<JSObject*> aProxy,
     JS::MutableHandleVector<jsid> aProps) const {
   return true;
 }
 
 const char* RemoteObjectProxyBase::className(
     JSContext* aCx, JS::Handle<JSObject*> aProxy) const {
--- a/dom/bindings/RemoteObjectProxy.h
+++ b/dom/bindings/RemoteObjectProxy.h
@@ -62,18 +62,16 @@ class RemoteObjectProxyBase : public js:
   bool get(JSContext* cx, JS::Handle<JSObject*> aProxy,
            JS::Handle<JS::Value> aReceiver, JS::Handle<jsid> aId,
            JS::MutableHandle<JS::Value> aVp) const final;
   bool set(JSContext* cx, JS::Handle<JSObject*> aProxy, JS::Handle<jsid> aId,
            JS::Handle<JS::Value> aValue, JS::Handle<JS::Value> aReceiver,
            JS::ObjectOpResult& aResult) const final;
 
   // SpiderMonkey extensions
-  bool hasOwn(JSContext* aCx, JS::Handle<JSObject*> aProxy,
-              JS::Handle<jsid> aId, bool* aBp) const override;
   bool getOwnEnumerablePropertyKeys(
       JSContext* aCx, JS::Handle<JSObject*> aProxy,
       JS::MutableHandleVector<jsid> aProps) const override;
   const char* className(JSContext* aCx,
                         JS::Handle<JSObject*> aProxy) const final;
 
   bool isCallable(JSObject* aObj) const final { return false; }
   bool isConstructor(JSObject* aObj) const final { return false; }