From 84dc68b734d368601deccd4d80179c8a2bdf0e96 Mon Sep 17 00:00:00 2001
From: Andreas Gal <gal@mozilla.com>
Date: Sun, 10 Oct 2010 15:36:41 -0700
Subject: [PATCH] bug 580128 - Clean up our passing from JS-into-C++ story.
 r=peterv/mrbkap

---
 js/src/xpconnect/src/XPCWrapper.cpp           |  8 ++--
 js/src/xpconnect/src/XPCWrapper.h             |  2 +-
 js/src/xpconnect/src/xpcjsruntime.cpp         |  3 +-
 js/src/xpconnect/src/xpcprivate.h             |  1 +
 js/src/xpconnect/tests/mochitest/Makefile.in  |  1 +
 .../tests/mochitest/file_bug505915.html       | 10 ++++
 .../tests/mochitest/test_bug505915.html       | 48 ++++++++++++++++++-
 js/src/xpconnect/wrappers/AccessCheck.cpp     | 10 ++--
 js/src/xpconnect/wrappers/AccessCheck.h       |  2 +-
 .../xpconnect/wrappers/FilteringWrapper.cpp   |  5 +-
 js/src/xpconnect/wrappers/WrapperFactory.cpp  | 28 +++++++++++
 js/src/xpconnect/wrappers/WrapperFactory.h    |  5 +-
 12 files changed, 104 insertions(+), 19 deletions(-)
 create mode 100644 js/src/xpconnect/tests/mochitest/file_bug505915.html

diff --git a/js/src/xpconnect/src/XPCWrapper.cpp b/js/src/xpconnect/src/XPCWrapper.cpp
index a359916e182..756b980c968 100644
--- a/js/src/xpconnect/src/XPCWrapper.cpp
+++ b/js/src/xpconnect/src/XPCWrapper.cpp
@@ -45,6 +45,8 @@
 #include "nsPIDOMWindow.h"
 #include "jswrapper.h"
 #include "XrayWrapper.h"
