Bug 1363208 part 4. Stop using cross-origin Xrays for WindowProxy. r=peterv,jandem
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 21 Jan 2019 03:30:31 +0000
changeset 454646 2e31b4d57c6af6b9fb934a694440f9264b65a3b3
parent 454645 9b1badc02fd19cf5d5b77f92de7853f7214cb638
child 454647 7faa84f9f73a76f37d605a6b07d46c24e516e576
push id35409
push userrmaries@mozilla.com
push dateMon, 21 Jan 2019 17:48:45 +0000
treeherdermozilla-central@4977d02e1191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, jandem
bugs1363208, 440572, 860494
milestone66.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 1363208 part 4. Stop using cross-origin Xrays for WindowProxy. r=peterv,jandem The change to test_bug440572.html is due to a behavior change. Specifically, before this change, any IDL-declared property, even one not exposed cross-origin, would prevent named frames with that name being visible cross-origin. The new behavior is that cross-origin-exposed IDL properties prevent corresponding frame names from being visible, but ones not exposed cross-origin don't. This matches the spec and other browsers. Same thing for the changes to test_bug860494.xul. The wpt test changes are just adding test coverage for the thing the other tests caught. Differential Revision: https://phabricator.services.mozilla.com/D15428
dom/bindings/BindingUtils.cpp
dom/tests/mochitest/bugs/test_bug440572.html
js/xpconnect/tests/chrome/test_bug860494.xul
js/xpconnect/wrappers/WrapperFactory.cpp
js/xpconnect/wrappers/WrapperFactory.h
testing/web-platform/tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
testing/web-platform/tests/html/browsers/origin/cross-origin-objects/frame-with-then.html
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -2898,17 +2898,17 @@ struct LenientThisPolicy : public MaybeG
 // There are some cross-origin things on globals, so we inherit from
 // MaybeGlobalThisPolicy.
 struct CrossOriginThisPolicy : public MaybeGlobalThisPolicy {
   // We want the HasValidThisValue of MaybeGlobalThisPolicy.
 
   // We want the ExtractThisObject of MaybeGlobalThisPolicy.
 
   static MOZ_ALWAYS_INLINE JSObject* MaybeUnwrapThisObject(JSObject* aObj) {
-    if (xpc::WrapperFactory::IsXrayWrapper(aObj)) {
+    if (xpc::WrapperFactory::IsCrossOriginWrapper(aObj)) {
       return js::UncheckedUnwrap(aObj);
     }
 
     // Else just return aObj; our UnwrapThisObject call will try to
     // CheckedUnwrap it, and either succeed or get a security error as needed.
     return aObj;
   }
 
--- a/dom/tests/mochitest/bugs/test_bug440572.html
+++ b/dom/tests/mochitest/bugs/test_bug440572.html
@@ -22,24 +22,30 @@ function receiveMessage(e)
   is(e.origin, "http://example.org", "wrong sender!");
   messages.set(e.data.from, e.data.result);
 }
 
 window.addEventListener("message", receiveMessage);
 
 function runtests()
 {
-  is(messages.size, 2, "received the right number of messages.");
+  is(messages.size, 4, "received the right number of messages.");
   is(messages.get("test"), "success", "test in frame failed.");
-  isnot(messages.get("dump"), "success", "parent[\"dump\"] should be the WebIDL property of Window.");
+  is(messages.get("dump"), "success", '"dump" in frame failed.');
+  is(messages.get("open"), "success", '"open" in frame failed.');
+  isnot(messages.get("close"), "success", "parent[\"close\"] should be the WebIDL property of Window.");
 
   SimpleTest.finish();
 }
 
 SimpleTest.waitForExplicitFinish();
 </script>
 <br>
 <iframe name="test" src="http://example.org:80/tests/dom/tests/mochitest/bugs/iframe_bug440572.html"></iframe>
 <br>
 <iframe name="dump" src="http://example.org:80/tests/dom/tests/mochitest/bugs/iframe_bug440572.html"></iframe>
+<br>
+<iframe name="open" src="http://example.org:80/tests/dom/tests/mochitest/bugs/iframe_bug440572.html"></iframe>
+<br>
+<iframe name="close" src="http://example.org:80/tests/dom/tests/mochitest/bugs/iframe_bug440572.html"></iframe>
 </body>
 </html>
 
--- a/js/xpconnect/tests/chrome/test_bug860494.xul
+++ b/js/xpconnect/tests/chrome/test_bug860494.xul
@@ -39,18 +39,18 @@ https://bugzilla.mozilla.org/show_bug.cg
     sb.topWin = top;
     sb.parentWin = window;
     sb.is = is;
     sb.ok = ok;
     Cu.evalInSandbox('ok(win.top === topWin, "iframe names shouldnt shadow |top| via cross-origin Xray");', sb);
     Cu.evalInSandbox('ok(win.parent === parentWin, "iframe names shouldnt shadow |parent| via cross-origin Xray");', sb);
     Cu.evalInSandbox('is(win.length, 7, "iframe names shouldnt shadow |length| via cross-origin Xray");', sb);
     Cu.evalInSandbox('ok(win.window === win, "iframe names shouldnt shadow |window| via cross-origin Xray");', sb);
-    Cu.evalInSandbox('var exn = "nothrow"; try { win.navigator; } catch (e) { exn = e; } ok(!!/denied/.exec(exn), "cross-origin Xray blocks subframes that shadow: navigator: " + exn);', sb);
-    Cu.evalInSandbox('var exn = "nothrow"; try { win.alert; } catch (e) { exn = e; } ok(!!/denied/.exec(exn), "cross-origin Xray blocks subframes that shadow: alert: " + exn);', sb);
+    Cu.evalInSandbox('ok(win.navigator === win[5], "iframe names that correspond to non-cross-origin-visible properties should expose the subframe: navigator");', sb);
+    Cu.evalInSandbox('ok(win.alert === win[6], "iframe names that correspond to non-cross-origin-visible properties should expose the subframe: alert");', sb);
 
     SimpleTest.finish();
   }
 
   ]]>
   </script>
   <iframe id="ifr" type="content" onload="go();" src="http://example.org/tests/js/xpconnect/tests/mochitest/file_bug860494.html" />
 </window>
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -356,16 +356,18 @@ static void DEBUG_CheckUnwrapSafety(Hand
     MOZ_ASSERT(handler->hasSecurityPolicy() == !subsumes);
   }
 }
 #else
 #  define DEBUG_CheckUnwrapSafety(obj, handler, origin, target) \
     {}
 #endif
 
