Bug 505915 - Throw exceptions eagerly when we try to convert a cross-origin security wrapper to a C++ object. r=jst
authorBlake Kaplan <mrbkap@gmail.com>
Wed, 02 Sep 2009 17:25:25 -0700
changeset 33306 0071000d8c4ceb7afc73b3e16c181f2d9b8636f8
parent 33305 70475458eabb6b54c966917bc1df9146e19e671b
child 33307 26a9e4b405a080076a79efdcb172d63633514a2a
push idunknown
push userunknown
push dateunknown
reviewersjst
bugs505915
milestone1.9.3a1pre
Bug 505915 - Throw exceptions eagerly when we try to convert a cross-origin security wrapper to a C++ object. r=jst
js/src/xpconnect/src/XPCWrapper.h
js/src/xpconnect/src/xpcconvert.cpp
js/src/xpconnect/src/xpcquickstubs.cpp
js/src/xpconnect/tests/mochitest/Makefile.in
js/src/xpconnect/tests/mochitest/test_bug505915.html
--- a/js/src/xpconnect/src/XPCWrapper.h
+++ b/js/src/xpconnect/src/XPCWrapper.h
@@ -210,16 +210,25 @@ public:
                                      (JS_GetContextPrivate(cx)));
       if (scriptNotify) {
         return NS_SUCCEEDED(scriptNotify->PreserveWrapper(wn));
       }
     }
     return JS_TRUE;
   }
 
