Bug 1081990 - Turn off COWs for Functions. r=gabor
authorBobby Holley <bobbyholley@gmail.com>
Sat, 18 Oct 2014 11:02:10 +0200
changeset 211113 576bab9d7f4cb6ec00188939b4046dd925cbb8ad
parent 211112 712da524ebdd413c8dabf493505867f55973e1b7
child 211114 2681f9b134c297bc70fe04ea8efb7f7ba9ede4d0
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersgabor
bugs1081990
milestone36.0a1
Bug 1081990 - Turn off COWs for Functions. r=gabor
content/base/test/chrome/cpows_child.js
js/xpconnect/tests/chrome/test_cows.xul
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/AccessCheck.h
js/xpconnect/wrappers/WrapperFactory.cpp
--- a/content/base/test/chrome/cpows_child.js
+++ b/content/base/test/chrome/cpows_child.js
@@ -147,17 +147,22 @@ function compartment_test()
     let results = [];
     function is(a, b, msg) { results.push({ result: a === b ? "PASS" : "FAIL", message: msg }) };
     function ok(x, msg) { results.push({ result: x ? "PASS" : "FAIL", message: msg }) };
 
     let cpowLocation = Cu.getCompartmentLocation(obj);
     ok(/Privileged Junk/.test(cpowLocation),
        "child->parent CPOWs should live in the privileged junk scope: " + cpowLocation);
     is(obj(), 42, "child->parent CPOW is invokable");
-    is(obj.expando, undefined, "child->parent CPOW cannot access properties");
+    try {
+      obj.expando;
+      ok(false, "child->parent CPOW cannot access properties");
+    } catch (e) {
+      ok(true, "child->parent CPOW cannot access properties");
+    }
 
     return results;
   }
   sendSyncMessage("cpows:compartment_test", {}, { getUnprivilegedObject: sb.getUnprivilegedObject,
                                                   testParentObject: testParentObject });
 }
 
 function regexp_test()
