Bug 789713 - Ignore domain when computing whether to share non-PreCreate WNs cross-compartment. r=mrbkap a=akeybl
authorBobby Holley <bobbyholley@gmail.com>
Fri, 14 Sep 2012 12:29:37 +0200
changeset 106847 3e04584e30f7cccbc8df0d758c386c277f256e47
parent 106846 bd535573e3842c436877824ac93ca71642731611
child 106848 75cf936e3bdc2698e81122891a95b8d0fa21064c
push id2084
push userbobbyholley@gmail.com
push dateFri, 14 Sep 2012 10:30:06 +0000
treeherdermozilla-aurora@3e04584e30f7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap, akeybl
bugs789713
milestone17.0a2
Bug 789713 - Ignore domain when computing whether to share non-PreCreate WNs cross-compartment. r=mrbkap a=akeybl
js/xpconnect/tests/mochitest/Makefile.in
js/xpconnect/tests/mochitest/test_bug789713.html
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/AccessCheck.h
js/xpconnect/wrappers/WrapperFactory.cpp
--- a/js/xpconnect/tests/mochitest/Makefile.in
+++ b/js/xpconnect/tests/mochitest/Makefile.in
@@ -61,16 +61,17 @@ MOCHITEST_FILES =	bug500931_helper.html 
 		test_bug691059.html \
 		file_bug706301.html \
 		test_bug745483.html \
 		file_bug760131.html \
 		test_bug764389.html \
 		test_bug772288.html \
 		test_bug781476.html \
 		file_bug781476.html \
+		test_bug789713.html \
 		file_nodelists.html \
 		file_exnstack.html \
 		file_expandosharing.html \
 		file_empty.html \
 		file_documentdomain.html \
 		test_lookupMethod.html \
 		file_bug738244.html \
 		file_mozMatchesSelector.html \
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/mochitest/test_bug789713.html
@@ -0,0 +1,58 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=789713
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 789713</title>
+  <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=789713">Mozilla Bug 789713</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+<iframe id="ifr"></iframe>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 789713 **/
+
+function go() {
+  var pass = true;
+  var doc = document.getElementById('ifr').contentDocument;
+
+  // Tree walkers use nsDOMGenericSH, which has a spineless PreCreate.
+  var walker = doc.createTreeWalker(doc.body);
+  pass = pass && (walker.root === doc.body);
+
+  // First, do the document.domain operation. This shouldn't crash.
+  document.domain = "example.org";
+
+  // Now, make sure that we still can't access cross-origin properties despite
+  // the fact that the WN is shared under the hood.
+  try {
+    walker.root;
+    pass = false;
+  } catch (e) { pass = pass && /Permission denied/.exec(e.message); }
+  window.parent.postMessage(pass, '*');
+}
+
+// We can't set document.domain on mochi.test, because it's forbidden to set
+// document.domain to a TLD.
+var ifr = document.getElementById('ifr');
+if (window.location.hostname == "mochi.test") {
+  SimpleTest.waitForExplicitFinish();
+  ifr.src = window.location.toString().replace("mochi.test:8888", "test1.example.org").split('?')[0];
+  window.onmessage = function(message) { ok(message.data, "Test succeeded and didn't crash"); SimpleTest.finish(); };
+} else {
+  ifr.src = "file_empty.html";
+  ifr.onload = go;
+}
+
+</script>
+</pre>
+</body>
+</html>
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -47,16 +47,33 @@ AccessCheck::subsumes(JSCompartment *a, 
 
     bool subsumes;
     nsresult rv = aprin->Subsumes(bprin, &subsumes);
     NS_ENSURE_SUCCESS(rv, false);
 
     return subsumes;
 }
 
+// Same as above, but ignoring document.domain.
+bool
+AccessCheck::subsumesIgnoringDomain(JSCompartment *a, JSCompartment *b)
+{
+    nsIPrincipal *aprin = GetCompartmentPrincipal(a);
+    nsIPrincipal *bprin = GetCompartmentPrincipal(b);
+
+    if (!aprin || !bprin)
+        return false;
+
+    bool subsumes;
+    nsresult rv = aprin->SubsumesIgnoringDomain(bprin, &subsumes);
+    NS_ENSURE_SUCCESS(rv, false);
+
+    return subsumes;
+}
+
 bool
 AccessCheck::isLocationObjectSameOrigin(JSContext *cx, JSObject *wrapper)
 {
     // The caller must ensure that the given wrapper wraps a Location object.
     MOZ_ASSERT(WrapperFactory::IsLocationObject(js::UnwrapObject(wrapper)));
 
     // Location objects are parented to the outer window for which they
     // were created. This gives us an easy way to determine whether our
--- a/js/xpconnect/wrappers/AccessCheck.h
+++ b/js/xpconnect/wrappers/AccessCheck.h
@@ -14,16 +14,17 @@
 
 class nsIPrincipal;
 
 namespace xpc {
 
 class AccessCheck {
   public:
     static bool subsumes(JSCompartment *a, JSCompartment *b);
+    static bool subsumesIgnoringDomain(JSCompartment *a, JSCompartment *b);
     static bool isChrome(JSCompartment *compartment);
     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 isLocationObjectSameOrigin(JSContext *cx, JSObject *wrapper);
 
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -222,19 +222,29 @@ WrapperFactory::PrepareForWrapping(JSCon
                     return DoubleWrap(cx, obj, flags);
                 }
 
                 // Ok, must be case (1). Fall through and create a new wrapper.
             }
 
             // Nasty hack for late-breaking bug 781476. This will confuse identity checks,
             // but it's probably better than any of our alternatives.
+            //
+            // Note: We have to ignore domain here. The JS engine assumes that, given a
+            // compartment c, if c->wrap(x) returns a cross-compartment wrapper at time t0,
+            // it will also return a cross-compartment wrapper for any time t1 > t0 unless
+            // an explicit transplant is performed. In particular, wrapper recomputation
+            // assumes that recomputing a wrapper will always result in a wrapper.
+            //
+            // This doesn't actually pose a security issue, because we'll still compute
+            // the correct (opaque) wrapper for the object below given the security
+            // characteristics of the two compartments.
             if (!AccessCheck::isChrome(js::GetObjectCompartment(scope)) &&
-                 AccessCheck::subsumes(js::GetObjectCompartment(scope),
-                                       js::GetObjectCompartment(obj)))
+                 AccessCheck::subsumesIgnoringDomain(js::GetObjectCompartment(scope),
+                                                     js::GetObjectCompartment(obj)))
             {
                 return DoubleWrap(cx, obj, flags);
             }
         }
     }
 
     // NB: Passing a holder here inhibits slim wrappers under
     // WrapNativeToJSVal.