Bug 1034262 - Honor the wantXrays of both sides of the membrane when computing same-origin wrappers. r=gabor
authorBobby Holley <bobbyholley@gmail.com>
Thu, 10 Jul 2014 10:04:30 -0700
changeset 193406 bb54fa82e9d2740bfaaa5dc9f1c093d43ee5df4a
parent 193405 61c1236e6a36a4fd2d16b4ef9e290a6bef62c995
child 193407 f2e5fb7f242cc39c3bc6da568567489673ff9cf0
push id27117
push userryanvm@gmail.com
push dateThu, 10 Jul 2014 22:23:14 +0000
treeherdermozilla-central@e1a037c085d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor
bugs1034262
milestone33.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 1034262 - Honor the wantXrays of both sides of the membrane when computing same-origin wrappers. r=gabor The basic problem in the testcase is that one compartment requests same-origin Xrays via wantXrays=true (the default for Sandboxes) while the other does not. The current code only considers the wantXrays flag of the compartment performing the access, so we end up in a situation where we have same-origin compartments, but Xray in one direction and Transparent in the other. This is a problem for crossCompartmentFunction.apply(null, [arg]). If both globals get transparent wrappers, there's obviously no problem. And if both globals get XrayWrappers, then the |apply| happens on the XrayWrapper of the function in the caller's compartment. So the Array is unpacked in the caller's compartment, and again we have no problem. But if the caller gets Transparent and the callee gets Xrays, then we end up invoking |apply| from the callee's side, which then gets an XrayWrapper to the array. This XrayWrapper may do surprising things, leading to the odd situation in the testcase. Same-origin Xrays are kind of broken anyway, but I don't think we'll ever be able to get rid of them. So the most sensible thing to do is probably to honor the flag (if set) from either compartment. This patch does that.
js/xpconnect/tests/unit/test_bug1034262.js
js/xpconnect/tests/unit/xpcshell.ini
js/xpconnect/wrappers/WrapperFactory.cpp
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/unit/test_bug1034262.js
@@ -0,0 +1,9 @@
+const Cu = Components.utils;
+function run_test() {
+  var sb1 = Cu.Sandbox('http://www.example.com', { wantXrays: true });
+  var sb2 = Cu.Sandbox('http://www.example.com', { wantXrays: false });
+  sb2.f = Cu.evalInSandbox('x => typeof x', sb1);
+  do_check_eq(Cu.evalInSandbox('f(dump)', sb2), 'function');
+  do_check_eq(Cu.evalInSandbox('f.call(null, dump)', sb2), 'function');
+  do_check_eq(Cu.evalInSandbox('f.apply(null, [dump])', sb2), 'function');
+}
--- a/js/xpconnect/tests/unit/xpcshell.ini
+++ b/js/xpconnect/tests/unit/xpcshell.ini
@@ -41,16 +41,17 @@ support-files =
 [test_bug885800.js]
 [test_bug961054.js]
 [test_bug976151.js]
 [test_bug1001094.js]
 [test_bug1021312.js]
 [test_bug1033253.js]
 [test_bug1033920.js]
 [test_bug1033927.js]
+[test_bug1034262.js]
 [test_bug_442086.js]
 [test_file.js]
 [test_blob.js]
 [test_blob2.js]
 [test_file2.js]
 [test_import.js]
 [test_import_fail.js]
 [test_isModuleLoaded.js]
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -416,17 +416,16 @@ WrapperFactory::Rewrap(JSContext *cx, Ha
     bool targetIsChrome = AccessCheck::isChrome(target);
     bool originSubsumesTarget = AccessCheck::subsumesConsideringDomain(origin, target);
     bool targetSubsumesOrigin = AccessCheck::subsumesConsideringDomain(target, origin);
     bool sameOrigin = targetSubsumesOrigin && originSubsumesTarget;
     XrayType xrayType = GetXrayType(obj);
     bool waiveXrayFlag = flags & WAIVE_XRAY_WRAPPER_FLAG;
 
     const Wrapper *wrapper;
-    CompartmentPrivate *targetdata = CompartmentPrivate::Get(target);
 
     //
     // First, handle the special cases.
     //
 
     // If UniversalXPConnect is enabled, this is just some dumb mochitest. Use
     // a vanilla CCW.
     if (xpc::IsUniversalXPConnectEnabled(target)) {
@@ -471,17 +470,19 @@ WrapperFactory::Rewrap(JSContext *cx, Ha
         bool securityWrapper = !targetSubsumesOrigin;
 
         // Xrays are warranted if either the target or the origin don't trust
         // each other. This is generally the case, unless the two are same-origin
         // and the caller has not requested same-origin Xrays.
         //
         // Xrays are a bidirectional protection, since it affords clarity to the
         // caller and privacy to the callee.
-        bool wantXrays = !(sameOrigin && !targetdata->wantXrays);
+        bool sameOriginXrays = CompartmentPrivate::Get(origin)->wantXrays ||
+                               CompartmentPrivate::Get(target)->wantXrays;
+        bool wantXrays = !sameOrigin || sameOriginXrays;
 
         // If Xrays are warranted, the caller may waive them for non-security
         // wrappers.
         bool waiveXrays = wantXrays && !securityWrapper && waiveXrayFlag;
 
         // We have slightly different behavior for the case when the object
         // being wrapped is in an XBL scope.
         bool originIsContentXBLScope = IsContentXBLScope(origin);