+const CrossOriginObjectWrapper CrossOriginObjectWrapper::singleton;
+
 static const Wrapper* SelectWrapper(bool securityWrapper, XrayType xrayType,
                                     bool waiveXrays, JSObject* obj) {
   // Waived Xray uses a modified CCW that has transparent behavior but
   // transitively waives Xrays on arguments.
   if (waiveXrays) {
     MOZ_ASSERT(!securityWrapper);
     return &WaiveXrayWrapper::singleton;
   }
@@ -389,21 +391,26 @@ static const Wrapper* SelectWrapper(bool
       return &PermissiveXrayJS::singleton;
     }
     MOZ_ASSERT(xrayType == XrayForOpaqueObject);
     return &PermissiveXrayOpaque::singleton;
   }
 
   // This is a security wrapper. Use the security versions and filter.
   if (xrayType == XrayForDOMObject &&
-      IdentifyCrossOriginObject(obj) != CrossOriginOpaque) {
+      IdentifyCrossOriginObject(obj) == CrossOriginLocation) {
     return &FilteringWrapper<CrossOriginXrayWrapper,
                              CrossOriginAccessiblePropertiesOnly>::singleton;
   }
 
+  if (xrayType == XrayForDOMObject &&
+      IdentifyCrossOriginObject(obj) == CrossOriginWindow) {
+    return &CrossOriginObjectWrapper::singleton;
+  }
+
   // There's never any reason to expose other objects to non-subsuming actors.
   // Just use an opaque wrapper in these cases.
   //
   // In general, we don't want opaque function wrappers to be callable.
   // But in the case of XBL, we rely on content being able to invoke
   // functions exposed from the XBL scope. We could remove this exception,
   // if needed, by using ExportFunction to generate the content-side
   // representations of XBL methods.