+  static JSBool IsSecurityWrapper(JSObject *wrapper)
+  {
+    JSClass *clasp = STOBJ_GET_CLASS(wrapper);
+    return clasp == &sXPC_COW_JSClass.base ||
+           clasp == &sXPC_SJOW_JSClass.base ||
+           clasp == &sXPC_SOW_JSClass.base ||
+           clasp == &sXPC_XOW_JSClass.base;
+  }
+
   /**
    * Given an arbitrary object, Unwrap will return the wrapped object if the
    * passed-in object is a wrapper that Unwrap knows about *and* the
    * currently running code has permission to access both the wrapper and
    * wrapped object.
    *
    * Since this is meant to be called from functions like
    * XPCWrappedNative::GetWrappedNativeOfJSObject, it does not set an
--- a/js/src/xpconnect/src/xpcconvert.cpp
+++ b/js/src/xpconnect/src/xpcconvert.cpp
@@ -1454,35 +1454,51 @@ XPCConvert::JSObject2NativeInterface(XPC
 
     if(!aOuter)
     {
         // Note that if we have a non-null aOuter then it means that we are
         // forcing the creation of a wrapper even if the object *is* a 
         // wrappedNative or other wise has 'nsISupportness'. 
         // This allows wrapJSAggregatedToNative to work.
 
+        // If we're looking at a security wrapper, see now if we're allowed to
+        // pass it to C++. If we are, then fall through to the code below. If
+        // we aren't, throw an exception eagerly.
+        JSObject* inner = nsnull;
+        if(XPCWrapper::IsSecurityWrapper(src))
+        {
+            inner = XPCWrapper::Unwrap(cx, src);
+            if(!inner)
+            {
+                if(pErr)
+                    *pErr = NS_ERROR_XPC_SECURITY_MANAGER_VETO;
+                return JS_FALSE;
+            }
+        }
+
         // Is this really a native xpcom object with a wrapper?
         XPCWrappedNative* wrappedNative =
-                    XPCWrappedNative::GetWrappedNativeOfJSObject(cx, src);
+                    XPCWrappedNative::GetWrappedNativeOfJSObject(cx,
+                                                                 inner
+                                                                 ? inner
+                                                                 : src);
         if(wrappedNative)
         {
             iface = wrappedNative->GetIdentityObject();
             return NS_SUCCEEDED(iface->QueryInterface(*iid, dest));
         }
         // else...
-        
+
         // XXX E4X breaks the world. Don't try wrapping E4X objects!
         // This hack can be removed (or changed accordingly) when the
         // DOM <-> E4X bindings are complete, see bug 270553
         if(JS_TypeOfValue(cx, OBJECT_TO_JSVAL(src)) == JSTYPE_XML)
             return JS_FALSE;
 
-        // Does the JSObject have 'nsISupportness'?
-        // XXX hmm, I wonder if this matters anymore with no 
-        // oldstyle DOM objects around.
+        // Deal with slim wrappers here.
         if(GetISupportsFromJSObject(src, &iface))
         {
             if(iface)
                 return NS_SUCCEEDED(iface->QueryInterface(*iid, dest));
 
             return JS_FALSE;
         }
     }
--- a/js/src/xpconnect/src/xpcquickstubs.cpp
+++ b/js/src/xpconnect/src/xpcquickstubs.cpp
@@ -809,16 +809,23 @@ xpc_qsUnwrapThisImpl(JSContext *cx,
                      JSObject *obj,
                      JSObject *callee,
                      const nsIID &iid,
                      void **ppThis,
                      nsISupports **pThisRef,
                      jsval *vp,
                      XPCLazyCallContext *lccx)
 {
+    if(XPCWrapper::IsSecurityWrapper(obj))
+    {
+        obj = XPCWrapper::Unwrap(cx, obj);
+        if(!obj)
+            return xpc_qsThrow(cx, NS_ERROR_XPC_SECURITY_MANAGER_VETO);
+    }
+
     JSObject *cur = obj;
     XPCWrappedNativeTearOff *tearoff;
     XPCWrappedNative *wrapper =
         XPCWrappedNative::GetWrappedNativeOfJSObject(cx, obj, callee, &cur,
                                                      &tearoff);
     if(wrapper)
     {
         nsresult rv = getNativeFromWrapper(wrapper, iid, ppThis, pThisRef, vp);
@@ -897,19 +904,27 @@ xpc_qsUnwrapArgImpl(JSContext *cx,
     {
         nsISupports *iface = static_cast<nsISupports*>(xpc_GetJSPrivate(src));
         if(NS_FAILED(getNative(iface, GetOffsetsFromSlimWrapper(src),
                                src, iid, ppArg, ppArgRef, vp)))
             return NS_ERROR_XPC_BAD_CONVERT_JS;
         return NS_OK;
     }
 
+    JSObject *inner = nsnull;
+    if(XPCWrapper::IsSecurityWrapper(src))
+    {
+        inner = XPCWrapper::Unwrap(cx, src);
+        if(!inner)
+            return NS_ERROR_XPC_SECURITY_MANAGER_VETO;
+    }
+
     // From XPCConvert::JSObject2NativeInterface
     XPCWrappedNative* wrappedNative =
-        XPCWrappedNative::GetWrappedNativeOfJSObject(cx, src);
+        XPCWrappedNative::GetWrappedNativeOfJSObject(cx, inner ? inner : src);
     nsISupports *iface;
     if(wrappedNative)
     {
         iface = wrappedNative->GetIdentityObject();
         if(NS_FAILED(getNativeFromWrapper(wrappedNative, iid, ppArg, ppArgRef,
                                           vp)))
             return NS_ERROR_XPC_BAD_CONVERT_JS;
         return NS_OK;
@@ -921,19 +936,17 @@ xpc_qsUnwrapArgImpl(JSContext *cx,
     // This hack can be removed (or changed accordingly) when the
     // DOM <-> E4X bindings are complete, see bug 270553
     if(JS_TypeOfValue(cx, OBJECT_TO_JSVAL(src)) == JSTYPE_XML)
     {
         *ppArgRef = nsnull;
         return NS_ERROR_XPC_BAD_CONVERT_JS;
     }
 
-    // Does the JSObject have 'nsISupportness'?
-    // XXX hmm, I wonder if this matters anymore with no
-    // oldstyle DOM objects around.
+    // Try to unwrap a slim wrapper.
     if(XPCConvert::GetISupportsFromJSObject(src, &iface))
     {
         if(!iface || NS_FAILED(iface->QueryInterface(iid, ppArg)))
         {
             *ppArgRef = nsnull;
             return NS_ERROR_XPC_BAD_CONVERT_JS;
         }
 
--- a/js/src/xpconnect/tests/mochitest/Makefile.in
+++ b/js/src/xpconnect/tests/mochitest/Makefile.in
@@ -59,13 +59,14 @@ include $(topsrcdir)/config/rules.mk
 		test_bug446584.html \
 		test_bug462428.html \
 		test_bug478438.html \
 		test_bug484107.html \
 		test_bug484459.html \
 		test_bug500691.html \
 		test_bug502959.html \
 		test_bug503926.html \
+		test_bug505915.html \
 		test_bug517163.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $^ $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/js/src/xpconnect/tests/mochitest/test_bug505915.html
@@ -0,0 +1,65 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=505915
+-->
+<head>
+  <title>Test for Bug 505915</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=505915">Mozilla Bug 505915</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 505915 **/
+
+function go() {
+    var ifr = $('ifr');
+    try {
+        document.documentElement.appendChild(ifr.contentWindow);
+        ok(false, "weird behavior");
+    } catch (e) {
+        ok(/NS_ERROR_XPC_SECURITY_MANAGER_VETO/.test(e),
+           "threw a security exception instead of an invalid child exception");
+    }
+
+    try {
+        document.createTreeWalker(ifr.contentDocument, 0, null, false);
+        ok(false, "should have thrown a security exception");
+    } catch (e) {
+        ok(/NS_ERROR_XPC_SECURITY_MANAGER_VETO/.test(e),
+           "threw a security exception instead of an invalid child exception");
+    }
+
+    var xhr = new XMLHttpRequest();
+    var orsc = new XPCSafeJSObjectWrapper(function(){});
+    xhr.onreadystatechange = orsc;
+    // we only can test that the above didn't throw (which it shouldn't).
+
+    try {
+        xhr.onreadystatechange = ifr.contentWindow;
+        ok(false, "weird behavior");
+    } catch (e) {
+        ok(/NS_ERROR_XPC_SECURITY_MANAGER_VETO/.test(e),
+           "threw a security exception instead of an invalid child exception");
+    }
+
+    SimpleTest.finish();
+}
+
+SimpleTest.waitForExplicitFinish();
+
+</script>
+</pre>
+
+<iframe id="ifr" onload="go()" src="http://example.org/"></iframe>
+
+</body>
+</html>