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 137982 1df3bdadb7ce7dfd3dcabe990ec95bea41df50bb
parent 137981 64f001fe04fb86e0160e4b32cb8401527fed5648
child 137983 7201a22e3c443b4023bc5a51f0c2563f6b872608
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs843829
milestone23.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 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