Bug 1471496 part 2. Change the way we do cross-compartment wrappers for Window and Location so they don't ever need to be recomputed. r=bholley

The end result we want is that on the web cross-compartment wrappers for
WindowProxy and Location are always CrossOriginObjectWrapper.  That needs to be true
for both cases that are different-origin (as now) and cases that are
same-origin, since they might become different-origin due to document.domain
changes but we don't want that to affect the wrappers involved.

On the web, all security checks are symmetric, so in WrapperFactory::Rewrap we
would have originSubsumesTarget == targetSubsumesOrigin in all web cases.

I claim that

  originSubsumesTarget == targetSubsumesOrigin &&
  (!targetSubsumesOrigin ||
   (!originCompartmentPrivate->wantXrays &&
    !targetCompartmentPrivate->wantXrays)) &&
  "object is a WindowProxy or Location"

is a necessary and sufficient condition for using CrossOriginObjectWrapper.

Comparing to our current code, if originSubsumesTarget and targetSubsumesOrigin
are both false, then for the WindowProxy and Location cases we currently end up
with the following arguments to SelectWrapper:

  securityWrapper: true
  xrayType: XrayForDOMObject
  waiveXrays: false

So SelectWrapper ends up returning CrossOriginObjectWrapper, which the new
condition keeps doing.

If originSubsumesTarget and targetSubsumesOrigin are both true, then there are
two cases.  If both compartments have wantXrays false (which is always the case
on the web), then we end up with the following arguments to SelectWrapper:

  securityWrapper: false
  xrayType: NotXray
  waiveXrays: false

and SelectWrapper returns CrossCompartmentWrapper.  We want to do
CrossOriginObjectWrapper instead, as explained above.

Finally, if originSubsumesTarget and targetSubsumesOrigin are both true but one
of the compartments has wantXrays set, then we get:

  securityWrapper: false
  xrayType: XrayForDOMObject
  waiveXrays: might be true or false

and then SelectWrapper might return a WaiveXrayWrapper or a PermissiveXrayDOM.
In this case we do _not_ want to start returning CrossOriginObjectWrapper, and
this is a non-web case anyway, since web compartments can't set wantXrays.

Differential Revision: https://phabricator.services.mozilla.com/D18030

--HG--
extra : moz-landing-system : lando

js/xpconnect/wrappers/AccessCheck.cpp

@@ -89,18 +89,12 @@ bool AccessCheck::isChrome(JSObject* obj) {
   return isChrome(js::GetObjectCompartment(obj));
 }
 