+#include "AccessCheck.h"
+#include "WrapperFactory.h"
 
 namespace XPCWrapper {
 
@@ -64,10 +66,8 @@ JSObject *
 Unwrap(JSContext *cx, JSObject *wrapper)
 {
   if (wrapper->isProxy()) {
-    if (wrapper->getProxyHandler() != &JSCrossCompartmentWrapper::singleton) {
-      // XXX Security check!
-    }
-
+    if (xpc::WrapperFactory::IsScriptAccessOnly(cx, wrapper))
+      return nsnull;
     return wrapper->unwrap();
   }
 
diff --git a/js/src/xpconnect/src/XPCWrapper.h b/js/src/xpconnect/src/XPCWrapper.h
index 9caf194045b..06f16f4589b 100644
--- a/js/src/xpconnect/src/XPCWrapper.h
+++ b/js/src/xpconnect/src/XPCWrapper.h
@@ -314,7 +314,7 @@ MaybePreserveWrapper(JSContext *cx, XPCWrappedNative *wn, uintN flags)
 inline JSBool
 IsSecurityWrapper(JSObject *wrapper)
 {
-  return !!wrapper->getClass()->ext.wrappedObject;
+  return wrapper->isWrapper() || !!wrapper->getClass()->ext.wrappedObject;
 }
 
 /**
diff --git a/js/src/xpconnect/src/xpcjsruntime.cpp b/js/src/xpconnect/src/xpcjsruntime.cpp
index ebf95f3c4c4..e584b48fffb 100644
--- a/js/src/xpconnect/src/xpcjsruntime.cpp
+++ b/js/src/xpconnect/src/xpcjsruntime.cpp
@@ -67,7 +67,8 @@ const char* XPCJSRuntime::mStrings[] = {
     "item",                 // IDX_ITEM
     "__proto__",            // IDX_PROTO
     "__iterator__",         // IDX_ITERATOR
-    "__exposedProps__"      // IDX_EXPOSEDPROPS
+    "__exposedProps__",     // IDX_EXPOSEDPROPS
+    "__scriptOnly__"        // IDX_SCRIPTONLY
 };
 
 /***************************************************************************/
diff --git a/js/src/xpconnect/src/xpcprivate.h b/js/src/xpconnect/src/xpcprivate.h
index 70613379b7b..a592d961831 100644
--- a/js/src/xpconnect/src/xpcprivate.h
+++ b/js/src/xpconnect/src/xpcprivate.h
@@ -661,6 +661,7 @@ public:
         IDX_PROTO                   ,
         IDX_ITERATOR                ,
         IDX_EXPOSEDPROPS            ,
+        IDX_SCRIPTONLY              ,
         IDX_TOTAL_COUNT // just a count of the above
     };
 
diff --git a/js/src/xpconnect/tests/mochitest/Makefile.in b/js/src/xpconnect/tests/mochitest/Makefile.in
index 589ffa69425..67223f14151 100644
--- a/js/src/xpconnect/tests/mochitest/Makefile.in
+++ b/js/src/xpconnect/tests/mochitest/Makefile.in
@@ -65,6 +65,7 @@ _TEST_FILES =	bug500931_helper.html \
 		test_bug503926.html \
 		test_bug504877.html \
 		test_bug505915.html \
+		file_bug505915.html \
 		test_bug517163.html \
 		test_bug553407.html \
 		test_bug560351.html \
diff --git a/js/src/xpconnect/tests/mochitest/file_bug505915.html b/js/src/xpconnect/tests/mochitest/file_bug505915.html
new file mode 100644
index 00000000000..51291269145
--- /dev/null
+++ b/js/src/xpconnect/tests/mochitest/file_bug505915.html
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+Inner frame for testing bug 505915.
+https://bugzilla.mozilla.org/show_bug.cgi?id=505915
+-->
+<head>
+<body onload="parent.postMessage('', '*');">
+</body>
+</html>
diff --git a/js/src/xpconnect/tests/mochitest/test_bug505915.html b/js/src/xpconnect/tests/mochitest/test_bug505915.html
index 5ad9b947a7b..59226bdad55 100644
--- a/js/src/xpconnect/tests/mochitest/test_bug505915.html
+++ b/js/src/xpconnect/tests/mochitest/test_bug505915.html
@@ -16,9 +16,10 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=505915
   
 </div>
 <pre id="test">
-<script type="application/javascript">
+<script type="application/javascript;version=1.7">
 
 /** Test for Bug 505915 **/
+window.addEventListener("message", function () { gen.next() }, false);
 
 function go() {
     var ifr = $('ifr');
@@ -48,7 +49,50 @@ function go() {
            "threw a security exception instead of an invalid child exception");
     }
 
+    // Location is always wrapped, so test it separately.
+
+    ifr.onload = null;
+    var path = "/tests/js/src/xpconnect/tests/mochitest/file_bug505915.html";
+
+    ifr.contentWindow.location = "http://mochi.test:8888/" + path;
+    yield;
+    try {
+        document.documentElement.appendChild(ifr.contentWindow.location);
+        ok(false, "weird behavior");
+    } catch (e) {
+        ok(!/NS_ERROR_XPC_SECURITY_MANAGER_VETO/.test(e),
+           "didn't throw a security exception");
+    }
+    oldLocation = ifr.contentWindow.location;
+    ifr.contentWindow.location = "http://example.org/" + path;
+    yield;
+    try {
+        document.documentElement.appendChild(oldLocation);
+        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.documentElement.appendChild(ifr.contentWindow.location);
+        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");
+    }
+    oldLocation = ifr.contentWindow.location;
+    ifr.contentWindow.location = "http://mochi.test:8888/" + path;
+    yield;
+    try {
+        document.documentElement.appendChild(oldLocation);
+        ok(false, "weird behavior");
+    } catch (e) {
+        ok(!/NS_ERROR_XPC_SECURITY_MANAGER_VETO/.test(e),
+           "didn't throw a security exception");
+    }
+
     SimpleTest.finish();
+    yield;
 }
 
 SimpleTest.waitForExplicitFinish();
@@ -56,7 +100,7 @@ SimpleTest.waitForExplicitFinish();
 </script>
 </pre>
 
-<iframe id="ifr" onload="go()" src="http://example.org/"></iframe>
+<iframe id="ifr" onload="gen = go(); gen.next();" src="http://example.org/"></iframe>
 
 </body>
 </html>
diff --git a/js/src/xpconnect/wrappers/AccessCheck.cpp b/js/src/xpconnect/wrappers/AccessCheck.cpp
index 44a6e982ec5..3cdc5fe795d 100644
--- a/js/src/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/src/xpconnect/wrappers/AccessCheck.cpp
@@ -64,19 +64,15 @@ AccessCheck::isSameOrigin(JSCompartment *a, JSCompartment *b)
 }
 
 bool
-AccessCheck::isLocationObjectSameOrigin(JSContext *cx, JSObject *obj)
+AccessCheck::isLocationObjectSameOrigin(JSContext *cx, JSObject *wrapper)
 {
-    JSCompartment *compartment = obj->compartment();
-
-    obj = obj->unwrap()->getParent();
+    JSObject *obj = wrapper->unwrap()->getParent();
     if (!obj->getClass()->ext.innerObject) {
         obj = obj->unwrap();
         JS_ASSERT(obj->getClass()->ext.innerObject);
     }
     OBJ_TO_INNER_OBJECT(cx, obj);
-    if (!obj)
-        return false;
-    return isSameOrigin(compartment, obj->compartment());
+    return obj && isSameOrigin(wrapper->compartment(), obj->compartment());
 }
 
 bool
