Bug 808608 - Remove specialized Location security wrappers. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Wed, 21 Nov 2012 13:20:05 -0800
changeset 113928 0f3cad59a9637db50b07b15c4454dfa4d1bc93c2
parent 113927 f630faad853a3c8d5eb25be027c2abaeeac725fc
child 113929 6b62f09b6f94ed1e59bedc1c77f152be9abff722
push id18459
push userbobbyholley@gmail.com
push dateWed, 21 Nov 2012 21:21:06 +0000
treeherdermozilla-inbound@030c89e22e3e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs808608
milestone20.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 808608 - Remove specialized Location security wrappers. r=mrbkap
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/AccessCheck.h
js/xpconnect/wrappers/FilteringWrapper.cpp
js/xpconnect/wrappers/WrapperFactory.cpp
js/xpconnect/wrappers/WrapperFactory.h
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -1587,28 +1587,20 @@ XPCWrappedNative::ReparentWrapperIfFound
 
                 if (!newMap->Add(wrapper))
                     MOZ_CRASH();
             }
 
             JSObject *ww = wrapper->GetWrapper();
             if (ww) {
                 JSObject *newwrapper;
-                MOZ_ASSERT(!xpc::WrapperFactory::IsComponentsObject(flat), 
-                           "Components object should never get here");
-                if (xpc::WrapperFactory::IsLocationObject(flat)) {
-                    newwrapper = xpc::WrapperFactory::WrapLocationObject(ccx, newobj);
-                    if (!newwrapper)
-                        MOZ_CRASH();
-                } else {
-                    NS_ASSERTION(wrapper->NeedsSOW(), "weird wrapper wrapper");
-                    newwrapper = xpc::WrapperFactory::WrapSOWObject(ccx, newobj);
-                    if (!newwrapper)
-                        MOZ_CRASH();
-                }
+                MOZ_ASSERT(wrapper->NeedsSOW(), "weird wrapper wrapper");
+                newwrapper = xpc::WrapperFactory::WrapSOWObject(ccx, newobj);
+                if (!newwrapper)
+                    MOZ_CRASH();
 
                 // Ok, now we do the special object-plus-wrapper transplant.
                 ww = xpc::TransplantObjectWithWrapper(ccx, flat, ww, newobj,
                                                       newwrapper);
                 if (!ww)
                     MOZ_CRASH();
 
                 flat = newobj;
@@ -2208,21 +2200,17 @@ XPCWrappedNative::GetSameCompartmentSecu
     if (xpc::AccessCheck::isChrome(cxCompartment)) {
         MOZ_ASSERT(wrapper == NULL);
         return flat;
     }
 
     // Check the possibilities. Note that we need to check for null in each
     // case in order to distinguish between the 'no need for wrapper' and
     // 'wrapping failed' cases.
-    if (xpc::WrapperFactory::IsLocationObject(flat)) {
-        wrapper = xpc::WrapperFactory::WrapLocationObject(cx, flat);
-        if (!wrapper)
-            return NULL;
-    } else if (NeedsSOW()) {
+    if (NeedsSOW()) {
         wrapper = xpc::WrapperFactory::WrapSOWObject(cx, flat);
         if (!wrapper)
             return NULL;
     } else if (xpc::WrapperFactory::IsComponentsObject(flat)) {
         wrapper = xpc::WrapperFactory::WrapComponentsObject(cx, flat);
         if (!wrapper)
             return NULL;
     }
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -75,42 +75,16 @@ AccessCheck::wrapperSubsumes(JSObject *w
 {
     MOZ_ASSERT(js::IsWrapper(wrapper));
     JSObject *wrapped = js::UnwrapObject(wrapper);
     return AccessCheck::subsumes(js::GetObjectCompartment(wrapper),
                                  js::GetObjectCompartment(wrapped));
 }
 
 bool
-AccessCheck::isLocationObjectSameOrigin(JSContext *cx, JSObject *wrapper)
-{
-    // The caller must ensure that the given wrapper wraps a Location object.
-    MOZ_ASSERT(WrapperFactory::IsLocationObject(js::UnwrapObject(wrapper)));
-
-    // Location objects are parented to the outer window for which they
-    // were created. This gives us an easy way to determine whether our
-    // object is same origin with the current inner window:
-
-    // Grab the outer window...
-    JSObject *obj = js::GetObjectParent(js::UnwrapObject(wrapper));
-    if (!js::GetObjectClass(obj)->ext.innerObject) {
-        // ...which might be wrapped in a security wrapper.
-        obj = js::UnwrapObject(obj);
-        MOZ_ASSERT(js::GetObjectClass(obj)->ext.innerObject);
-    }
-
-    // Now innerize it to find the *current* inner window for our outer.
-    obj = JS_ObjectToInnerObject(cx, obj);
-
-    // Which lets us compare the current compartment against the old one.
-    return obj && subsumes(js::GetObjectCompartment(wrapper),
-                           js::GetObjectCompartment(obj));
-}
-
-bool
 AccessCheck::isChrome(JSCompartment *compartment)
 {
     nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager();
     if (!ssm) {
         return false;
     }
 
     bool privileged;
@@ -294,19 +268,17 @@ AccessCheck::isScriptAccessOnly(JSContex
         CrossOriginAccessiblePropertiesOnly>::singleton) {
         jsid scriptOnlyId = GetRTIdByIndex(cx, XPCJSRuntime::IDX_SCRIPTONLY);
         jsval scriptOnly;
         if (JS_LookupPropertyById(cx, obj, scriptOnlyId, &scriptOnly) &&
             scriptOnly == JSVAL_TRUE)
             return true; // script-only
     }
 
-    // Allow non-script access to same-origin location objects and any other
-    // objects.
-    return WrapperFactory::IsLocationObject(obj) && !isLocationObjectSameOrigin(cx, wrapper);
+    return false;
 }
 
 void
 AccessCheck::deny(JSContext *cx, jsid id)
 {
     if (id == JSID_VOID) {
         JS_ReportError(cx, "Permission denied to access object");
     } else {
--- a/js/xpconnect/wrappers/AccessCheck.h
+++ b/js/xpconnect/wrappers/AccessCheck.h
@@ -24,17 +24,16 @@ class AccessCheck {
     static bool isChrome(JSCompartment *compartment);
     static bool isChrome(JSObject *obj);
     static bool callerIsChrome();
     static nsIPrincipal *getPrincipal(JSCompartment *compartment);
     static bool isCrossOriginAccessPermitted(JSContext *cx, JSObject *obj, jsid id,
                                              js::Wrapper::Action act);
     static bool callerIsXBL(JSContext *cx);
     static bool isSystemOnlyAccessPermitted(JSContext *cx);
-    static bool isLocationObjectSameOrigin(JSContext *cx, JSObject *wrapper);
 
     static bool needsSystemOnlyWrapper(JSObject *obj);
 
     static bool isScriptAccessOnly(JSContext *cx, JSObject *wrapper);
 
     static void deny(JSContext *cx, jsid id);
 };
 
@@ -53,66 +52,24 @@ struct OnlyIfSubjectIsSystem : public Po
         return false;
     }
 };
 
 // This policy only permits access to properties that are safe to be used
 // across origins.
 struct CrossOriginAccessiblePropertiesOnly : public Policy {
     static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
-        // Location objects should always use LocationPolicy.
-        MOZ_ASSERT(!WrapperFactory::IsLocationObject(js::UnwrapObject(wrapper)));
         return AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act);
     }
     static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
         AccessCheck::deny(cx, id);
         return false;
     }
 };
 
