Bug 1515590 part 1 - Fix incorrect SandboxProxyHandler::getOwnPropertyDescriptor implementation. r=bzbarsky

The code was trying to implement the getOwnPropertyDescriptor trap in terms of
getPropertyDescriptor, by comparing the "holder" object we found to the object
we did the lookup on. This becomes buggy when wrappers like WindowProxy are involved.

The patch removes this check and calls JS_GetOwnPropertyDescriptor instead of
JS_GetPropertyDescriptor.

Differential Revision: https://phabricator.services.mozilla.com/D15091

--HG--
extra : moz-landing-system : lando

js/xpconnect/src/Sandbox.cpp

@@ -515,6 +515,13 @@ class SandboxProxyHandler : public js::Wrapper {
       JS::AutoIdVector& props) const override;
   virtual JSObject* enumerate(JSContext* cx,
                               JS::Handle<JSObject*> proxy) const override;
+
+ private:
+  // Implements the custom getPropertyDescriptor behavior. If the getOwn
+  // argument is true we only look for "own" properties.
+  bool getPropertyDescriptorImpl(
+      JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
+      bool getOwn, JS::MutableHandle<JS::PropertyDescriptor> desc) const;
 };
 
 static const SandboxProxyHandler sandboxProxyHandler;
@@ -668,14 +675,21 @@ static bool IsMaybeWrappedDOMConstructor(JSObject* obj) {
   return dom::IsDOMConstructor(obj);
 }
 
-bool SandboxProxyHandler::getPropertyDescriptor(
+bool SandboxProxyHandler::getPropertyDescriptorImpl(
     JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
-    JS::MutableHandle<PropertyDescriptor> desc) const {
+    bool getOwn, JS::MutableHandle<PropertyDescriptor> desc) const {
   JS::RootedObject obj(cx, wrappedObject(proxy));
 
   MOZ_ASSERT(js::GetObjectCompartment(obj) == js::GetObjectCompartment(proxy));
-  if (!JS_GetPropertyDescriptorById(cx, obj, id, desc)) {
-    return false;
+
+  if (getOwn) {
+    if (!JS_GetOwnPropertyDescriptorById(cx, obj, id, desc)) {
+      return false;
+    }
+  } else {
+    if (!JS_GetPropertyDescriptorById(cx, obj, id, desc)) {
+      return false;
+    }
   }
 
   if (!desc.object()) {
@@ -707,18 +721,16 @@ bool SandboxProxyHandler::getPropertyDescriptor(
   return true;
 }
 
+bool SandboxProxyHandler::getPropertyDescriptor(
+    JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
+    JS::MutableHandle<PropertyDescriptor> desc) const {
+  return getPropertyDescriptorImpl(cx, proxy, id, /* getOwn = */ false, desc);
+}
+
 bool SandboxProxyHandler::getOwnPropertyDescriptor(
     JSContext* cx, JS::Handle<JSObject*> proxy, JS::Handle<jsid> id,
     JS::MutableHandle<PropertyDescriptor> desc) const {
-  if (!getPropertyDescriptor(cx, proxy, id, desc)) {
-    return false;
-  }
-
-  if (desc.object() != wrappedObject(proxy)) {
-    desc.object().set(nullptr);
-  }
-
-  return true;
+  return getPropertyDescriptorImpl(cx, proxy, id, /* getOwn = */ true, desc);
 }
 
 /*