diff --git a/js/src/xpconnect/wrappers/AccessCheck.h b/js/src/xpconnect/wrappers/AccessCheck.h
index 7642d032c24..7bdec2ec46e 100644
--- a/js/src/xpconnect/wrappers/AccessCheck.h
+++ b/js/src/xpconnect/wrappers/AccessCheck.h
@@ -52,7 +52,7 @@ class AccessCheck {
     static bool isCrossOriginAccessPermitted(JSContext *cx, JSObject *obj, jsid id,
                                              JSWrapper::Action act);
     static bool isSystemOnlyAccessPermitted(JSContext *cx);
-    static bool isLocationObjectSameOrigin(JSContext *cx, JSObject *obj);
+    static bool isLocationObjectSameOrigin(JSContext *cx, JSObject *wrapper);
 
     static bool needsSystemOnlyWrapper(JSObject *obj);
 
diff --git a/js/src/xpconnect/wrappers/FilteringWrapper.cpp b/js/src/xpconnect/wrappers/FilteringWrapper.cpp
index 7348ad8562b..e3b6716b39a 100644
--- a/js/src/xpconnect/wrappers/FilteringWrapper.cpp
+++ b/js/src/xpconnect/wrappers/FilteringWrapper.cpp
@@ -41,6 +41,7 @@
 #include "AccessCheck.h"
 #include "CrossOriginWrapper.h"
 #include "XrayWrapper.h"
+#include "WrapperFactory.h"
 
 #include "XPCWrapper.h"
 
@@ -160,8 +161,8 @@ FilteringWrapper<Base, Policy>::enter(JSContext *cx, JSObject *wrapper, jsid id,
 
 template<> SOW SOW::singleton(0);
 template<> COW COW::singleton(0);
-template<> XOW XOW::singleton(0);
-template<> NNXOW NNXOW::singleton(0);
+template<> XOW XOW::singleton(WrapperFactory::SCRIPT_ACCESS_ONLY_FLAG);
+template<> NNXOW NNXOW::singleton(WrapperFactory::SCRIPT_ACCESS_ONLY_FLAG);
 template<> LW  LW::singleton(0);
 template<> XLW XLW::singleton(0);
 
diff --git a/js/src/xpconnect/wrappers/WrapperFactory.cpp b/js/src/xpconnect/wrappers/WrapperFactory.cpp
index 71210c35886..4b596206c8c 100644
--- a/js/src/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/src/xpconnect/wrappers/WrapperFactory.cpp
@@ -241,4 +241,32 @@ WrapperFactory::WrapLocationObject(JSContext *cx, JSObject *obj)
     return wrapperObj;
 }
 
+bool
+WrapperFactory::IsScriptAccessOnly(JSContext *cx, JSObject *wrapper)
+{
+    JS_ASSERT(wrapper->isWrapper());
+
+    uintN flags;
+    JSObject *obj = wrapper->unwrap(&flags);
+
+    // If the wrapper indicates script-only access, we are done.
+    if (flags & SCRIPT_ACCESS_ONLY_FLAG)
+        return true;
+
+    // In addition, chrome objects can explicitly opt-in by setting .scriptOnly to true.
+    if (wrapper->getProxyHandler() == &FilteringWrapper<JSCrossCompartmentWrapper,
+        CrossOriginAccessiblePropertiesOnly>::singleton) {
+        jsid scriptOnlyId = GetRTIdByIndex(cx, XPCJSRuntime::IDX_SCRIPTONLY);
+        jsval scriptOnly;
+        if (JS_LookupPropertyById(cx, obj, scriptOnlyId, &scriptOnly) &&
+            scriptOnly == JSVAL_TRUE)
+            return true; // script-only
+    }
+
+    // Allow non-script access to same-origin location objects and any other
+    // objects.
+    return IsLocationObject(obj) &&
+           !xpc::AccessCheck::isLocationObjectSameOrigin(cx, wrapper);
+}
+
 }
diff --git a/js/src/xpconnect/wrappers/WrapperFactory.h b/js/src/xpconnect/wrappers/WrapperFactory.h
index 93a8b59512b..b8b518bd4e4 100644
--- a/js/src/xpconnect/wrappers/WrapperFactory.h
+++ b/js/src/xpconnect/wrappers/WrapperFactory.h
@@ -45,7 +45,8 @@ namespace xpc {
 class WrapperFactory {
   public:
     enum { WAIVE_XRAY_WRAPPER_FLAG = (1<<0),
-           IS_XRAY_WRAPPER_FLAG = (1<<1) };
+           IS_XRAY_WRAPPER_FLAG = (1<<1),
+           SCRIPT_ACCESS_ONLY_FLAG = (1<<2) };
 
     // Return true if any of any of the nested wrappers have the flag set.
     static bool HasWrapperFlag(JSObject *wrapper, uintN flag) {
@@ -58,6 +59,8 @@ class WrapperFactory {
         return HasWrapperFlag(wrapper, IS_XRAY_WRAPPER_FLAG);
     }
 
+    static bool IsScriptAccessOnly(JSContext *cx, JSObject *wrapper);
+
     // Prepare a given object for wrapping in a new compartment.
     static JSObject *PrepareForWrapping(JSContext *cx,
                                         JSObject *scope,