Bug 843829 - Wrap unwaived content JS objects in opaque wrappers for XBL scopes. r=mrbkap
☠☠ backed out by e60919ded783 ☠ ☠
authorBobby Holley <bobbyholley@gmail.com>
Tue, 02 Apr 2013 18:51:20 -0700
changeset 127133 1df3bdadb7ce
parent 127132 64f001fe04fb
child 127134 7201a22e3c44
push id117
push usertomi.aarnio@nokia.com
push dateWed, 03 Apr 2013 12:07:07 +0000
reviewersmrbkap
bugs843829
milestone23.0a1
Bug 843829 - Wrap unwaived content JS objects in opaque wrappers for XBL scopes. r=mrbkap
content/xbl/test/file_bug821850.xhtml
js/xpconnect/wrappers/AccessCheck.h
js/xpconnect/wrappers/FilteringWrapper.cpp
js/xpconnect/wrappers/FilteringWrapper.h
js/xpconnect/wrappers/WrapperFactory.cpp
--- a/content/xbl/test/file_bug821850.xhtml
+++ b/content/xbl/test/file_bug821850.xhtml
@@ -66,16 +66,30 @@ https://bugzilla.mozilla.org/show_bug.cg
         <field name="objectField">({ foo: 2, bar: {a: 1} })</field>
         <field name="contentField">XPCNativeWrapper.unwrap(window).contentVal</field>
         <method name="method">
           <parameter name="arg" />
           <body>
             return "method:" + arg;
           </body>
         </method>
+        <method name="passMeAJSObject">
+          <parameter name="arg" />
+          <body>
+            is(typeof arg.prop, 'undefined', "No properties");
+            is(Object.getOwnPropertyNames(arg).length, 0, "Should have no own properties");
+            try {
+              arg.foo = 2;
+              ok(true, "Stuff fails silently");
+            } catch (e) {
+              ok(false, "Stuff should fail silently");
+            }
+            is(typeof arg.foo, 'undefined', "Shouldn't place props");
+          </body>
+        </method>
         <property name="prop">
           <getter>return this._prop;</getter>
           <setter>this._prop = "set:" + val;</setter>
         </property>
       </implementation>
       <handlers>
         <handler event="testevent" action="ok(true, 'called event handler'); finish();"/>
       </handlers>
@@ -115,21 +129,24 @@ https://bugzilla.mozilla.org/show_bug.cg
     is(bound.primitiveField, 2, "Can see primitive fields");
     is(typeof bound.objectField, "object", "objectField exists");
     checkThrows(function() bound.objectField.foo);
     is(bound.method("foo"), "method:foo", "Can invoke XBL method from content");
     is(bound.prop, "propVal", "Can access properties from content");
     bound.prop = "someOtherVal";
     is(bound.prop, "set:someOtherVal", "Can set properties from content");
 
+    // Make sure we can't pass JS objects to the XBL scope.
+    var proto = bound.__proto__;
+    proto.passMeAJSObject({prop: 2});
+
     //
     // Try sticking a bunch of stuff on the prototype object.
     //
 