--- a/js/xpconnect/wrappers/WrapperFactory.h
+++ b/js/xpconnect/wrappers/WrapperFactory.h
@@ -6,16 +6,46 @@
 
 #ifndef _xpc_WRAPPERFACTORY_H
 #define _xpc_WRAPPERFACTORY_H
 
 #include "js/Wrapper.h"
 
 namespace xpc {
 
+/**
+ * A wrapper that's only used for cross-origin objects. This should be
+ * just like a CrossCompartmentWrapper but (as an implementation
+ * detail) doesn't actually do any compartment-entering and (as an
+ * implementation detail) delegates all the security decisions and
+ * compartment-entering to the target object, which is always a
+ * proxy.
+ *
+ * We could also inherit from CrossCompartmentWrapper but then we
+ * would need to override all the proxy hooks to avoid the
+ * compartment-entering bits.
+ */
+class CrossOriginObjectWrapper : public js::Wrapper {
+ public:
+  // We want to claim to have a security policy, so code doesn't just
+  // CheckedUnwrap us willy-nilly.  But we're OK with the BaseProxyHandler
+  // implementation of enter(), which allows entering.  Our target is what
+  // really does the security checks.
+  //
+  // We don't want to inherit from CrossCompartmentWrapper, because we don't
+  // want the compartment-entering behavior it has.  But we do want to set the
+  // CROSS_COMPARTMENT flag on js::Wrapper so that we test true for
+  // is<js::CrossCompartmentWrapperObject> and so forth.
+  constexpr explicit CrossOriginObjectWrapper()
+      : js::Wrapper(CROSS_COMPARTMENT, /* aHasPrototype = */ false,
+                    /* aHasSecurityPolicy = */ true) {}
+
+  static const CrossOriginObjectWrapper singleton;
+};
+
 class WrapperFactory {
  public:
   enum {
     WAIVE_XRAY_WRAPPER_FLAG = js::Wrapper::LAST_USED_FLAG << 1,
     IS_XRAY_WRAPPER_FLAG = WAIVE_XRAY_WRAPPER_FLAG << 1
   };
 
   // Return true if any of any of the nested wrappers have the flag set.
@@ -24,16 +54,22 @@ class WrapperFactory {
     js::UncheckedUnwrap(wrapper, true, &flags);
     return !!(flags & flag);
   }
 
   static bool IsXrayWrapper(JSObject* wrapper) {
     return HasWrapperFlag(wrapper, IS_XRAY_WRAPPER_FLAG);
   }
 
+  static bool IsCrossOriginWrapper(JSObject* obj) {
+    return IsXrayWrapper(obj) ||
+           (js::IsProxy(obj) &&
+            js::GetProxyHandler(obj) == &CrossOriginObjectWrapper::singleton);
+  }
+
   static bool HasWaiveXrayFlag(JSObject* wrapper) {
     return HasWrapperFlag(wrapper, WAIVE_XRAY_WRAPPER_FLAG);
   }
 
   static bool IsCOW(JSObject* wrapper);
 
   static JSObject* GetXrayWaiver(JS::HandleObject obj);
   static JSObject* CreateXrayWaiver(JSContext* cx, JS::HandleObject obj);
--- a/testing/web-platform/tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
+++ b/testing/web-platform/tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
@@ -145,21 +145,32 @@ addTest(function() {
   assert_throws("SecurityError", function() { C.location.__proto__ = new Object(); }, "proto set on cross-origin Location");
   var setters = [Object.getOwnPropertyDescriptor(Object.prototype, '__proto__').set];
   if (Object.setPrototypeOf)
     setters.push(function(p) { Object.setPrototypeOf(this, p); });
   setters.forEach(function(protoSetter) {
     assert_throws(new TypeError, function() { protoSetter.call(C, new Object()); }, "proto setter |call| on cross-origin Window");
     assert_throws(new TypeError, function() { protoSetter.call(C.location, new Object()); }, "proto setter |call| on cross-origin Location");
   });
+  // Hack to avoid "duplicate test name" harness issues.
+  setters.forEach(function(protoSetter) {
+    test(function() { protoSetter.call(C, null); },
+         "proto setter |call| on cross-origin Window with null (" + protoSetter + ")");
+    test(function() { protoSetter.call(C.location, null); },
+         "proto setter |call| on cross-origin Location with null (" + protoSetter + ")");
+  });
   if (Reflect.setPrototypeOf) {
     assert_false(Reflect.setPrototypeOf(C, new Object()),
                  "Reflect.setPrototypeOf on cross-origin Window");
+    assert_true(Reflect.setPrototypeOf(C, null),
+                "Reflect.setPrototypeOf on cross-origin Window with null");
     assert_false(Reflect.setPrototypeOf(C.location, new Object()),
                 "Reflect.setPrototypeOf on cross-origin Location");
+    assert_true(Reflect.setPrototypeOf(C.location, null),
+                "Reflect.setPrototypeOf on cross-origin Location with null");
   }
 }, "[[SetPrototypeOf]] should return false");
 
 /*
  * [[IsExtensible]]
  */
 addTest(function() {
   assert_true(Object.isExtensible(C), "cross-origin Window should be extensible");
@@ -225,16 +236,29 @@ addTest(function() {
     checkPropertyDescriptor(desc, prop, false);
   });
 }, "[[GetOwnProperty]] - Property descriptors for cross-origin properties should be set up correctly");
 
 addTest(function() {
   assert_equals(typeof D.then, "object");
 }, "[[GetOwnProperty]] - Subframe named 'then' should shadow the default 'then' value");
 
+addTest(function() {
+  assert_equals(typeof D.close, "function");
+  assert_equals(typeof D.open, "object");
+}, "[[GetOwnProperty]] - Subframes should be visible cross-origin only if their names don't match the names of cross-origin-exposed IDL properties");
+
+addTest(function() {
+  assert_equals(typeof Object.getOwnPropertyDescriptor(C, '0').value, "object");
+  assert_equals(typeof Object.getOwnPropertyDescriptor(C, '1').value, "object");
+  assert_throws("SecurityError", function() {
+    Object.getOwnPropertyDescriptor(C, '2');
+  });
+}, "[[GetOwnProperty]] - Should be able to get a property descriptor for an indexed property only if it corresponds to a child window.");
+
 /*
  * [[Delete]]
  */
 addTest(function() {
   assert_throws("SecurityError", function() { delete C[0]; }, "Can't delete cross-origin indexed property");
   assert_throws("SecurityError", function() { delete C[100]; }, "Can't delete cross-origin indexed property");
   assert_throws("SecurityError", function() { delete C.location; }, "Can't delete cross-origin property");
   assert_throws("SecurityError", function() { delete C.parent; }, "Can't delete cross-origin property");
--- a/testing/web-platform/tests/html/browsers/origin/cross-origin-objects/frame-with-then.html
+++ b/testing/web-platform/tests/html/browsers/origin/cross-origin-objects/frame-with-then.html
@@ -1,10 +1,14 @@
 <!doctype html>
 <html>
   <body>
     <!--- Some frames to test ordering -->
     <iframe name="a"></iframe>
     <!-- A subframe to test "then" behavior -->
     <iframe name="then"></iframe>
     <iframe name="b"></iframe>
+    <!-- Two subframes with names corresponding to IDL-defined properties; one
+         a cross-origin-exposed property and one not exposed cross-origin -->
+    <iframe name="close"></iframe>
+    <iframe name="open"></iframe>
   </body>
 </html>