Bug 965898 - Switch policies for get{,Own}PropertyDescriptor. r=gabor
authorBobby Holley <bobbyholley@gmail.com>
Wed, 30 Jul 2014 12:23:03 -0700
changeset 196848 a5b95c1ec2528af2da5c4517055bac74623ff43c
parent 196847 60dcc2593586391ef35aa365e4ecab8543d02421
child 196849 d099909ba007c821bf0e5d5d9ef11a0f004570c6
push id46984
push userbobbyholley@gmail.com
push dateWed, 30 Jul 2014 19:24:00 +0000
treeherdermozilla-inbound@22e1b7b69877 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor
bugs965898
milestone34.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 965898 - Switch policies for get{,Own}PropertyDescriptor. r=gabor
js/src/jsproxy.cpp
js/src/jsproxy.h
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/AccessCheck.h
js/xpconnect/wrappers/ChromeObjectWrapper.cpp
js/xpconnect/wrappers/FilteringWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -392,27 +392,27 @@ BaseProxyHandler::slice(JSContext *cx, H
 
     return js::SliceSlowly(cx, proxy, proxy, begin, end, result);
 }
 
 bool
 DirectProxyHandler::getPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
                                           MutableHandle<PropertyDescriptor> desc) const
 {
-    assertEnteredPolicy(cx, proxy, id, GET | SET);
+    assertEnteredPolicy(cx, proxy, id, GET | SET | GET_PROPERTY_DESCRIPTOR);
     JS_ASSERT(!hasPrototype()); // Should never be called if there's a prototype.
     RootedObject target(cx, proxy->as<ProxyObject>().target());
     return JS_GetPropertyDescriptorById(cx, target, id, desc);
 }
 
 bool
 DirectProxyHandler::getOwnPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
                                              MutableHandle<PropertyDescriptor> desc) const
 {
-    assertEnteredPolicy(cx, proxy, id, GET | SET);
+    assertEnteredPolicy(cx, proxy, id, GET | SET | GET_PROPERTY_DESCRIPTOR);
     RootedObject target(cx, proxy->as<ProxyObject>().target());
     return js::GetOwnPropertyDescriptor(cx, target, id, desc);
 }
 
 bool
 DirectProxyHandler::defineProperty(JSContext *cx, HandleObject proxy, HandleId id,
                                    MutableHandle<PropertyDescriptor> desc) const
 {
@@ -2190,17 +2190,17 @@ const ScriptedDirectProxyHandler Scripte
 
 bool
 Proxy::getPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
                              MutableHandle<PropertyDescriptor> desc)
 {
     JS_CHECK_RECURSION(cx, return false);
     const BaseProxyHandler *handler = proxy->as<ProxyObject>().handler();
     desc.object().set(nullptr); // default result if we refuse to perform this action
-    AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::GET, true);
+    AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::GET_PROPERTY_DESCRIPTOR, true);
     if (!policy.allowed())
         return policy.returnValue();
     if (!handler->hasPrototype())
         return handler->getPropertyDescriptor(cx, proxy, id, desc);
     if (!handler->getOwnPropertyDescriptor(cx, proxy, id, desc))
         return false;
     if (desc.object())
         return true;
@@ -2221,17 +2221,17 @@ Proxy::getPropertyDescriptor(JSContext *
 bool
 Proxy::getOwnPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
                                 MutableHandle<PropertyDescriptor> desc)
 {
     JS_CHECK_RECURSION(cx, return false);
 
     const BaseProxyHandler *handler = proxy->as<ProxyObject>().handler();
     desc.object().set(nullptr); // default result if we refuse to perform this action
-    AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::GET, true);
+    AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::GET_PROPERTY_DESCRIPTOR, true);
     if (!policy.allowed())
         return policy.returnValue();
     return handler->getOwnPropertyDescriptor(cx, proxy, id, desc);
 }
 
 bool
 Proxy::getOwnPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
                                 MutableHandleValue vp)
--- a/js/src/jsproxy.h
+++ b/js/src/jsproxy.h
@@ -150,27 +150,28 @@ class JS_FRIEND_API(BaseProxyHandler)
      * on the proxy's |id| property. In the case when |act| is CALL, |id| is
      * generally JSID_VOID.
      *
      * The |act| parameter to enter() specifies the action being performed.
      * If |bp| is false, the trap suggests that the caller throw (though it
      * may still decide to squelch the error).
      *
      * We make these OR-able so that assertEnteredPolicy can pass a union of them.