-    var proto = bound.__proto__;
     proto.someExpando = 201;
     is(bound.someExpando, 201, "Can stick non-XBL properties on the proto");
 
     // Previously, this code checked that content couldn't tamper with its XBL
     // prototype. But we decided to allow this to reduce regression risk, so for
     // now just check that this works.
     function checkMayTamper(obj, propName, desc) {
       var accessor = !('value' in Object.getOwnPropertyDescriptor(obj, propName));
--- a/js/xpconnect/wrappers/AccessCheck.h
+++ b/js/xpconnect/wrappers/AccessCheck.h
@@ -48,16 +48,40 @@ struct Opaque : public Policy {
     {
         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;
+    }
+    static bool deny(js::Wrapper::Action act) {
+        return true;
+    }
+    static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl)
+    {
+        // 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) {
--- a/js/xpconnect/wrappers/FilteringWrapper.cpp
+++ b/js/xpconnect/wrappers/FilteringWrapper.cpp
@@ -140,16 +140,40 @@ FilteringWrapper<Base, Policy>::nativeCa
 {
     if (Policy::allowNativeCall(cx, test, impl))
         return Base::Permissive::nativeCall(cx, test, impl, args);
     return Base::Restrictive::nativeCall(cx, test, impl, args);
 }
 
 template <typename Base, typename Policy>
 bool
+FilteringWrapper<Base, Policy>::defaultValue(JSContext *cx, JS::Handle<JSObject*> obj,
+                                             JSType hint, MutableHandleValue vp)
+{
+    return Base::defaultValue(cx, obj, hint, vp);
+}
+
+// With our entirely-opaque wrapper, the DefaultValue algorithm throws,
+// causing spurious exceptions. Manually implement something benign.
+template<>
+bool
+FilteringWrapper<CrossCompartmentSecurityWrapper, GentlyOpaque>
+                ::defaultValue(JSContext *cx, JS::Handle<JSObject*> obj,
+                               JSType hint, MutableHandleValue vp)
+{
+    JSString *str = JS_NewStringCopyZ(cx, "[Opaque]");
+    if (!str)
+        return false;
+    vp.set(JS::StringValue(str));
+    return true;
+}
+
+
+template <typename Base, typename Policy>
+bool
 FilteringWrapper<Base, Policy>::enter(JSContext *cx, JS::Handle<JSObject*> wrapper,
                                       JS::Handle<jsid> id, Wrapper::Action act, bool *bp)
 {
     // This is a super ugly hacky to get around Xray Resolve wonkiness.
     //
     // Basically, XPCWN Xrays sometimes call into the Resolve hook of the
     // scriptable helper, and pass the wrapper itself as the object upon which
     // the resolve is happening. Then, special handling happens in
@@ -174,23 +198,27 @@ FilteringWrapper<Base, Policy>::enter(JS
 
 #define SOW FilteringWrapper<CrossCompartmentSecurityWrapper, OnlyIfSubjectIsSystem>
 #define SCSOW FilteringWrapper<SameCompartmentSecurityWrapper, OnlyIfSubjectIsSystem>
 #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<> 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
@@ -34,16 +34,18 @@ class FilteringWrapper : public Base {
                            js::AutoIdVector &props) MOZ_OVERRIDE;
     virtual bool keys(JSContext *cx, JS::Handle<JSObject*> wrapper,
                       js::AutoIdVector &props) MOZ_OVERRIDE;
     virtual bool iterate(JSContext *cx, JS::Handle<JSObject*> wrapper, unsigned flags,
                          JS::MutableHandle<JS::Value> vp) MOZ_OVERRIDE;
     virtual bool nativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl,
                             JS::CallArgs args) MOZ_OVERRIDE;
 
+    virtual bool defaultValue(JSContext *cx, JS::Handle<JSObject*> obj, JSType hint, JS::MutableHandleValue vp) MOZ_OVERRIDE;
+
     virtual bool enter(JSContext *cx, JS::Handle<JSObject*> wrapper, JS::Handle<jsid> id,
                        js::Wrapper::Action act, bool *bp) MOZ_OVERRIDE;
 
     static FilteringWrapper singleton;
 };
 
 }
 
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -288,16 +288,20 @@ DEBUG_CheckUnwrapSafety(JSObject *obj, j
         MOZ_ASSERT(handler->isSafeToUnwrap());
     } 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));
     }
 }
 #else
 #define DEBUG_CheckUnwrapSafety(obj, handler, origin, target) {}
 #endif
@@ -356,16 +360,17 @@ WrapperFactory::Rewrap(JSContext *cx, JS
     JSCompartment *origin = js::GetObjectCompartment(obj);
     JSCompartment *target = js::GetContextCompartment(cx);
     bool originIsChrome = AccessCheck::isChrome(origin);
     bool targetIsChrome = AccessCheck::isChrome(target);
     bool originSubsumesTarget = AccessCheck::subsumes(origin, target);
     bool targetSubsumesOrigin = AccessCheck::subsumes(target, origin);
     bool sameOrigin = targetSubsumesOrigin && originSubsumesTarget;
     XrayType xrayType = GetXrayType(obj);
+    bool waiveXrayFlag = flags & WAIVE_XRAY_WRAPPER_FLAG;
 
     // By default we use the wrapped proto of the underlying object as the
     // prototype for our wrapper, but we may select something different below.
     JSObject *proxyProto = wrappedProto;
 
     Wrapper *wrapper;
     CompartmentPrivate *targetdata = EnsureCompartmentPrivate(target);
 
@@ -390,16 +395,30 @@ WrapperFactory::Rewrap(JSContext *cx, JS
                                     ComponentsObjectPolicy>::singleton;
     } else if (AccessCheck::needsSystemOnlyWrapper(obj) &&
                !(targetIsChrome || (targetSubsumesOrigin && nsContentUtils::IsCallerXBL())))
     {
         wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper,
                                     OnlyIfSubjectIsSystem>::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.
+    //
+    // See bug 843829.
+    else if (targetSubsumesOrigin && !originSubsumesTarget &&
+             !waiveXrayFlag && xrayType == NotXray &&
+             IsXBLScope(target))
+    {
+        wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper, GentlyOpaque>::singleton;
+    }
+
     //
     // Now, handle the regular cases.
     //
     // These are wrappers we can compute using a rule-based approach. In order
     // to do so, we need to compute some parameters.
     //
     else {
 
@@ -412,18 +431,17 @@ WrapperFactory::Rewrap(JSContext *cx, JS
         // and the caller has not requested same-origin Xrays.
         //
         // Xrays are a bidirectional protection, since it affords clarity to the
         // caller and privacy to the callee.
         bool wantXrays = !(sameOrigin && !targetdata->wantXrays);
 
         // If Xrays are warranted, the caller may waive them for non-security
         // wrappers.
-        bool waiveXrays = wantXrays && !securityWrapper &&
-                          (flags & WAIVE_XRAY_WRAPPER_FLAG);
+        bool waiveXrays = wantXrays && !securityWrapper && waiveXrayFlag;
 
         wrapper = SelectWrapper(securityWrapper, wantXrays, xrayType, waiveXrays);
     }
 
 
 
     // If the prototype of a chrome object being wrapped in content is a prototype
     // for a standard class, use the one from the content compartment so