-// We need a special security policy for Location objects.
-//
-// Location objects are special because their effective principal is that of
-// the outer window, not the inner window. So while the security characteristics
-// of most objects can be inferred from their compartments, those of the Location
-// object cannot. This has two implications:
-//
-// 1 - Same-compartment access of Location objects is not necessarily allowed.
-//     This means that objects must see a security wrapper around Location objects
-//     in their own compartment.
-// 2 - Cross-origin access of Location objects is not necessarily forbidden.
-//     Since the security decision depends on the current state of the outer window,
-//     we can't make it at wrap time. Instead, we need to make it at the time of
-//     access.
-//
-// So for any Location object access, be it same-compartment or cross-compartment,
-// we need to do a dynamic security check to determine whether the outer window is
-// same-origin with the caller.
-//
-// So this policy first checks whether the access is something that any code,
-// same-origin or not, is allowed to make. If it isn't, it _also_ checks the
-// state of the outer window to determine whether we happen to be same-origin
-// at the moment.
-struct LocationPolicy : public Policy {
-    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
-        // We should only be dealing with Location objects here.
-        MOZ_ASSERT(WrapperFactory::IsLocationObject(js::UnwrapObject(wrapper)));
-
-        if ((AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act) ||
-             AccessCheck::isLocationObjectSameOrigin(cx, wrapper))) {
-            return true;
-        }
-        return false;
-    }
-    static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
-        AccessCheck::deny(cx, id);
-        return false;
-    }
-};
-
 // This policy only permits access to properties if they appear in the
 // objects exposed properties list.
 struct ExposedPropertiesOnly : public Policy {
     static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act);
 
     static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) {
         // For gets, silently fail.
         if (act == js::Wrapper::GET)
--- a/js/xpconnect/wrappers/FilteringWrapper.cpp
+++ b/js/xpconnect/wrappers/FilteringWrapper.cpp
@@ -131,33 +131,27 @@ FilteringWrapper<Base, Policy>::enter(JS
     return Base::enter(cx, wrapper, id, act, bp);
 }
 
 #define SOW FilteringWrapper<CrossCompartmentSecurityWrapper, OnlyIfSubjectIsSystem>
 #define SCSOW FilteringWrapper<SameCompartmentSecurityWrapper, OnlyIfSubjectIsSystem>
 #define XOW FilteringWrapper<SecurityXrayXPCWN, CrossOriginAccessiblePropertiesOnly>
 #define DXOW   FilteringWrapper<SecurityXrayDOM, CrossOriginAccessiblePropertiesOnly>
 #define NNXOW FilteringWrapper<CrossCompartmentSecurityWrapper, CrossOriginAccessiblePropertiesOnly>
-#define LW    FilteringWrapper<SCSecurityXrayXPCWN, LocationPolicy>
-#define XLW   FilteringWrapper<SecurityXrayXPCWN, LocationPolicy>
 #define CW FilteringWrapper<SameCompartmentSecurityWrapper, ComponentsObjectPolicy>
 #define XCW FilteringWrapper<CrossCompartmentSecurityWrapper, ComponentsObjectPolicy>
 template<> SOW SOW::singleton(WrapperFactory::SCRIPT_ACCESS_ONLY_FLAG |
                               WrapperFactory::SOW_FLAG);
 template<> SCSOW SCSOW::singleton(WrapperFactory::SCRIPT_ACCESS_ONLY_FLAG |
                                   WrapperFactory::SOW_FLAG);
 template<> XOW XOW::singleton(WrapperFactory::SCRIPT_ACCESS_ONLY_FLAG);
 template<> DXOW DXOW::singleton(WrapperFactory::SCRIPT_ACCESS_ONLY_FLAG);
 template<> NNXOW NNXOW::singleton(WrapperFactory::SCRIPT_ACCESS_ONLY_FLAG);
-template<> LW  LW::singleton(WrapperFactory::SHADOWING_FORBIDDEN);
-template<> XLW XLW::singleton(WrapperFactory::SHADOWING_FORBIDDEN);
 
 template<> CW CW::singleton(0);
 template<> XCW XCW::singleton(0);
 
 template class SOW;
 template class XOW;
 template class DXOW;
 template class NNXOW;
-template class LW;
-template class XLW;
 template class ChromeObjectWrapperBase;
 }
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -296,18 +296,17 @@ static void
 DEBUG_CheckUnwrapSafety(JSObject *obj, js::Wrapper *handler,
                         JSCompartment *origin, JSCompartment *target)
 {
     typedef FilteringWrapper<CrossCompartmentSecurityWrapper, OnlyIfSubjectIsSystem> XSOW;
 
     if (AccessCheck::isChrome(target) || xpc::IsUniversalXPConnectEnabled(target)) {
         // If the caller is chrome (or effectively so), unwrap should always be allowed.
         MOZ_ASSERT(handler->isSafeToUnwrap());
-    } else if (WrapperFactory::IsLocationObject(obj) ||
-               WrapperFactory::IsComponentsObject(obj) ||
+    } else if (WrapperFactory::IsComponentsObject(obj) ||
                handler == &XSOW::singleton)
     {
         // This is an object that is restricted regardless of origin.
         MOZ_ASSERT(!handler->isSafeToUnwrap());
     } else {
         // Otherwise, it should depend on whether the target subsumes the origin.
         MOZ_ASSERT(handler->isSafeToUnwrap() == AccessCheck::subsumes(target, origin));
     }
@@ -366,20 +365,17 @@ WrapperFactory::Rewrap(JSContext *cx, JS
                 return nullptr;
             }
         }
 
         XPCWrappedNative *wn;
         if (targetdata &&
             (wn = GetWrappedNative(cx, obj)) &&
             wn->HasProto() && wn->GetProto()->ClassIsDOMObject()) {
-            if (IsLocationObject(obj))
-                wrapper = &FilteringWrapper<SecurityXrayXPCWN, LocationPolicy>::singleton;
-            else
-                wrapper = &FilteringWrapper<SecurityXrayXPCWN, CrossOriginAccessiblePropertiesOnly>::singleton;
+            wrapper = &FilteringWrapper<SecurityXrayXPCWN, CrossOriginAccessiblePropertiesOnly>::singleton;
         } else if (mozilla::dom::UseDOMXray(obj)) {
             wrapper = &FilteringWrapper<SecurityXrayDOM, CrossOriginAccessiblePropertiesOnly>::singleton;
         } else if (IsComponentsObject(obj)) {
             wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper,
                                         ComponentsObjectPolicy>::singleton;
         } else {
             wrapper = &ChromeObjectWrapper::singleton;
 
@@ -414,29 +410,26 @@ WrapperFactory::Rewrap(JSContext *cx, JS
                 MOZ_ASSERT(homeProto);
                 proxyProto = homeProto;
             }
         }
     } else if (AccessCheck::subsumes(target, origin)) {
         // For the same-origin case we use a transparent wrapper, unless one
         // of the following is true:
         // * The object is flagged as needing a SOW.
-        // * The object is a Location object.
         // * The object is a Components object.
         // * The context compartment specifically requested Xray vision into
         //   same-origin compartments.
         //
         // The first two cases always require a security wrapper for non-chrome
         // access, regardless of the origin of the object.
         XrayType type;
         if (AccessCheck::needsSystemOnlyWrapper(obj)) {
             wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper,
                                         OnlyIfSubjectIsSystem>::singleton;
-        } else if (IsLocationObject(obj)) {
-            wrapper = &FilteringWrapper<SecurityXrayXPCWN, LocationPolicy>::singleton;
         } else if (IsComponentsObject(obj)) {
             wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper,
                                         ComponentsObjectPolicy>::singleton;
         } else if (!targetdata || !targetdata->wantXrays ||
                    (type = GetXrayType(obj)) == NotXray) {
             wrapper = &CrossCompartmentWrapper::singleton;
         } else if (type == XrayForDOMObject) {
             wrapper = &PermissiveXrayDOM::singleton;
@@ -452,24 +445,18 @@ WrapperFactory::Rewrap(JSContext *cx, JS
         XrayType type = GetXrayType(obj);
         if (type == NotXray) {
             wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper,
                                         CrossOriginAccessiblePropertiesOnly>::singleton;
         } else if (type == XrayForDOMObject) {
             wrapper = &FilteringWrapper<SecurityXrayDOM,
                                         CrossOriginAccessiblePropertiesOnly>::singleton;
         } else {
-            // Location objects can become same origin after navigation, so we might
-            // have to grant transparent access later on.
-            if (IsLocationObject(obj)) {
-                wrapper = &FilteringWrapper<SecurityXrayXPCWN, LocationPolicy>::singleton;
-            } else {
-                wrapper = &FilteringWrapper<SecurityXrayXPCWN,
-                                            CrossOriginAccessiblePropertiesOnly>::singleton;
-            }
+            wrapper = &FilteringWrapper<SecurityXrayXPCWN,
+                                        CrossOriginAccessiblePropertiesOnly>::singleton;
         }
     }
 
     DEBUG_CheckUnwrapSafety(obj, wrapper, origin, target);
 
     if (existing && proxyProto == wrappedProto)
         return Wrapper::Renew(cx, existing, obj, wrapper);
 
@@ -492,33 +479,16 @@ WrapperFactory::WrapForSameCompartment(J
     MOZ_ASSERT(wn, "Trying to wrap a dead WN!");
 
     // The WN knows what to do.
     JSObject *wrapper = wn->GetSameCompartmentSecurityWrapper(cx);
     MOZ_ASSERT_IF(wrapper != obj, !Wrapper::wrapperHandler(wrapper)->isSafeToUnwrap());
     return wrapper;
 }
 
-bool
-WrapperFactory::IsLocationObject(JSObject *obj)
-{
-    const char *name = js::GetObjectClass(obj)->name;
-    return name[0] == 'L' && !strcmp(name, "Location");
-}
-
-JSObject *
-WrapperFactory::WrapLocationObject(JSContext *cx, JSObject *obj)
-{
-    JSObject *proto;
-    if (!js::GetObjectProto(cx, obj, &proto))
-        return nullptr;
-    return Wrapper::New(cx, obj, proto, js::GetObjectParent(obj),
-                        &FilteringWrapper<SCSecurityXrayXPCWN, LocationPolicy>::singleton);
-}
-
 // Call WaiveXrayAndWrap when you have a JS object that you don't want to be
 // wrapped in an Xray wrapper. cx->compartment is the compartment that will be
 // using the returned object. If the object to be wrapped is already in the
 // correct compartment, then this returns the unwrapped object.
 bool
 WrapperFactory::WaiveXrayAndWrap(JSContext *cx, jsval *vp)
 {
     if (JSVAL_IS_PRIMITIVE(*vp))
--- a/js/xpconnect/wrappers/WrapperFactory.h
+++ b/js/xpconnect/wrappers/WrapperFactory.h
@@ -64,22 +64,16 @@ class WrapperFactory {
                             JSObject *wrappedProto,
                             JSObject *parent,
                             unsigned flags);
 
     // Wrap an object for same-compartment access.
     static JSObject *WrapForSameCompartment(JSContext *cx,
                                             JSObject *obj);
 
-    // Return true if this is a location object.
-    static bool IsLocationObject(JSObject *obj);
-
-    // Wrap a location object.
-    static JSObject *WrapLocationObject(JSContext *cx, JSObject *obj);
-
     // Wrap wrapped object into a waiver wrapper and then re-wrap it.
     static bool WaiveXrayAndWrap(JSContext *cx, jsval *vp);
 
     // Wrap a (same compartment) object in a SOW.
     static JSObject *WrapSOWObject(JSContext *cx, JSObject *obj);
 
     // Return true if this is a Components object.
     static bool IsComponentsObject(JSObject *obj);