Bug 1032457 - Implement the |allowCallbacks| parameter to exportFunction. r=gabor

js/xpconnect/src/ExportHelpers.cpp

@@ -79,7 +79,8 @@ StackScopedCloneRead(JSContext *cx, JSStructuredCloneReader *reader, uint32_t ta
       if (!JS_WrapObject(cx, &obj))
           return nullptr;
 
-      if (!xpc::NewFunctionForwarder(cx, JSID_VOIDHANDLE, obj, &functionValue))
+      FunctionForwarderOptions forwarderOptions(cx);
+      if (!xpc::NewFunctionForwarder(cx, JSID_VOIDHANDLE, obj, forwarderOptions, &functionValue))
           return nullptr;
 
       return &functionValue.toObject();
@@ -139,9 +140,14 @@ StackScopedCloneWrite(JSContext *cx, JSStructuredCloneWriter *writer,
         return true;
     }
 
-    if (cloneData->mOptions->cloneFunctions && JS_ObjectIsCallable(cx, obj)) {
-        cloneData->mFunctions.append(obj);
-        return JS_WriteUint32Pair(writer, SCTAG_FUNCTION, cloneData->mFunctions.length() - 1);
+    if (JS_ObjectIsCallable(cx, obj)) {
+        if (cloneData->mOptions->cloneFunctions) {
+            cloneData->mFunctions.append(obj);
+            return JS_WriteUint32Pair(writer, SCTAG_FUNCTION, cloneData->mFunctions.length() - 1);
+        } else {
+            JS_ReportError(cx, "Permission denied to pass a Function via structured clone");
+            return false;
+        }
     }
 
     JS_ReportError(cx, "Encountered unsupported value type writing stack-scoped structured clone");
@@ -201,17 +207,30 @@ CloningFunctionForwarder(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
-    RootedValue v(cx, js::GetFunctionNativeReserved(&args.callee(), 0));
-    NS_ASSERTION(v.isObject(), "weird function");
-    RootedObject origFunObj(cx, UncheckedUnwrap(&v.toObject()));
+    // Grab the options from the reserved slot.
+    RootedObject optionsObj(cx, &js::GetFunctionNativeReserved(&args.callee(), 1).toObject());
+    FunctionForwarderOptions options(cx, optionsObj);
+    if (!options.Parse())
+        return false;
+
+    // Grab and unwrap the underlying callable.
+    RootedObject forwarderObj(cx, &js::GetFunctionNativeReserved(&args.callee(), 0).toObject());
+    RootedObject origFunObj(cx, UncheckedUnwrap(forwarderObj));
     {
         JSAutoCompartment ac(cx, origFunObj);
         // Note: only the arguments are cloned not the |this| or the |callee|.
         // Function forwarder does not use those.
-        StackScopedCloneOptions options;
-        options.wrapReflectors = true;
+        StackScopedCloneOptions cloneOptions;
+        cloneOptions.wrapReflectors = true;
         for (unsigned i = 0; i < args.length(); i++) {
-            if (!StackScopedClone(cx, options, args[i])) {
+            RootedObject argObj(cx, args[i].isObject() ? &args[i].toObject() : nullptr);
+            if (options.allowCallbacks && argObj && JS_ObjectIsCallable(cx, argObj)) {
+                FunctionForwarderOptions innerOptions(cx);
+                if (!JS_WrapObject(cx, &argObj))
+                    return false;
+                if (!xpc::NewFunctionForwarder(cx, JSID_VOIDHANDLE, argObj, innerOptions, args[i]))
+                    return nullptr;
+            } else if (!StackScopedClone(cx, cloneOptions, args[i])) {
                 return false;
             }
         }
@@ -244,7 +263,7 @@ NonCloningFunctionForwarder(JSContext *cx, unsigned argc, Value *vp)
 }
 bool
 NewFunctionForwarder(JSContext *cx, HandleId idArg, HandleObject callable,
-                     MutableHandleValue vp)
+                     FunctionForwarderOptions &options, MutableHandleValue vp)
 {
     RootedId id(cx, idArg);
     if (id == JSID_VOIDHANDLE)
@@ -255,9 +274,18 @@ NewFunctionForwarder(JSContext *cx, HandleId idArg, HandleObject callable,
     if (!fun)
         return false;
 
-    JSObject *funobj = JS_GetFunctionObject(fun);
-    js::SetFunctionNativeReserved(funobj, 0, ObjectValue(*callable));
-    vp.setObject(*funobj);
+    // Stash the callable in slot 0.
+    AssertSameCompartment(cx, callable);
+    RootedObject funObj(cx, JS_GetFunctionObject(fun));
+    js::SetFunctionNativeReserved(funObj, 0, ObjectValue(*callable));
+
+    // Stash the options in slot 1.
+    RootedObject optionsObj(cx, options.ToJSObject(cx));
+    if (!optionsObj)
+        return false;
+    js::SetFunctionNativeReserved(funObj, 1, ObjectValue(*optionsObj));
+
+    vp.setObject(*funObj);
     return true;
 }
 
@@ -339,7 +367,9 @@ ExportFunction(JSContext *cx, HandleValue vfunction, HandleValue vscope, HandleV
 
         // And now, let's create the forwarder function in the target compartment
         // for the function the be exported.
-        if (!NewFunctionForwarder(cx, id, funObj, rval)) {
+        FunctionForwarderOptions forwarderOptions(cx);
+        forwarderOptions.allowCallbacks = options.allowCallbacks;
+        if (!NewFunctionForwarder(cx, id, funObj, forwarderOptions, rval)) {
             JS_ReportError(cx, "Exporting function failed");
             return false;
         }

js/xpconnect/src/xpcprivate.h

@@ -3281,13 +3281,17 @@ Atob(JSContext *cx, unsigned argc, jsval *vp);
 bool
 Btoa(JSContext *cx, unsigned argc, jsval *vp);
 
+class FunctionForwarderOptions;
 
 // Helper function that creates a JSFunction that wraps a native function that
-// forwards the call to the original 'callable'. Any object-valued arguments are
-// cloned at call time for improved security.
+// forwards the call to the original 'callable'. For improved security, any
+// object-valued arguments are cloned at call time, unless either:
+//
+// * The object is a function and FunctionForwarderOptions::allowCallbacks is set
+// * The object is a reflector, in which case it is wrapped.
 bool
 NewFunctionForwarder(JSContext *cx, JS::HandleId id, JS::HandleObject callable,
-                     JS::MutableHandleValue vp);
+                     FunctionForwarderOptions &options, JS::MutableHandleValue vp);
 
 // Old-style function forwarding without structured-cloning for arguments. This
 // is deprecated.
@@ -3404,11 +3408,16 @@ public:
                   JSObject* options = nullptr)
         : OptionsBase(cx, options)
         , defineAs(cx, JSID_VOID)
+        , allowCallbacks(false)
     { }
 
-    virtual bool Parse() { return ParseId("defineAs", &defineAs); };
+    virtual bool Parse() {
+        return ParseId("defineAs", &defineAs) &&
+               ParseBoolean("allowCallbacks", &allowCallbacks);
+    };
 
     JS::RootedId defineAs;
+    bool allowCallbacks;
 };
 
 class MOZ_STACK_CLASS StackScopedCloneOptions : public OptionsBase {
@@ -3425,10 +3434,51 @@ public:
                ParseBoolean("cloneFunctions", &cloneFunctions);
     };
 
+    // When a reflector is encountered, wrap it rather than aborting the clone.
     bool wrapReflectors;
+
+    // When a function is encountered, clone it (exportFunction-style) rather than
+    // aborting the clone.
     bool cloneFunctions;
 };
 
+class MOZ_STACK_CLASS FunctionForwarderOptions : public OptionsBase {
+public:
+    FunctionForwarderOptions(JSContext *cx = xpc_GetSafeJSContext(),
+                             JSObject* options = nullptr)
+        : OptionsBase(cx, options)
+        , allowCallbacks(false)
+    { }
+
+    JSObject *ToJSObject(JSContext *cx) {
+        JS::RootedObject global(cx, JS::CurrentGlobalOrNull(cx));
+        JS::RootedObject obj(cx, JS_NewObjectWithGivenProto(cx, nullptr, JS::NullPtr(), global));
+        if (!obj)
+            return nullptr;
+
+        JS::RootedValue val(cx);
+        unsigned attrs = JSPROP_READONLY | JSPROP_PERMANENT;
+        val = JS::BooleanValue(allowCallbacks);
+        if (!JS_DefineProperty(cx, obj, "allowCallbacks", val, attrs))
+            return nullptr;
+
+        return obj;
+    }
+
+    virtual bool Parse() {
+        return ParseBoolean("allowCallbacks", &allowCallbacks);
+    };
+
+    // Allow callback arguments. This is similar to setting cloneFunctions in
+    // StackScopedCloneOptions, except that cloneFunctions will clone any Function
+    // encountered in the object graph, whereas this option only allows the base
+    // object to be supported.
+    //
+    // So invoking: |forwardedFunction(callback)| will work, but
+    // |forwardedFunction({ cb: callback })| will not.
+    bool allowCallbacks;
+};
+
 JSObject *
 CreateGlobalObject(JSContext *cx, const JSClass *clasp, nsIPrincipal *principal,
                    JS::CompartmentOptions& aOptions);

js/xpconnect/tests/unit/test_exportFunction.js

@@ -10,6 +10,7 @@ function run_test() {
   epsb.do_check_true = do_check_true;
   epsb.do_check_eq = do_check_eq;
   epsb.do_check_neq = do_check_neq;
+  subsb.do_check_true = do_check_true;
 
   // Exporting should work if prinicipal of the source sandbox
   // subsumes the principal of the target sandbox.
@@ -17,7 +18,7 @@ function run_test() {
     Object.prototype.protoProp = "common";
     var wasCalled = false;
     var _this = this;
-    this.funToExport = function(a, obj, native, mixed) {
+    this.funToExport = function(a, obj, native, mixed, callback) {
       do_check_eq(a, 42);
       do_check_neq(obj, subsb.tobecloned);
       do_check_eq(obj.cloned, "cloned");
@@ -26,13 +27,18 @@ function run_test() {
       do_check_eq(_this, this);
       do_check_eq(mixed.xrayed, subsb.xrayed);
       do_check_eq(mixed.xrayed2, subsb.xrayed2);
+      if (typeof callback == 'function') {
+        do_check_eq(typeof subsb.callback, 'function');
+        do_check_neq(callback, subsb.callback);
+        callback();
+      }
       wasCalled = true;
     };
     this.checkIfCalled = function() {
       do_check_true(wasCalled);
       wasCalled = false;
     }
-    exportFunction(funToExport, subsb, { defineAs: "imported" });
+    exportFunction(funToExport, subsb, { defineAs: "imported", allowCallbacks: true });
   }.toSource() + ")()", epsb);
 
   subsb.xrayed = Cu.evalInSandbox("(" + function () {
@@ -47,7 +53,17 @@ function run_test() {
     xrayed2 = XPCNativeWrapper(new XMLHttpRequest());
     mixed = { xrayed: xrayed, xrayed2: xrayed2 };
     tobecloned = { cloned: "cloned" };
-    imported(42,tobecloned, native, mixed);
+    invokedCallback = false;
+    callback = function() { invokedCallback = true; };
+    imported(42, tobecloned, native, mixed, callback);
+    do_check_true(invokedCallback);
+    try {
+      // Callbacks must be functions, not objects leading to functions.
+      imported(42, tobecloned, native, mixed, { cb: callback });
+      do_check_true(false);
+    } catch (e) {
+      do_check_true(/denied/.test(e) && /Function/.test(e));
+    }
   }.toSource() + ")()", subsb);
 
   // Invoking an exported function with cross-origin arguments should throw.
@@ -114,6 +130,16 @@ function run_test() {
     imported2(42, tobecloned, native, mixed);
   }.toSource() + ")()", subsb);
 
+  // Make sure that functions may not be passed when allowCallbacks is not set.
+  try {
+    Cu.evalInSandbox("(" + function () {
+      imported2(42, tobecloned, native, mixed, callback);
+    }.toSource() + ")()", subsb);
+    do_check_true(false);
+  } catch (e) {
+    do_check_true(/denied/.test(e) && /Function/.test(e));
+  }
+
   Cu.evalInSandbox("(" + function() {
     checkIfCalled();
   }.toSource() + ")()", epsb);