Bug 1083060 - Refactor the XOW access control code to use an enum rather than a char*. r=bz
authorBobby Holley <bobbyholley@gmail.com>
Fri, 17 Oct 2014 16:17:02 +0200
changeset 211044 78f725c6441db73e22b886040a0782f11478bbe0
parent 211043 4714ac6ccbfe6d715edaf82ac1e6f71cecafd13e
child 211045 ba0373a2af17c17e65456136dd8d2f03aea2fabd
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbz
bugs1083060
milestone36.0a1
Bug 1083060 - Refactor the XOW access control code to use an enum rather than a char*. r=bz Srsly.
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/AccessCheck.h
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -88,26 +88,26 @@ AccessCheck::isChrome(JSObject *obj)
 nsIPrincipal *
 AccessCheck::getPrincipal(JSCompartment *compartment)
 {
     return GetCompartmentPrincipal(compartment);
 }
 
 // Hardcoded policy for cross origin property access. See the HTML5 Spec.
 static bool
-IsPermitted(const char *name, JSFlatString *prop, bool set)
+IsPermitted(CrossOriginObjectType type, JSFlatString *prop, bool set)
 {
     size_t propLength = JS_GetStringLength(JS_FORGET_STRING_FLATNESS(prop));
     if (!propLength)
         return false;
 
     char16_t propChar0 = JS_GetFlatStringCharAt(prop, 0);
-    if (name[0] == 'L' && !strcmp(name, "Location"))
+    if (type == CrossOriginLocation)
         return dom::LocationBinding::IsPermitted(prop, propChar0, set);
-    if (name[0] == 'W' && !strcmp(name, "Window"))
+    if (type == CrossOriginWindow)
         return dom::WindowBinding::IsPermitted(prop, propChar0, set);
 
     return false;
 }
 
 static bool
 IsFrameId(JSContext *cx, JSObject *objArg, jsid idArg)
 {
@@ -136,20 +136,29 @@ IsFrameId(JSContext *cx, JSObject *objAr
             return false;
         }
         col->NamedItem(idAsString, getter_AddRefs(domwin));
     }
 
     return domwin != nullptr;
 }
 
-static bool
-IsWindow(const char *name)
+CrossOriginObjectType
+IdentifyCrossOriginObject(JSObject *obj)
 {
-    return name[0] == 'W' && !strcmp(name, "Window");
+    obj = js::UncheckedUnwrap(obj, /* stopAtOuter = */ false);
+    const js::Class *clasp = js::GetObjectClass(obj);
+    MOZ_ASSERT(!XrayUtils::IsXPCWNHolderClass(Jsvalify(clasp)), "shouldn't have a holder here");
+
+    if (clasp->name[0] == 'L' && !strcmp(clasp->name, "Location"))
+        return CrossOriginLocation;
+    if (clasp->name[0] == 'W' && !strcmp(clasp->name, "Window"))
+        return CrossOriginWindow;
+
+    return CrossOriginOpaque;
 }
 
 bool
 AccessCheck::isCrossOriginAccessPermitted(JSContext *cx, HandleObject wrapper, HandleId id,
                                           Wrapper::Action act)
 {
     if (act == Wrapper::CALL)
         return false;
@@ -161,35 +170,28 @@ AccessCheck::isCrossOriginAccessPermitte
     // is allowed, and rely on FilteringWrapper to filter out any disallowed accessors.
     if (act == Wrapper::GET_PROPERTY_DESCRIPTOR) {
         return isCrossOriginAccessPermitted(cx, wrapper, id, Wrapper::GET) ||
                isCrossOriginAccessPermitted(cx, wrapper, id, Wrapper::SET);
     }
 
     RootedObject obj(cx, Wrapper::wrappedObject(wrapper));
 
-    const char *name;
-    const js::Class *clasp = js::GetObjectClass(obj);
-    MOZ_ASSERT(!XrayUtils::IsXPCWNHolderClass(Jsvalify(clasp)), "shouldn't have a holder here");
-    if (clasp->ext.innerObject)
-        name = "Window";
-    else
-        name = clasp->name;
-
+    CrossOriginObjectType type = IdentifyCrossOriginObject(obj);
     if (JSID_IS_STRING(id)) {
-        if (IsPermitted(name, JSID_TO_FLAT_STRING(id), act == Wrapper::SET))
+        if (IsPermitted(type, JSID_TO_FLAT_STRING(id), act == Wrapper::SET))
             return true;
     }
 
     if (act != Wrapper::GET)
         return false;
 
     // Check for frame IDs. If we're resolving named frames, make sure to only
     // resolve ones that don't shadow native properties. See bug 860494.
-    if (IsWindow(name)) {
+    if (type == CrossOriginWindow) {
         if (JSID_IS_STRING(id)) {
             bool wouldShadow = false;
             if (!XrayUtils::HasNativeProperty(cx, wrapper, id, &wouldShadow) ||
                 wouldShadow)
             {
                 // If the named subframe matches the name of a DOM constructor,
                 // the global resolve triggered by the HasNativeProperty call
                 // above will try to perform a CheckedUnwrap on |wrapper|, and
--- a/js/xpconnect/wrappers/AccessCheck.h
+++ b/js/xpconnect/wrappers/AccessCheck.h
@@ -22,16 +22,23 @@ class AccessCheck {
     static bool subsumesConsideringDomain(JSCompartment *a, JSCompartment *b);
     static bool isChrome(JSCompartment *compartment);
     static bool isChrome(JSObject *obj);
     static nsIPrincipal *getPrincipal(JSCompartment *compartment);
     static bool isCrossOriginAccessPermitted(JSContext *cx, JS::HandleObject obj,
                                              JS::HandleId id, js::Wrapper::Action act);
 };
 
+enum CrossOriginObjectType {
+    CrossOriginWindow,
+    CrossOriginLocation,
+    CrossOriginOpaque
+};
+CrossOriginObjectType IdentifyCrossOriginObject(JSObject *obj);
+
 struct Policy {
 };
 
 // This policy allows no interaction with the underlying callable. Everything throws.
 struct Opaque : public Policy {
     static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
         return false;
     }