Bug 834707 - Kill dynamic SOWs. r=gabor
authorBobby Holley <bobbyholley@gmail.com>
Mon, 06 May 2013 19:38:23 -0700
changeset 137869 a28afac94104dd7713950647c3e5d84adc61357d
parent 137868 f6f8e02a9ab01720038bfb9a4ce4996ca8cbc819
child 137870 78b49aecbefa5f4fc962fafb55895d363d403bbc
push idunknown
push userunknown
push dateunknown
reviewersgabor
bugs834707, 825392
milestone23.0a1
Bug 834707 - Kill dynamic SOWs. r=gabor Now that XBL scopes are here to stay (no more pref), we can remove all the machinery that makes SOWs dynamic. We still need SOWs until bug 825392 is fixed, but they can now be totally opaque. One side effect of this patch is that, due to our usage of Opaque, we now allow CALL on SOWs. But this shouldn't be a problem, because SOWs are used for anonymous elements which are not callable (and we probably wouldn't mind it even if they were).
js/src/jswrapper.h
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/AccessCheck.h
js/xpconnect/wrappers/FilteringWrapper.cpp
js/xpconnect/wrappers/FilteringWrapper.h
js/xpconnect/wrappers/WrapperFactory.cpp
js/xpconnect/wrappers/WrapperFactory.h
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -40,17 +40,17 @@ class JS_FRIEND_API(Wrapper) : public Di
 
     /*
      * Wrappers can explicitly specify that they are unsafe to unwrap from a
      * security perspective (as is the case for SecurityWrappers). If a wrapper
      * is not safe to unwrap, operations requiring full access to the underlying
      * object (via CheckedUnwrap) will throw. Otherwise, they will succeed.
      */
     void setSafeToUnwrap(bool safe) { mSafeToUnwrap = safe; }
-    virtual bool isSafeToUnwrap() { return mSafeToUnwrap; }
+    bool isSafeToUnwrap() { return mSafeToUnwrap; }
 
     static JSObject *New(JSContext *cx, JSObject *obj, JSObject *proto,
                          JSObject *parent, Wrapper *handler);
 
     static JSObject *Renew(JSContext *cx, JSObject *existing, JSObject *obj, Wrapper *handler);
 
     static Wrapper *wrapperHandler(JSObject *wrapper);
 
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -253,51 +253,29 @@ AccessCheck::isCrossOriginAccessPermitte
             }
         }
         return IsFrameId(cx, obj, id);
     }
     return false;
 }
 
 bool
-AccessCheck::isSystemOnlyAccessPermitted(JSContext *cx)
-{
-    MOZ_ASSERT(cx == nsContentUtils::GetCurrentJSContext());
-    return nsContentUtils::CanAccessNativeAnon();
-}
-
-bool
 AccessCheck::needsSystemOnlyWrapper(JSObject *obj)
 {
     JSObject* wrapper = obj;
     if (dom::GetSameCompartmentWrapperForDOMBinding(wrapper))
         return wrapper != obj;
 
     if (!IS_WN_WRAPPER(obj))
         return false;
 
     XPCWrappedNative *wn = static_cast<XPCWrappedNative *>(js::GetObjectPrivate(obj));
     return wn->NeedsSOW();
 }
 
-bool
-OnlyIfSubjectIsSystem::isSafeToUnwrap()
-{
-    // It's nasty to use the context stack here, but the alternative is passing cx all
-    // the way down through CheckedUnwrap, which we just undid in a 100k patch. :-(
-    JSContext *cx = nsContentUtils::GetCurrentJSContext();
-    if (!cx)
-        return true;
-    // If XBL scopes are enabled for this compartment, this hook doesn't need to
-    // be dynamic at all, since SOWs can be opaque.
-    if (xpc::AllowXBLScope(js::GetContextCompartment(cx)))
-        return false;
-    return AccessCheck::isSystemOnlyAccessPermitted(cx);
-}
-
 enum Access { READ = (1<<0), WRITE = (1<<1), NO_ACCESS = 0 };
 
 static void
 EnterAndThrow(JSContext *cx, JSObject *wrapper, const char *msg)
 {
     JSAutoCompartment ac(cx, wrapper);
     JS_ReportError(cx, msg);
 }
--- a/js/xpconnect/wrappers/AccessCheck.h
+++ b/js/xpconnect/wrappers/AccessCheck.h
@@ -23,17 +23,16 @@ class AccessCheck {
     static bool wrapperSubsumes(JSObject *wrapper);
     static bool subsumesIgnoringDomain(JSCompartment *a, JSCompartment *b);
     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 isSystemOnlyAccessPermitted(JSContext *cx);
 
     static bool needsSystemOnlyWrapper(JSObject *obj);
 };
 
 struct Policy {
 };
 
 // This policy only allows calling the underlying callable. All other operations throw.
@@ -43,19 +42,16 @@ struct Opaque : public Policy {
     }
     static bool deny(js::Wrapper::Action act) {
         return false;
     }
     static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl)
     {
         return false;
     }
