Bug 1481467 part 2 - Use JSAutoRealm instead of JSAutoRealmAllowCCW in JSXrayTraits::getOwnPropertyFromTargetIfSafe. r=bz
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 08 Aug 2018 15:12:16 +0200
changeset 430613 8b38554d067fb3b071b19f630219a385ce6241b3
parent 430612 980f42a5b1bc4ee20ec33650c6cc598d865e357d
child 430614 dc3ee665001f01a78281fdf2ccf8abf1c7852065
push id106201
push userjandemooij@gmail.com
push dateWed, 08 Aug 2018 13:14:54 +0000
treeherdermozilla-inbound@dc3ee665001f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1481467
milestone63.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 1481467 part 2 - Use JSAutoRealm instead of JSAutoRealmAllowCCW in JSXrayTraits::getOwnPropertyFromTargetIfSafe. r=bz Because getOwnPropertyFromTargetIfSafe operates in the Xray target realm/compartment and we cannot use the Xray wrapper with JSAutoRealm, we pass the caller's global as wrapperGlobal and use that.
js/xpconnect/wrappers/XrayWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.h
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -272,84 +272,88 @@ ReportWrapperDenial(JSContext* cx, Handl
 
 bool JSXrayTraits::getOwnPropertyFromWrapperIfSafe(JSContext* cx,
                                                    HandleObject wrapper,
                                                    HandleId id,
                                                    MutableHandle<PropertyDescriptor> outDesc)
 {
     MOZ_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
     RootedObject target(cx, getTargetObject(wrapper));
+    RootedObject wrapperGlobal(cx, JS::CurrentGlobalOrNull(cx));
     {
         JSAutoRealm ar(cx, target);
         JS_MarkCrossZoneId(cx, id);
-        if (!getOwnPropertyFromTargetIfSafe(cx, target, wrapper, id, outDesc))
+        if (!getOwnPropertyFromTargetIfSafe(cx, target, wrapper, wrapperGlobal, id, outDesc))
             return false;
     }
     return JS_WrapPropertyDescriptor(cx, outDesc);
 }
 
 bool JSXrayTraits::getOwnPropertyFromTargetIfSafe(JSContext* cx,
                                                   HandleObject target,
                                                   HandleObject wrapper,
+                                                  HandleObject wrapperGlobal,
                                                   HandleId id,
                                                   MutableHandle<PropertyDescriptor> outDesc)
 {
     // Note - This function operates in the target compartment, because it
     // avoids a bunch of back-and-forth wrapping in enumerateNames.
     MOZ_ASSERT(getTargetObject(wrapper) == target);
     MOZ_ASSERT(js::IsObjectInContextCompartment(target, cx));
     MOZ_ASSERT(WrapperFactory::IsXrayWrapper(wrapper));
+    MOZ_ASSERT(JS_IsGlobalObject(wrapperGlobal));
+    js::AssertSameCompartment(wrapper, wrapperGlobal);
     MOZ_ASSERT(outDesc.object() == nullptr);
 
     Rooted<PropertyDescriptor> desc(cx);
     if (!JS_GetOwnPropertyDescriptorById(cx, target, id, &desc))
         return false;
 
     // If the property doesn't exist at all, we're done.
     if (!desc.object())
         return true;
 
     // Disallow accessor properties.
     if (desc.hasGetterOrSetter()) {
-        JSAutoRealmAllowCCW ar(cx, wrapper);
+        JSAutoRealm ar(cx, wrapperGlobal);
         JS_MarkCrossZoneId(cx, id);
         return ReportWrapperDenial(cx, id, WrapperDenialForXray, "property has accessor");
     }
 
     // Apply extra scrutiny to objects.
     if (desc.value().isObject()) {
         RootedObject propObj(cx, js::UncheckedUnwrap(&desc.value().toObject()));
         JSAutoRealm ar(cx, propObj);
 
         // Disallow non-subsumed objects.
         if (!AccessCheck::subsumes(target, propObj)) {
-            JSAutoRealmAllowCCW ar(cx, wrapper);
+            JSAutoRealm ar(cx, wrapperGlobal);
             JS_MarkCrossZoneId(cx, id);
             return ReportWrapperDenial(cx, id, WrapperDenialForXray, "value not same-origin with target");
         }
 
         // Disallow non-Xrayable objects.
         XrayType xrayType = GetXrayType(propObj);
         if (xrayType == NotXray || xrayType == XrayForOpaqueObject) {
-            JSAutoRealmAllowCCW ar(cx, wrapper);
+            JSAutoRealm ar(cx, wrapperGlobal);
             JS_MarkCrossZoneId(cx, id);
             return ReportWrapperDenial(cx, id, WrapperDenialForXray, "value not Xrayable");
         }
 
         // Disallow callables.
         if (JS::IsCallable(propObj)) {
-            JSAutoRealmAllowCCW ar(cx, wrapper);
+            JSAutoRealm ar(cx, wrapperGlobal);
             JS_MarkCrossZoneId(cx, id);
             return ReportWrapperDenial(cx, id, WrapperDenialForXray, "value is callable");
         }
     }
 
     // Disallow any property that shadows something on its (Xrayed)
     // prototype chain.
-    JSAutoRealmAllowCCW ar2(cx, wrapper);
+    JSAutoRealm ar2(cx, wrapperGlobal);
     JS_MarkCrossZoneId(cx, id);
     RootedObject proto(cx);
     bool foundOnProto = false;
     if (!JS_GetPrototype(cx, wrapper, &proto) ||
         (proto && !JS_HasPropertyById(cx, proto, id, &foundOnProto)))
     {
         return false;
     }
@@ -654,32 +658,35 @@ JSXrayTraits::resolveOwnProperty(JSConte
     }
 
     return true;
 }
 
 bool
 JSXrayTraits::delete_(JSContext* cx, HandleObject wrapper, HandleId id, ObjectOpResult& result)
 {
+    MOZ_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
+
     RootedObject holder(cx, ensureHolder(cx, wrapper));
     if (!holder)
         return false;
 
     // If we're using Object Xrays, we allow callers to attempt to delete any
     // property from the underlying object that they are able to resolve. Note
     // that this deleting may fail if the property is non-configurable.
     JSProtoKey key = getProtoKey(holder);
     bool isObjectOrArrayInstance = (key == JSProto_Object || key == JSProto_Array) &&
                                    !isPrototype(holder);
     if (isObjectOrArrayInstance) {
+        RootedObject wrapperGlobal(cx, JS::CurrentGlobalOrNull(cx));
         RootedObject target(cx, getTargetObject(wrapper));
         JSAutoRealm ar(cx, target);
         JS_MarkCrossZoneId(cx, id);
         Rooted<PropertyDescriptor> desc(cx);
-        if (!getOwnPropertyFromTargetIfSafe(cx, target, wrapper, id, &desc))
+        if (!getOwnPropertyFromTargetIfSafe(cx, target, wrapper, wrapperGlobal, id, &desc))
             return false;
         if (desc.object())
             return JS_DeletePropertyById(cx, target, id, result);
     }
     return result.succeed();
 }
 
 bool
@@ -794,41 +801,47 @@ AppendNamesFromFunctionAndPropertySpecs(
 
     return true;
 }
 
 bool
 JSXrayTraits::enumerateNames(JSContext* cx, HandleObject wrapper, unsigned flags,
                              AutoIdVector& props)
 {
+    MOZ_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
+
     RootedObject target(cx, getTargetObject(wrapper));
     RootedObject holder(cx, ensureHolder(cx, wrapper));
     if (!holder)
         return false;
 
     JSProtoKey key = getProtoKey(holder);
     if (!isPrototype(holder)) {
         // For Object and Array instances, we expose some properties from the underlying
         // object, but only after filtering them carefully.
         if (key == JSProto_Object || key == JSProto_Array) {
             MOZ_ASSERT(props.empty());
+            RootedObject wrapperGlobal(cx, JS::CurrentGlobalOrNull(cx));
             {
                 JSAutoRealm ar(cx, target);
                 AutoIdVector targetProps(cx);
                 if (!js::GetPropertyKeys(cx, target, flags | JSITER_OWNONLY, &targetProps))
                     return false;
                 // Loop over the properties, and only pass along the ones that
                 // we determine to be safe.
                 if (!props.reserve(targetProps.length()))
                     return false;
                 for (size_t i = 0; i < targetProps.length(); ++i) {
                     Rooted<PropertyDescriptor> desc(cx);
                     RootedId id(cx, targetProps[i]);
-                    if (!getOwnPropertyFromTargetIfSafe(cx, target, wrapper, id, &desc))
+                    if (!getOwnPropertyFromTargetIfSafe(cx, target, wrapper, wrapperGlobal, id,
+                                                        &desc))
+                    {
                         return false;
+                    }
                     if (desc.object())
                         props.infallibleAppend(id);
                 }
             }
             for (size_t i = 0; i < props.length(); ++i)
                 JS_MarkCrossZoneId(cx, props[i]);
             return true;
         }
--- a/js/xpconnect/wrappers/XrayWrapper.h
+++ b/js/xpconnect/wrappers/XrayWrapper.h
@@ -262,20 +262,22 @@ public:
     }
 
     // Operates in the wrapper compartment.
     static bool getOwnPropertyFromWrapperIfSafe(JSContext* cx,
                                                 JS::HandleObject wrapper,
                                                 JS::HandleId id,
                                                 JS::MutableHandle<JS::PropertyDescriptor> desc);
 
-    // Like the above, but operates in the target compartment.
+    // Like the above, but operates in the target compartment. wrapperGlobal is
+    // the caller's global (must be in the wrapper compartment).
     static bool getOwnPropertyFromTargetIfSafe(JSContext* cx,
                                                JS::HandleObject target,
                                                JS::HandleObject wrapper,
+                                               JS::HandleObject wrapperGlobal,
                                                JS::HandleId id,
                                                JS::MutableHandle<JS::PropertyDescriptor> desc);
 
     static const JSClass HolderClass;
     static JSXrayTraits singleton;
 };
 
 // These traits are used when the target is not Xrayable and we therefore want