--- a/js/xpconnect/tests/chrome/test_cows.xul
+++ b/js/xpconnect/tests/chrome/test_cows.xul
@@ -96,65 +96,28 @@ function COWTests() {
     var nonempty = {foo: 42, bar: 33};
     is(getCOW(empty).foo, undefined,
        "shouldn't throw when accessing exposed properties that doesn't exist");
 
     PROPS_TO_TEST.forEach(function(name) {
         isPropHidden(getCOW(nonempty), name, "object without exposedProps");
     });
 
-    // Test function objects without __exposedProps__
+    // Test function objects.
     var func = function(x) { return 42; };
+    func.__exposedProps__ = { foo: "r" };
     func.foo = "foo property";
     var funcCOW = getCOW(func);
-    PROPS_TO_TEST.forEach(function(name) {
-        isPropHidden(funcCOW, name, "function without exposedProps");
-    });
-    is([name for (name in funcCOW)].length, 0,
-       "function without exposedProps shouldn't have any properties");
-    is(funcCOW(), 42, "COW without exposedProps should still be callable");
-
-    // Test function objects with __exposedProps__
-    var func = function(x) { return 42; };
-    func.foo = "foo property";
-    func.__exposedProps__ = { foo: "r" };
-    var funcCOWr = getCOW(func);
-    PROPS_TO_TEST.forEach(function(name) {
-        if (name == "foo") {
-            isProp(funcCOWr, name, "foo property",
-                   "function with exposedProps");
-        }
-        else {
-            isPropHidden(funcCOWr, name, "function with exposedProps");
-        }
-    });
-    is([name for (name in funcCOWr)].length, 1,
-       "function with exposedProps should only enumerate exposed props");
-    is([name for (name in funcCOWr)][0], "foo",
-       "function with exposedProps should only enumerate exposed props");
-    is(funcCOWr(), 42, "COW with exposedProps should be callable");
-
-    // Test function objects with __exposedProps__
-    var func = function(x) { return 42; };
-    func.foo = "foo property";
-    func.__exposedProps__ = { foo: "r", bar: "r" };
-    var funcCOWr2 = getCOW(func);
-    PROPS_TO_TEST.forEach(function(name) {
-        if (name == "foo") {
-            isProp(funcCOWr2, name, "foo property",
-                   "function with more exposedProps");
-        }
-        else {
-            isPropHidden(funcCOWr2, name, "function with more exposedProps");
-        }
-    });
-    is([name for (name in funcCOWr2)].length, 1,
-       "function with exposedProps should only enumerate exposed props");
-    is([name for (name in funcCOWr2)][0], "foo",
-       "function with exposedProps should only enumerate exposed props");
+    try {
+      funcCOW.foo;
+      ok(false, 'Functions are no longer COWable');
+    } catch (e) {
+      ok(/denied|insecure/.test(e), 'Functions are no longer COWable');
+    }
+    is(funcCOW(), 42, "Chrome functions should be callable");
 
     // Test object with empty exposedProps
     var strict = { __exposedProps__: {}, foo: "foo property" };
     var strictCOW = getCOW(strict);
     PROPS_TO_TEST.forEach(function(name) {
         isPropHidden(strictCOW, name, "object with empty exposedProps");
     });
     is([name for (name in strictCOW)].length, 0,
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -223,18 +223,17 @@ EnterAndThrow(JSContext *cx, JSObject *w
 }
 
 bool
 ExposedPropertiesOnly::check(JSContext *cx, HandleObject wrapper, HandleId id, Wrapper::Action act)
 {
     RootedObject wrappedObject(cx, Wrapper::wrappedObject(wrapper));
 
     if (act == Wrapper::CALL)
-        return true;
-
+        return false;
 
     // 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, wrapper, id, Wrapper::GET) ||
                check(cx, wrapper, id, Wrapper::SET);
     }
 
--- a/js/xpconnect/wrappers/AccessCheck.h
+++ b/js/xpconnect/wrappers/AccessCheck.h
@@ -102,17 +102,13 @@ struct ExposedPropertiesOnly : public Po
     static const bool AllowGetPrototypeOf = true;
 
     static bool check(JSContext *cx, JS::HandleObject wrapper, JS::HandleId id, js::Wrapper::Action act);
 
     static bool deny(js::Wrapper::Action act, JS::HandleId id);
     static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl) {
         return false;
     }
-    static bool checkCall(JSContext *cx, JS::HandleObject wrapper, const JS::CallArgs &args) {
-        // XXXbholley - This goes away in the next patch.
-        return AccessCheck::checkPassToPrivilegedCode(cx, wrapper, args);
-    }
 };
 
 }
 
 #endif /* __AccessCheck_h__ */
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -108,30 +108,17 @@ WrapperFactory::WaiveXray(JSContext *cx,
 }
 
 // In general, we're trying to deprecate COWs incrementally as we introduce
 // Xrays to the corresponding object types. But switching off COWs for certain
 // things would be too tumultuous at present, so we punt on them for later.
 static bool
 ForceCOWBehavior(JSObject *obj)
 {
-    JSProtoKey key = IdentifyStandardInstanceOrPrototype(obj);
-    if (key == JSProto_Function && GetXrayType(obj) == XrayForDOMObject) {
-        // This means that we've got a DOM constructor, which we never want to
-        // expose COW-style.
-        return false;
-    }
-    if (key == JSProto_Object || key == JSProto_Function) {
-        MOZ_ASSERT(GetXrayType(obj) == XrayForJSObject,
-                   "We should use XrayWrappers for standard ES Object, Array, and Function "
-                   "instances modulo this hack");
-        return true;
-    }
-
-    return false;
+    return IdentifyStandardInstanceOrPrototype(obj) == JSProto_Object;
 }
 
 inline bool
 ShouldWaiveXray(JSContext *cx, JSObject *originalObj)
 {
     unsigned flags;
     (void) js::UncheckedUnwrap(originalObj, /* stopAtOuter = */ true, &flags);
 
@@ -478,16 +465,24 @@ WrapperFactory::Rewrap(JSContext *cx, Ha
     }
 
     // Let the SpecialPowers scope make its stuff easily accessible to content.
     else if (CompartmentPrivate::Get(origin)->forcePermissiveCOWs) {
         CrashIfNotInAutomation();
         wrapper = &CrossCompartmentWrapper::singleton;
     }
 
+    // If this is a chrome function being exposed to content, we need to allow
+    // call (but nothing else).
+    else if (originIsChrome && !targetIsChrome &&
+             IdentifyStandardInstance(obj) == JSProto_Function)
+    {
+        wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper, OpaqueWithCall>::singleton;
+    }
+
     // If this is a chrome object being exposed to content without Xrays, use
     // a COW.
     //
     // We make an exception for Object instances, because we still rely on COWs
     // for those in a lot of places in the tree.
     else if (originIsChrome && !targetIsChrome &&
              (xrayType == NotXray || ForceCOWBehavior(obj)))
     {