-    static bool isSafeToUnwrap() {
-        return false;
-    }
 };
 
 // This policy is designed to protect privileged callers from untrusted non-
 // Xrayable objects. Nothing is allowed, and nothing throws.
 struct GentlyOpaque : public Policy {
     static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
         return false;
     }
@@ -66,87 +62,52 @@ struct GentlyOpaque : public Policy {
     {
         // We allow nativeCall here because the alternative is throwing (which
         // happens in SecurityWrapper::nativeCall), which we don't want. There's
         // unlikely to be too much harm to letting this through, because this
         // wrapper is only used to wrap less-privileged objects in more-privileged
         // scopes, so unwrapping here only drops privileges.
         return true;
     }
-
-    static bool isSafeToUnwrap() {
-        return false;
-    }
-};
-
-// This policy only permits access to the object if the subject can touch
-// system objects.
-struct OnlyIfSubjectIsSystem : public Policy {
-    static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) {
-        return AccessCheck::isSystemOnlyAccessPermitted(cx);
-    }
-
-    static bool deny(js::Wrapper::Action act) {
-        return false;
-    }
-
-    static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl)
-    {
-        return AccessCheck::isSystemOnlyAccessPermitted(cx);
-    }
-
-    static bool isSafeToUnwrap();
 };
 
 // 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) {
         return AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act);
     }
     static bool deny(js::Wrapper::Action act) {
         return false;
     }
     static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl)
     {
         return false;
     }
-
-    static bool isSafeToUnwrap() {
-        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(js::Wrapper::Action act) {
         // Fail silently for GETs.
         return act == js::Wrapper::GET;
     }
     static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl);
-
-    static bool isSafeToUnwrap() {
-        return false;
-    }
 };
 
 // Components specific policy
 struct ComponentsObjectPolicy : public Policy {
     static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act);
 
     static bool deny(js::Wrapper::Action act) {
         return false;
     }
     static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl) {
         return false;
     }
-
-    static bool isSafeToUnwrap() {
-        return false;
-    }
 };
 
 }
 
 #endif /* __AccessCheck_h__ */
--- a/js/xpconnect/wrappers/FilteringWrapper.cpp
+++ b/js/xpconnect/wrappers/FilteringWrapper.cpp
@@ -24,23 +24,16 @@ FilteringWrapper<Base, Policy>::Filterin
 {
 }
 
 template <typename Base, typename Policy>
 FilteringWrapper<Base, Policy>::~FilteringWrapper()
 {
 }
 