-     * For example, get{,Own}PropertyDescriptor is invoked by both calls to ::get()
-     * and ::set() (since we need to look up the accessor), so its
-     * assertEnteredPolicy would pass GET | SET.
+     * For example, get{,Own}PropertyDescriptor is invoked by calls to ::get()
+     * ::set(), in addition to being invoked on its own, so there are several
+     * valid Actions that could have been entered.
      */
     typedef uint32_t Action;
     enum {
         NONE      = 0x00,
         GET       = 0x01,
         SET       = 0x02,
         CALL      = 0x04,
-        ENUMERATE = 0x08
+        ENUMERATE = 0x08,
+        GET_PROPERTY_DESCRIPTOR = 0x10
     };
 
     virtual bool enter(JSContext *cx, HandleObject wrapper, HandleId id, Action act,
                        bool *bp) const;
 
     /* ES5 Harmony fundamental proxy traps. */
     virtual bool preventExtensions(JSContext *cx, HandleObject proxy) const = 0;
     virtual bool getPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -152,16 +152,23 @@ AccessCheck::isCrossOriginAccessPermitte
                                           Wrapper::Action act)
 {
     if (act == Wrapper::CALL)
         return false;
 
     if (act == Wrapper::ENUMERATE)
         return true;
 
+    // For the case of getting a property descriptor, we allow if either GET or SET
+    // is allowed, and rely on FilteringWrapper to filter out any disallowed accessors.
+    if (act == Wrapper::GET_PROPERTY_DESCRIPTOR) {
+        return isCrossOriginAccessPermitted(cx, wrapperArg, idArg, Wrapper::GET) ||
+               isCrossOriginAccessPermitted(cx, wrapperArg, idArg, Wrapper::SET);
+    }
+
     RootedId id(cx, idArg);
     RootedObject wrapper(cx, wrapperArg);
     RootedObject obj(cx, Wrapper::wrappedObject(wrapper));
 
     const char *name;
     const js::Class *clasp = js::GetObjectClass(obj);
     MOZ_ASSERT(!XrayUtils::IsXPCWNHolderClass(Jsvalify(clasp)), "shouldn't have a holder here");
     if (clasp->ext.innerObject)
@@ -207,16 +214,24 @@ ExposedPropertiesOnly::check(JSContext *
 {
     RootedObject wrapper(cx, wrapperArg);
     RootedId id(cx, idArg);
     RootedObject wrappedObject(cx, Wrapper::wrappedObject(wrapper));
 
     if (act == Wrapper::CALL)
         return true;
 
+
+    // For the case of getting a property descriptor, we allow if either GET or SET
+    // is allowed, and rely on FilteringWrapper to filter out any disallowed accessors.
+    if (act == Wrapper::GET_PROPERTY_DESCRIPTOR) {
+        return check(cx, wrapperArg, idArg, Wrapper::GET) ||
+               check(cx, wrapperArg, idArg, Wrapper::SET);
+    }
+
     RootedId exposedPropsId(cx, GetRTIdByIndex(cx, XPCJSRuntime::IDX_EXPOSEDPROPS));
 
     // We need to enter the wrappee's compartment to look at __exposedProps__,
     // but we want to be in the wrapper's compartment if we call Deny().
     //
     // Unfortunately, |cx| can be in either compartment when we call ::check. :-(
     JSAutoCompartment ac(cx, wrappedObject);
 
--- a/js/xpconnect/wrappers/AccessCheck.h
+++ b/js/xpconnect/wrappers/AccessCheck.h
@@ -74,18 +74,19 @@ struct CrossOriginAccessiblePropertiesOn
 };
 
 // This policy only permits access to properties if they appear in the
 // objects exposed properties list.
 struct ExposedPropertiesOnly : public Policy {
     static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act);
 
     static bool deny(js::Wrapper::Action act, JS::HandleId id) {
-        // Fail silently for GETs and ENUMERATEs.
-        return act == js::Wrapper::GET || act == js::Wrapper::ENUMERATE;
+        // Fail silently for GET ENUMERATE, and GET_PROPERTY_DESCRIPTOR.
+        return act == js::Wrapper::GET || act == js::Wrapper::ENUMERATE ||
+               act == js::Wrapper::GET_PROPERTY_DESCRIPTOR;
     }
     static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl) {
         return false;
     }
 };
 
 }
 
