From 11d6106099e1228c9787a83d5a22086751ed6c55 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 21 Nov 2012 13:20:05 -0800 Subject: [PATCH] Bug 808608 - Remove specialized Location security wrappers. r=mrbkap --- js/xpconnect/src/XPCWrappedNative.cpp | 22 +++-------- js/xpconnect/wrappers/AccessCheck.cpp | 30 +-------------- js/xpconnect/wrappers/AccessCheck.h | 43 ---------------------- js/xpconnect/wrappers/FilteringWrapper.cpp | 6 --- js/xpconnect/wrappers/WrapperFactory.cpp | 38 ++----------------- js/xpconnect/wrappers/WrapperFactory.h | 6 --- 6 files changed, 10 insertions(+), 135 deletions(-) diff --git a/js/xpconnect/src/XPCWrappedNative.cpp b/js/xpconnect/src/XPCWrappedNative.cpp index ce349fc955fb..facdfa4cac97 100644 --- a/js/xpconnect/src/XPCWrappedNative.cpp +++ b/js/xpconnect/src/XPCWrappedNative.cpp @@ -1592,18 +1592,10 @@ XPCWrappedNative::ReparentWrapperIfFound(XPCCallContext& ccx, 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, @@ -2213,11 +2205,7 @@ XPCWrappedNative::GetSameCompartmentSecurityWrapper(JSContext *cx) // 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; diff --git a/js/xpconnect/wrappers/AccessCheck.cpp b/js/xpconnect/wrappers/AccessCheck.cpp index fd5fd3998883..92fa09c29394 100644 --- a/js/xpconnect/wrappers/AccessCheck.cpp +++ b/js/xpconnect/wrappers/AccessCheck.cpp @@ -79,32 +79,6 @@ AccessCheck::wrapperSubsumes(JSObject *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) { @@ -299,9 +273,7 @@ AccessCheck::isScriptAccessOnly(JSContext *cx, JSObject *wrapper) 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 diff --git a/js/xpconnect/wrappers/AccessCheck.h b/js/xpconnect/wrappers/AccessCheck.h index 0eb7fc01e3e4..f26cbefbfc64 100644 --- a/js/xpconnect/wrappers/AccessCheck.h +++ b/js/xpconnect/wrappers/AccessCheck.h @@ -29,7 +29,6 @@ class AccessCheck { 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); @@ -58,8 +57,6 @@ struct OnlyIfSubjectIsSystem : public Policy { // 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) { @@ -68,46 +65,6 @@ struct CrossOriginAccessiblePropertiesOnly : public Policy { } }; -// 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 { diff --git a/js/xpconnect/wrappers/FilteringWrapper.cpp b/js/xpconnect/wrappers/FilteringWrapper.cpp index 13a7eda255df..a86225e36bee 100644 --- a/js/xpconnect/wrappers/FilteringWrapper.cpp +++ b/js/xpconnect/wrappers/FilteringWrapper.cpp @@ -136,8 +136,6 @@ FilteringWrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, #define XOW FilteringWrapper #define DXOW FilteringWrapper #define NNXOW FilteringWrapper -#define LW FilteringWrapper -#define XLW FilteringWrapper #define CW FilteringWrapper #define XCW FilteringWrapper template<> SOW SOW::singleton(WrapperFactory::SCRIPT_ACCESS_ONLY_FLAG | @@ -147,8 +145,6 @@ template<> SCSOW SCSOW::singleton(WrapperFactory::SCRIPT_ACCESS_ONLY_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); @@ -157,7 +153,5 @@ template class SOW; template class XOW; template class DXOW; template class NNXOW; -template class LW; -template class XLW; template class ChromeObjectWrapperBase; } diff --git a/js/xpconnect/wrappers/WrapperFactory.cpp b/js/xpconnect/wrappers/WrapperFactory.cpp index d17c1c75ab49..7f86bbfed034 100644 --- a/js/xpconnect/wrappers/WrapperFactory.cpp +++ b/js/xpconnect/wrappers/WrapperFactory.cpp @@ -301,8 +301,7 @@ DEBUG_CheckUnwrapSafety(JSObject *obj, js::Wrapper *handler, 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. @@ -371,10 +370,7 @@ WrapperFactory::Rewrap(JSContext *cx, JSObject *existing, JSObject *obj, if (targetdata && (wn = GetWrappedNative(cx, obj)) && wn->HasProto() && wn->GetProto()->ClassIsDOMObject()) { - if (IsLocationObject(obj)) - wrapper = &FilteringWrapper::singleton; - else - wrapper = &FilteringWrapper::singleton; + wrapper = &FilteringWrapper::singleton; } else if (mozilla::dom::UseDOMXray(obj)) { wrapper = &FilteringWrapper::singleton; } else if (IsComponentsObject(obj)) { @@ -419,7 +415,6 @@ WrapperFactory::Rewrap(JSContext *cx, JSObject *existing, JSObject *obj, // 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. @@ -430,8 +425,6 @@ WrapperFactory::Rewrap(JSContext *cx, JSObject *existing, JSObject *obj, if (AccessCheck::needsSystemOnlyWrapper(obj)) { wrapper = &FilteringWrapper::singleton; - } else if (IsLocationObject(obj)) { - wrapper = &FilteringWrapper::singleton; } else if (IsComponentsObject(obj)) { wrapper = &FilteringWrapper::singleton; @@ -457,14 +450,8 @@ WrapperFactory::Rewrap(JSContext *cx, JSObject *existing, JSObject *obj, wrapper = &FilteringWrapper::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::singleton; - } else { - wrapper = &FilteringWrapper::singleton; - } + wrapper = &FilteringWrapper::singleton; } } @@ -497,23 +484,6 @@ WrapperFactory::WrapForSameCompartment(JSContext *cx, JSObject *obj) 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::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 diff --git a/js/xpconnect/wrappers/WrapperFactory.h b/js/xpconnect/wrappers/WrapperFactory.h index cc6e2838fe33..86e809068fee 100644 --- a/js/xpconnect/wrappers/WrapperFactory.h +++ b/js/xpconnect/wrappers/WrapperFactory.h @@ -69,12 +69,6 @@ class WrapperFactory { 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);