From 13040f75564b14e244375c8ce65978ee84088dba Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 21 Mar 2013 08:20:41 -0700 Subject: [PATCH] Bug 658909 - Make isSafeToUnwrap pseudo-dynamic for SOWs. r=mrbkap This can go away as soon as XBL scopes are no longer behind a pref. --- js/src/jswrapper.h | 2 +- js/xpconnect/wrappers/AccessCheck.cpp | 13 +++++++++++++ js/xpconnect/wrappers/AccessCheck.h | 17 +++++++++++++++++ js/xpconnect/wrappers/FilteringWrapper.cpp | 7 +++++++ js/xpconnect/wrappers/FilteringWrapper.h | 3 +++ js/xpconnect/wrappers/WrapperFactory.cpp | 7 ++++--- 6 files changed, 45 insertions(+), 4 deletions(-) diff --git a/js/src/jswrapper.h b/js/src/jswrapper.h index d8cd4a064f9c..9e4c1bc8f05b 100644 --- a/js/src/jswrapper.h +++ b/js/src/jswrapper.h @@ -46,7 +46,7 @@ class JS_FRIEND_API(Wrapper) : public DirectProxyHandler * object (via UnwrapObjectChecked) will throw. Otherwise, they will succeed. */ void setSafeToUnwrap(bool safe) { mSafeToUnwrap = safe; } - bool isSafeToUnwrap() { return mSafeToUnwrap; } + virtual bool isSafeToUnwrap() { return mSafeToUnwrap; } static JSObject *New(JSContext *cx, JSObject *obj, JSObject *proto, JSObject *parent, Wrapper *handler); diff --git a/js/xpconnect/wrappers/AccessCheck.cpp b/js/xpconnect/wrappers/AccessCheck.cpp index a5153a583875..91a3a00311bf 100644 --- a/js/xpconnect/wrappers/AccessCheck.cpp +++ b/js/xpconnect/wrappers/AccessCheck.cpp @@ -269,6 +269,19 @@ AccessCheck::isScriptAccessOnly(JSContext *cx, JSObject *wrapper) return false; } +bool +OnlyIfSubjectIsSystem::isSafeToUnwrap() +{ + if (XPCJSRuntime::Get()->XBLScopesEnabled()) + return false; + // It's nasty to use the context stack here, but the alternative is passing cx all + // the way down through UnwrapObjectChecked, which we just undid in a 100k patch. :-( + JSContext *cx = nsContentUtils::GetCurrentJSContext(); + if (!cx) + return true; + return AccessCheck::isSystemOnlyAccessPermitted(cx); +} + enum Access { READ = (1<<0), WRITE = (1<<1), NO_ACCESS = 0 }; static bool diff --git a/js/xpconnect/wrappers/AccessCheck.h b/js/xpconnect/wrappers/AccessCheck.h index dd95e0407316..c426a688a939 100644 --- a/js/xpconnect/wrappers/AccessCheck.h +++ b/js/xpconnect/wrappers/AccessCheck.h @@ -50,6 +50,9 @@ struct Opaque : public Policy { { return false; } + static bool isSafeToUnwrap() { + return false; + } }; // This policy only permits access to the object if the subject can touch @@ -67,6 +70,8 @@ struct OnlyIfSubjectIsSystem : public Policy { { return AccessCheck::isSystemOnlyAccessPermitted(cx); } + + static bool isSafeToUnwrap(); }; // This policy only permits access to properties that are safe to be used @@ -82,6 +87,10 @@ struct CrossOriginAccessiblePropertiesOnly : public Policy { { return false; } + + static bool isSafeToUnwrap() { + return false; + } }; // This policy only permits access to properties if they appear in the @@ -94,6 +103,10 @@ struct ExposedPropertiesOnly : public Policy { return act == js::Wrapper::GET; } static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl); + + static bool isSafeToUnwrap() { + return false; + } }; // Components specific policy @@ -106,6 +119,10 @@ struct ComponentsObjectPolicy : public Policy { static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl) { return false; } + + static bool isSafeToUnwrap() { + return false; + } }; } diff --git a/js/xpconnect/wrappers/FilteringWrapper.cpp b/js/xpconnect/wrappers/FilteringWrapper.cpp index d8d06841f927..5ba3b356b2a0 100644 --- a/js/xpconnect/wrappers/FilteringWrapper.cpp +++ b/js/xpconnect/wrappers/FilteringWrapper.cpp @@ -29,6 +29,13 @@ FilteringWrapper::~FilteringWrapper() { } +template +bool +FilteringWrapper::isSafeToUnwrap() +{ + return Policy::isSafeToUnwrap(); +} + template static bool Filter(JSContext *cx, JSObject *wrapper, AutoIdVector &props) diff --git a/js/xpconnect/wrappers/FilteringWrapper.h b/js/xpconnect/wrappers/FilteringWrapper.h index b084da305292..c02fc80bb07d 100644 --- a/js/xpconnect/wrappers/FilteringWrapper.h +++ b/js/xpconnect/wrappers/FilteringWrapper.h @@ -19,6 +19,9 @@ class FilteringWrapper : public Base { FilteringWrapper(unsigned flags); virtual ~FilteringWrapper(); + // This is potentially dynamic until XBL scopes are no longer behind a pref. + virtual bool isSafeToUnwrap(); + virtual bool getPropertyDescriptor(JSContext *cx, JSObject *wrapper, jsid id, js::PropertyDescriptor *desc, unsigned flags) MOZ_OVERRIDE; virtual bool getOwnPropertyDescriptor(JSContext *cx, JSObject *wrapper, jsid id, js::PropertyDescriptor *desc, unsigned flags) MOZ_OVERRIDE; virtual bool getOwnPropertyNames(JSContext *cx, JSObject *wrapper, js::AutoIdVector &props) MOZ_OVERRIDE; diff --git a/js/xpconnect/wrappers/WrapperFactory.cpp b/js/xpconnect/wrappers/WrapperFactory.cpp index 057953ed553b..ad5f32d16326 100644 --- a/js/xpconnect/wrappers/WrapperFactory.cpp +++ b/js/xpconnect/wrappers/WrapperFactory.cpp @@ -284,8 +284,8 @@ DEBUG_CheckUnwrapSafety(JSObject *obj, js::Wrapper *handler, // The Components object that is restricted regardless of origin. MOZ_ASSERT(!handler->isSafeToUnwrap()); } else if (AccessCheck::needsSystemOnlyWrapper(obj)) { - // SOWs are opaque to everyone but Chrome and XBL scopes. - MOZ_ASSERT(handler->isSafeToUnwrap() == nsContentUtils::CanAccessNativeAnon()); + // SOWs have a dynamic unwrap check, so we can't really say anything useful + // about them here :-( } else { // Otherwise, it should depend on whether the target subsumes the origin. MOZ_ASSERT(handler->isSafeToUnwrap() == AccessCheck::subsumes(target, origin)); @@ -499,7 +499,8 @@ WrapperFactory::WrapForSameCompartment(JSContext *cx, JSObject *obj) // The WN knows what to do. JSObject *wrapper = wn->GetSameCompartmentSecurityWrapper(cx); - MOZ_ASSERT_IF(wrapper != obj, !Wrapper::wrapperHandler(wrapper)->isSafeToUnwrap()); + MOZ_ASSERT_IF(wrapper != obj && IsComponentsObject(js::UnwrapObject(obj)), + !Wrapper::wrapperHandler(wrapper)->isSafeToUnwrap()); return wrapper; }