Bug 905493. Fix setting document.location via an Xray to not enter the content compartment. r=peterv

dom/bindings/BindingUtils.cpp

@@ -872,6 +872,15 @@ GetNativePropertyHooks(JSContext *cx, JS::Handle<JSObject*> obj,
   return ifaceAndProtoJSClass->mNativeHooks;
 }
 
+// Try to resolve a property as an unforgeable property from the given
+// NativeProperties, if it's there.  nativeProperties is allowed to be null (in
+// which case we of course won't resolve anything).
+static bool
+XrayResolveUnforgeableProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
+                               JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
+                               JS::MutableHandle<JSPropertyDescriptor> desc,
+                               const NativeProperties* nativeProperties);
+
 bool
 XrayResolveOwnProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                        JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
@@ -881,7 +890,29 @@ XrayResolveOwnProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
   const NativePropertyHooks *nativePropertyHooks =
     GetNativePropertyHooks(cx, obj, type);
 
-  return type != eInstance || !nativePropertyHooks->mResolveOwnProperty ||
+  if (type != eInstance) {
+    return true;
+  }
+
+  // Check for unforgeable properties before doing mResolveOwnProperty weirdness
+  const NativePropertiesHolder& nativeProperties =
+    nativePropertyHooks->mNativeProperties;
+  if (!XrayResolveUnforgeableProperty(cx, wrapper, obj, id, desc,
+                                      nativeProperties.regular)) {
+    return false;
+  }
+  if (desc.object()) {
+    return true;
+  }
+  if (!XrayResolveUnforgeableProperty(cx, wrapper, obj, id, desc,
+                                      nativeProperties.chromeOnly)) {
+    return false;
+  }
+  if (desc.object()) {
+    return true;
+  }
+
+  return !nativePropertyHooks->mResolveOwnProperty ||
          nativePropertyHooks->mResolveOwnProperty(cx, wrapper, obj, id, desc, flags);
 }
 
@@ -936,6 +967,20 @@ XrayResolveAttribute(JSContext* cx, JS::Handle<JSObject*> wrapper,
   return true;
 }
 
+/* static */ bool
+XrayResolveUnforgeableProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
+                               JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
+                               JS::MutableHandle<JSPropertyDescriptor> desc,
+                               const NativeProperties* nativeProperties)
+{
+  return !nativeProperties || !nativeProperties->unforgeableAttributes ||
+         XrayResolveAttribute(cx, wrapper, obj, id,
+                              nativeProperties->unforgeableAttributes,
+                              nativeProperties->unforgeableAttributeIds,
+                              nativeProperties->unforgeableAttributeSpecs,
+                              desc);
+}
+
 static bool
 XrayResolveProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
                     JS::Handle<JSObject*> obj, JS::Handle<jsid> id,
@@ -1008,18 +1053,6 @@ XrayResolveProperty(JSContext* cx, JS::Handle<JSObject*> wrapper,
         return true;
       }
     }
-    if (nativeProperties->unforgeableAttributes) {
-      if (!XrayResolveAttribute(cx, wrapper, obj, id,
-                                nativeProperties->unforgeableAttributes,
-                                nativeProperties->unforgeableAttributeIds,
-                                nativeProperties->unforgeableAttributeSpecs,
-                                desc)) {
-        return false;
-      }
-      if (desc.object()) {
-        return true;
-      }
-    }
   }
 
   if (nativeProperties->constants) {

dom/bindings/Codegen.py

@@ -7285,14 +7285,16 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(ClassMethod):
   return false;
 }
 MOZ_ASSERT_IF(desc.object(), desc.object() == ${holder});"""
+            # We don't want to look at the unforgeable holder at all
+            # in the xray case; that part got handled already.
             getUnforgeable = CallOnUnforgeableHolder(self.descriptor,
-                                                     getUnforgeable, "isXray")
+                                                     getUnforgeable)
             getUnforgeable += """if (desc.object()) {
   desc.object().set(proxy);
-  return !isXray || JS_WrapPropertyDescriptor(cx, desc);
-}
-
-"""
+  return true;
+}"""
+            getUnforgeable = CGIfWrapper(CGGeneric(getUnforgeable),
+                                         "!isXray").define() + "\n\n"
         else:
             getUnforgeable = ""
 

dom/bindings/test/Makefile.in

@@ -68,11 +68,13 @@ MOCHITEST_FILES := \
   test_exception_messages.html \
   test_bug707564.html \
   test_defineProperty.html \
+  file_document_location_set_via_xray.html \
   $(NULL)
 
 MOCHITEST_CHROME_FILES = \
   test_bug775543.html \
   test_bug707564-chrome.html \
+  test_document_location_set_via_xray.html \
   $(NULL)
 
 ifdef GNU_CC

dom/bindings/test/file_document_location_set_via_xray.html

@@ -0,0 +1,5 @@
+<body>
+<script>
+document.x = 5;
+</script>
+</body>

dom/bindings/test/test_document_location_set_via_xray.html

@@ -0,0 +1,49 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=905493
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 905493</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=905493">Mozilla Bug 905493</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+<iframe id="t" src="http://example.org/tests/dom/bindings/test/file_document_location_set_via_xray.html"></iframe>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 905493 **/
+
+function test()
+{
+  var doc = document.getElementById("t").contentWindow.document;
+  ok(!("x" in doc), "Should have an Xray here");
+  is(doc.x, undefined, "Really should have an Xray here");
+  is(doc.wrappedJSObject.x, 5, "And wrapping the right thing");
+  document.getElementById("t").onload = function() {
+    ok(true, "Load happened");
+    SimpleTest.finish();
+  };
+  try {
+    // Test the forwarding location setter
+    doc.location = "chrome://mochikit/content/tests/SimpleTest/test.css";
+  } catch (e) {
+    // Load failed
+    ok(false, "Load failed");
+    SimpleTest.finish();
+  }
+}
+
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(test);
+
+</script>
+</pre>
+</body>
+</html>