-CrossOriginObjectType IdentifyCrossOriginObject(JSObject* obj) {
+bool IsCrossOriginAccessibleObject(JSObject* obj) {
   obj = js::UncheckedUnwrap(obj, /* stopAtWindowProxy = */ false);
   const js::Class* clasp = js::GetObjectClass(obj);
 
-  if (clasp->name[0] == 'L' && !strcmp(clasp->name, "Location")) {
-    return CrossOriginLocation;
-  }
-  if (clasp->name[0] == 'W' && !strcmp(clasp->name, "Window")) {
-    return CrossOriginWindow;
-  }
-
-  return CrossOriginOpaque;
+  return (clasp->name[0] == 'L' && !strcmp(clasp->name, "Location")) ||
+         (clasp->name[0] == 'W' && !strcmp(clasp->name, "Window"));
 }
 
 bool AccessCheck::checkPassToPrivilegedCode(JSContext* cx, HandleObject wrapper,

js/xpconnect/wrappers/AccessCheck.h

@@ -35,12 +35,13 @@ class AccessCheck {
                                       const nsACString& accessType);
 };
 
-enum CrossOriginObjectType {
-  CrossOriginWindow,
-  CrossOriginLocation,
-  CrossOriginOpaque
-};
-CrossOriginObjectType IdentifyCrossOriginObject(JSObject* obj);
+/**
+ * Returns true if the given object (which is expected to be stripped of
+ * cross-compartment wrappers in practice, but this function doesn't assume
+ * that) is a WindowProxy or Location object, which need special wrapping
+ * behavior due to being usable cross-origin in limited ways.
+ */
+bool IsCrossOriginAccessibleObject(JSObject* obj);
 
 struct Policy {
   static bool checkCall(JSContext* cx, JS::HandleObject wrapper,

js/xpconnect/wrappers/WrapperFactory.cpp

@@ -339,13 +339,16 @@ static void DEBUG_CheckUnwrapSafety(HandleObject obj,
   } else if (AccessCheck::isChrome(target) ||
              xpc::IsUniversalXPConnectEnabled(target)) {
     // If the caller is chrome (or effectively so), unwrap should always be
-    // allowed.
-    MOZ_ASSERT(!handler->hasSecurityPolicy());
+    // allowed, but we might have a CrossOriginObjectWrapper here which allows
+    // it dynamically.
+    MOZ_ASSERT(!handler->hasSecurityPolicy() ||
+               handler == &CrossOriginObjectWrapper::singleton);
   } else if (RealmPrivate::Get(origin)->forcePermissiveCOWs) {
     // Similarly, if this is a privileged scope that has opted to make itself
     // accessible to the world (allowed only during automation), unwrap should
-    // be allowed.
-    MOZ_ASSERT(!handler->hasSecurityPolicy());
+    // be allowed.  Again, it might be allowed dynamically.
+    MOZ_ASSERT(!handler->hasSecurityPolicy() ||
+               handler == &CrossOriginObjectWrapper::singleton);
   } else {
     // Otherwise, it should depend on whether the target subsumes the origin.
     JS::Compartment* originComp = JS::GetCompartmentForRealm(origin);
@@ -354,7 +357,17 @@ static void DEBUG_CheckUnwrapSafety(HandleObject obj,
              ? AccessCheck::subsumesConsideringDomain(target, originComp)
              : AccessCheck::subsumesConsideringDomainIgnoringFPD(target,
                                                                  originComp));
-    MOZ_ASSERT(handler->hasSecurityPolicy() == !subsumes);
+    if (!subsumes) {
+      // If the target (which is where the wrapper lives) does not subsume the
+      // origin (which is where the wrapped object lives), then we should have a
+      // security check on the wrapper here.
+      MOZ_ASSERT(handler->hasSecurityPolicy());
+    } else {
+      // Even if target subsumes origin, we might have a wrapper with a security
+      // policy here, if it happens to be a CrossOriginObjectWrapper.
+      MOZ_ASSERT(!handler->hasSecurityPolicy() ||
+                 handler == &CrossOriginObjectWrapper::singleton);
+    }
   }
 }
 #else
@@ -404,12 +417,6 @@ static const Wrapper* SelectWrapper(bool securityWrapper, XrayType xrayType,
     return &PermissiveXrayOpaque::singleton;
   }
 
-  // This is a security wrapper. Use the security versions and filter.
-  if (xrayType == XrayForDOMObject &&
-      IdentifyCrossOriginObject(obj) != CrossOriginOpaque) {
-    return &CrossOriginObjectWrapper::singleton;
-  }
-
   // There's never any reason to expose other objects to non-subsuming actors.
   // Just use an opaque wrapper in these cases.
   //
@@ -501,6 +508,18 @@ JSObject* WrapperFactory::Rewrap(JSContext* cx, HandleObject existing,
     }
   }
 
+  // Special handling for the web's cross-origin objects (WindowProxy and
+  // Location).  We only need or want to do this in web-like contexts, where all
+  // security relationships are symmetric and there are no forced Xrays.
+  else if (originSubsumesTarget == targetSubsumesOrigin &&
+           // Check for the more rare case of cross-origin objects before doing
+           // the more-likely-to-pass checks for wantXrays.
+           IsCrossOriginAccessibleObject(obj) &&
+           (!targetSubsumesOrigin || (!originCompartmentPrivate->wantXrays &&
+                                      !targetCompartmentPrivate->wantXrays))) {
+    wrapper = &CrossOriginObjectWrapper::singleton;
+  }
+
   //
   // Now, handle the regular cases.
   //