--- a/js/xpconnect/wrappers/ChromeObjectWrapper.cpp
+++ b/js/xpconnect/wrappers/ChromeObjectWrapper.cpp
@@ -72,17 +72,17 @@ PropIsFromStandardPrototype(JSContext *c
 }
 
 bool
 ChromeObjectWrapper::getPropertyDescriptor(JSContext *cx,
                                            HandleObject wrapper,
                                            HandleId id,
                                            JS::MutableHandle<JSPropertyDescriptor> desc) const
 {
-    assertEnteredPolicy(cx, wrapper, id, GET | SET);
+    assertEnteredPolicy(cx, wrapper, id, GET | SET | GET_PROPERTY_DESCRIPTOR);
     // First, try a lookup on the base wrapper if permitted.
     desc.object().set(nullptr);
     if (AllowedByBase(cx, wrapper, id, Wrapper::GET) &&
         !ChromeObjectWrapperBase::getPropertyDescriptor(cx, wrapper, id,
                                                         desc)) {
         return false;
     }
 
@@ -269,17 +269,18 @@ ChromeObjectWrapper::objectClassIs(Handl
 bool
 ChromeObjectWrapper::enter(JSContext *cx, HandleObject wrapper,
                            HandleId id, js::Wrapper::Action act, bool *bp) const
 {
     if (AllowedByBase(cx, wrapper, id, act))
         return true;
     // COWs fail silently for GETs, and that also happens to be the only case
     // where we might want to redirect the lookup to the home prototype chain.
-    *bp = act == Wrapper::GET || act == Wrapper::ENUMERATE;
+    *bp = act == Wrapper::GET || act == Wrapper::ENUMERATE ||
+          act == Wrapper::GET_PROPERTY_DESCRIPTOR;
     if (!*bp || id == JSID_VOID)
         return false;
 
     // Note that PropIsFromStandardPrototype needs to invoke getPropertyDescriptor
     // before we've fully entered the policy. Waive our policy.
     js::AutoWaivePolicy policy(cx, wrapper, id, act);
     return PropIsFromStandardPrototype(cx, wrapper, id);
 }
--- a/js/xpconnect/wrappers/FilteringWrapper.cpp
+++ b/js/xpconnect/wrappers/FilteringWrapper.cpp
@@ -75,29 +75,31 @@ FilterPropertyDescriptor(JSContext *cx, 
 }
 
 template <typename Base, typename Policy>
 bool
 FilteringWrapper<Base, Policy>::getPropertyDescriptor(JSContext *cx, HandleObject wrapper,
                                                       HandleId id,
                                                       JS::MutableHandle<JSPropertyDescriptor> desc) const
 {
-    assertEnteredPolicy(cx, wrapper, id, BaseProxyHandler::GET | BaseProxyHandler::SET);
+    assertEnteredPolicy(cx, wrapper, id, BaseProxyHandler::GET | BaseProxyHandler::SET |
+                                         BaseProxyHandler::GET_PROPERTY_DESCRIPTOR);
     if (!Base::getPropertyDescriptor(cx, wrapper, id, desc))
         return false;
     return FilterPropertyDescriptor<Policy>(cx, wrapper, id, desc);
 }
 
 template <typename Base, typename Policy>
 bool
 FilteringWrapper<Base, Policy>::getOwnPropertyDescriptor(JSContext *cx, HandleObject wrapper,
                                                          HandleId id,
                                                          JS::MutableHandle<JSPropertyDescriptor> desc) const
 {
-    assertEnteredPolicy(cx, wrapper, id, BaseProxyHandler::GET | BaseProxyHandler::SET);
+    assertEnteredPolicy(cx, wrapper, id, BaseProxyHandler::GET | BaseProxyHandler::SET |
+                                         BaseProxyHandler::GET_PROPERTY_DESCRIPTOR);
     if (!Base::getOwnPropertyDescriptor(cx, wrapper, id, desc))
         return false;
     return FilterPropertyDescriptor<Policy>(cx, wrapper, id, desc);
 }
 
 template <typename Base, typename Policy>
 bool
 FilteringWrapper<Base, Policy>::getOwnPropertyNames(JSContext *cx, HandleObject wrapper,
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -2359,17 +2359,18 @@ XrayWrapper<Base, Traits>::preventExtens
 }
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::getPropertyDescriptor(JSContext *cx, HandleObject wrapper, HandleId id,
                                                  JS::MutableHandle<JSPropertyDescriptor> desc)
                                                  const
 {
-    assertEnteredPolicy(cx, wrapper, id, BaseProxyHandler::GET | BaseProxyHandler::SET);
+    assertEnteredPolicy(cx, wrapper, id, BaseProxyHandler::GET | BaseProxyHandler::SET |
+                                         BaseProxyHandler::GET_PROPERTY_DESCRIPTOR);
     RootedObject holder(cx, Traits::singleton.ensureHolder(cx, wrapper));
     if (Traits::isResolving(cx, holder, id)) {
         desc.object().set(nullptr);
         return true;
     }
 
     typename Traits::ResolvingIdImpl resolving(cx, wrapper, id);
 
@@ -2491,17 +2492,18 @@ XrayWrapper<Base, Traits>::getPropertyDe
 }
 
 template <typename Base, typename Traits>
 bool
 XrayWrapper<Base, Traits>::getOwnPropertyDescriptor(JSContext *cx, HandleObject wrapper, HandleId id,
                                                     JS::MutableHandle<JSPropertyDescriptor> desc)
                                                     const
 {
-    assertEnteredPolicy(cx, wrapper, id, BaseProxyHandler::GET | BaseProxyHandler::SET);
+    assertEnteredPolicy(cx, wrapper, id, BaseProxyHandler::GET | BaseProxyHandler::SET |
+                                         BaseProxyHandler::GET_PROPERTY_DESCRIPTOR);
     RootedObject holder(cx, Traits::singleton.ensureHolder(cx, wrapper));
     if (Traits::isResolving(cx, holder, id)) {
         desc.object().set(nullptr);
         return true;
     }
 
     typename Traits::ResolvingIdImpl resolving(cx, wrapper, id);