-template <typename Base, typename Policy>
-bool
-FilteringWrapper<Base, Policy>::isSafeToUnwrap()
-{
-    return Policy::isSafeToUnwrap();
-}
-
 template <typename Policy>
 static bool
 Filter(JSContext *cx, HandleObject wrapper, AutoIdVector &props)
 {
     size_t w = 0;
     RootedId id(cx);
     for (size_t n = 0; n < props.length(); ++n) {
         id = props[n];
@@ -192,34 +185,33 @@ FilteringWrapper<Base, Policy>::enter(JS
     if (!Policy::check(cx, wrapper, id, act)) {
         *bp = JS_IsExceptionPending(cx) ? false : Policy::deny(act);
         return false;
     }
     *bp = true;
     return true;
 }
 
-#define SOW FilteringWrapper<CrossCompartmentSecurityWrapper, OnlyIfSubjectIsSystem>
-#define SCSOW FilteringWrapper<SameCompartmentSecurityWrapper, OnlyIfSubjectIsSystem>
+// NB: don't need SOW here because the resulting wrapper would be identical to
+// NNXOW.
+#define SCSOW FilteringWrapper<SameCompartmentSecurityWrapper, Opaque>
 #define XOW FilteringWrapper<SecurityXrayXPCWN, CrossOriginAccessiblePropertiesOnly>
 #define DXOW   FilteringWrapper<SecurityXrayDOM, CrossOriginAccessiblePropertiesOnly>
 #define NNXOW FilteringWrapper<CrossCompartmentSecurityWrapper, Opaque>
 #define CW FilteringWrapper<SameCompartmentSecurityWrapper, ComponentsObjectPolicy>
 #define XCW FilteringWrapper<CrossCompartmentSecurityWrapper, ComponentsObjectPolicy>
 #define GO FilteringWrapper<CrossCompartmentSecurityWrapper, GentlyOpaque>
-template<> SOW SOW::singleton(WrapperFactory::SOW_FLAG);
-template<> SCSOW SCSOW::singleton(WrapperFactory::SOW_FLAG);
+template<> SCSOW SCSOW::singleton(0);
 template<> XOW XOW::singleton(0);
 template<> DXOW DXOW::singleton(0);
 template<> NNXOW NNXOW::singleton(0);
 
 template<> CW CW::singleton(0);
 template<> XCW XCW::singleton(0);
 
 template<> GO GO::singleton(0);
 
-template class SOW;
 template class XOW;
 template class DXOW;
 template class NNXOW;
 template class ChromeObjectWrapperBase;
 template class GO;
 }
--- a/js/xpconnect/wrappers/FilteringWrapper.h
+++ b/js/xpconnect/wrappers/FilteringWrapper.h
@@ -14,19 +14,16 @@
 namespace xpc {
 
 template <typename Base, typename Policy>
 class FilteringWrapper : public Base {
   public:
     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, JS::Handle<JSObject*> wrapper,
                                        JS::Handle<jsid> id, js::PropertyDescriptor *desc,
                                        unsigned flags) MOZ_OVERRIDE;
     virtual bool getOwnPropertyDescriptor(JSContext *cx, JS::Handle<JSObject*> wrapper,
                                           JS::Handle<jsid> id, js::PropertyDescriptor *desc,
                                           unsigned flags) MOZ_OVERRIDE;
     virtual bool getOwnPropertyNames(JSContext *cx, JS::Handle<JSObject*> wrapper,
                                      js::AutoIdVector &props) MOZ_OVERRIDE;
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -284,23 +284,19 @@ WrapperFactory::PrepareForWrapping(JSCon
 #ifdef DEBUG
 static void
 DEBUG_CheckUnwrapSafety(JSObject *obj, js::Wrapper *handler,
                         JSCompartment *origin, JSCompartment *target)
 {
     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::IsComponentsObject(obj))
-    {
+    } else if (WrapperFactory::IsComponentsObject(obj)) {
         // The Components object that is restricted regardless of origin.
         MOZ_ASSERT(!handler->isSafeToUnwrap());
-    } else if (AccessCheck::needsSystemOnlyWrapper(obj)) {
-        // SOWs have a dynamic unwrap check, so we can't really say anything useful
-        // about them here :-(
     } else if (handler == &FilteringWrapper<CrossCompartmentSecurityWrapper, GentlyOpaque>::singleton) {
         // We explicitly use a SecurityWrapper to protect privileged callers from
         // less-privileged objects that they should never see. Skip the check in
         // this case.
     } else {
         // Otherwise, it should depend on whether the target subsumes the origin.
         MOZ_ASSERT(handler->isSafeToUnwrap() == AccessCheck::subsumes(target, origin));
     }
@@ -396,18 +392,17 @@ WrapperFactory::Rewrap(JSContext *cx, Ha
     // remote-XUL whitelisted domains, since they don't have XBL scopes.
     } else if (IsComponentsObject(obj) && !AccessCheck::isChrome(target)) {
         wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper,
                                     ComponentsObjectPolicy>::singleton;
     } else if (AccessCheck::needsSystemOnlyWrapper(obj) &&
                xpc::AllowXBLScope(target) &&
                !(targetIsChrome || (targetSubsumesOrigin && nsContentUtils::IsCallerXBL())))
     {
-        wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper,
-                                    OnlyIfSubjectIsSystem>::singleton;
+        wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper, Opaque>::singleton;
     }
 
     // Normally, a non-xrayable non-waived content object that finds itself in
     // a privileged scope is wrapped with a CrossCompartmentWrapper, even though
     // the lack of a waiver _really_ should give it an opaque wrapper. This is
     // a bit too entrenched to change for content-chrome, but we can at least fix
     // it for XBL scopes.
     //
@@ -570,17 +565,17 @@ WrapperFactory::WrapSOWObject(JSContext 
     // XUL domain, in which we can't have SOWs. We should never be called in
     // that case.
     MOZ_ASSERT(xpc::AllowXBLScope(js::GetContextCompartment(cx)));
     if (!JS_GetPrototype(cx, obj, proto.address()))
         return NULL;
     JSObject *wrapperObj =
         Wrapper::New(cx, obj, proto, JS_GetGlobalForObject(cx, obj),
                      &FilteringWrapper<SameCompartmentSecurityWrapper,
-                     OnlyIfSubjectIsSystem>::singleton);
+                     Opaque>::singleton);
     return wrapperObj;
 }
 
 bool
 WrapperFactory::IsComponentsObject(JSObject *obj)
 {
     const char *name = js::GetObjectClass(obj)->name;
     return name[0] == 'n' && !strcmp(name, "nsXPCComponents");
--- a/js/xpconnect/wrappers/WrapperFactory.h
+++ b/js/xpconnect/wrappers/WrapperFactory.h
@@ -11,18 +11,17 @@
 #include "jsapi.h"
 #include "jswrapper.h"
 
 namespace xpc {
 
 class WrapperFactory {
   public:
     enum { WAIVE_XRAY_WRAPPER_FLAG = js::Wrapper::LAST_USED_FLAG << 1,
-           IS_XRAY_WRAPPER_FLAG    = WAIVE_XRAY_WRAPPER_FLAG << 1,
-           SOW_FLAG                = IS_XRAY_WRAPPER_FLAG << 1 };
+           IS_XRAY_WRAPPER_FLAG    = WAIVE_XRAY_WRAPPER_FLAG << 1 };
 
     // Return true if any of any of the nested wrappers have the flag set.
     static bool HasWrapperFlag(JSObject *wrapper, unsigned flag) {
         unsigned flags = 0;
         js::UncheckedUnwrap(wrapper, true, &flags);
         return !!(flags & flag);
     }