Bug 930091 - Check objects at COW membranes. r=gabor,r=msucan
authorBobby Holley <bobbyholley@gmail.com>
Sun, 20 Jul 2014 15:36:32 -0600
changeset 216011 2eddd81bb167dae26bacb94777eded1b73ab1a88
parent 216010 030adb40ad1921a88b6ea4e46d97206084dc6147
child 216012 8a186f354cd61de0ad6c13a5a7b01cb3e6f26ac6
push id3857
push userraliiev@mozilla.com
push dateTue, 02 Sep 2014 16:39:23 +0000
treeherdermozilla-beta@5638b907b505 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor, msucan
bugs930091
milestone33.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 930091 - Check objects at COW membranes. r=gabor,r=msucan
browser/devtools/webconsole/test/browser_webconsole_jsterm.js
dom/tests/mochitest/chrome/test_resize_move_windows.xul
js/xpconnect/tests/chrome/test_bug860494.xul
js/xpconnect/tests/unit/test_bug930091.js
js/xpconnect/tests/unit/xpcshell.ini
js/xpconnect/wrappers/ChromeObjectWrapper.cpp
js/xpconnect/wrappers/ChromeObjectWrapper.h
--- a/browser/devtools/webconsole/test/browser_webconsole_jsterm.js
+++ b/browser/devtools/webconsole/test/browser_webconsole_jsterm.js
@@ -154,19 +154,19 @@ function testJSTerm(hud)
   // bug 614561
   jsterm.clearOutput();
   jsterm.execute("pprint('hi')");
   checkResult("\"  0: \"h\"\n  1: \"i\"\"", "pprint('hi')");
   yield undefined;
 
   // check that pprint(function) shows function source, bug 618344
   jsterm.clearOutput();
-  jsterm.execute("pprint(print)");
+  jsterm.execute("pprint(function() { var someCanaryValue = 42; })");
   checkResult(function(node) {
-    return node.textContent.indexOf("aOwner.helperResult") > -1;
+    return node.textContent.indexOf("someCanaryValue") > -1;
   }, "pprint(function) shows source");
   yield undefined;
 
   // check that an evaluated null produces "null", bug 650780
   jsterm.clearOutput();
   jsterm.execute("null");
   checkResult("null", "null is null");
   yield undefined;
--- a/dom/tests/mochitest/chrome/test_resize_move_windows.xul
+++ b/dom/tests/mochitest/chrome/test_resize_move_windows.xul
@@ -236,16 +236,19 @@ https://bugzilla.mozilla.org/show_bug.cg
         "  is_range(window.innerHeight, 170, 5, 'parameter height should be taken into account');" +
         "  is_range(window.screenX, 25, 5, 'parameter screenX should be taken into account');" +
         "  is_range(window.screenY, 25, 5, 'parameter screenY should be taken into account');" +
         "  checkChangeIsEnabled(window, next);" +
         "} <\/script>", '',
         'width=170,height=170,screenX=25,screenY=25');
 
       SimpleTest.waitForFocus(function() {
+        // This test relies on passing cross-origin windows to a chrome function,
+        // which is generally forbidden if we don't twiddle the knob below.
+        Components.utils.forcePermissiveCOWs();
         w.wrappedJSObject.ok = SimpleTest.ok;
         w.wrappedJSObject.checkChangeIsEnabled = window.checkChangeIsEnabled;
         w.wrappedJSObject.check(function() {
           // The current window can change the size and position of the created one.
           checkChangeIsEnabled(w, function() {
             w.close();
 
             // If we call window.open with an empty string as a third parameter,
--- a/js/xpconnect/tests/chrome/test_bug860494.xul
+++ b/js/xpconnect/tests/chrome/test_bug860494.xul
@@ -36,20 +36,20 @@ https://bugzilla.mozilla.org/show_bug.cg
 
     // Now test XOWs.
     var sb = new Cu.Sandbox('http://www.example.com');
     sb.win = iwin;
     sb.topWin = top;
     sb.parentWin = window;
     sb.is = is;
     sb.ok = ok;
-    Cu.evalInSandbox('is(win.top, topWin, "iframe names shouldnt shadow |top| via cross-origin Xray");', sb);
-    Cu.evalInSandbox('is(win.parent, parentWin, "iframe names shouldnt shadow |parent| via cross-origin Xray");', sb);
+    Cu.evalInSandbox('ok(win.top === topWin, "iframe names shouldnt shadow |top| via cross-origin Xray");', sb);
+    Cu.evalInSandbox('ok(win.parent === parentWin, "iframe names shouldnt shadow |parent| via cross-origin Xray");', sb);
     Cu.evalInSandbox('is(win.length, 7, "iframe names shouldnt shadow |length| via cross-origin Xray");', sb);
-    Cu.evalInSandbox('is(win.window, win, "iframe names shouldnt shadow |window| via cross-origin Xray");', sb);
+    Cu.evalInSandbox('ok(win.window === win, "iframe names shouldnt shadow |window| via cross-origin Xray");', sb);
     Cu.evalInSandbox('var exn = "nothrow"; try { win.navigator; } catch (e) { exn = e; } ok(!!/denied/.exec(exn), "cross-origin Xray blocks subframes that shadow: navigator: " + exn);', sb);
     Cu.evalInSandbox('var exn = "nothrow"; try { win.alert; } catch (e) { exn = e; } ok(!!/denied/.exec(exn), "cross-origin Xray blocks subframes that shadow: alert: " + exn);', sb);
 
     SimpleTest.finish();
   }
 
   ]]>
   </script>
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/unit/test_bug930091.js
@@ -0,0 +1,25 @@
+const Cu = Components.utils;
+
+function checkThrows(fn) {
+  try {
+    fn();
+    ok(false, "Should have thrown");
+  } catch (e) {
+    do_check_true(/denied|insecure/.test(e));
+  }
+}
+
+function run_test() {
+  var xosb = new Cu.Sandbox('http://www.example.org');
+  var sb = new Cu.Sandbox('http://www.example.com');
+  sb.do_check_true = do_check_true;
+  sb.fun = function() { ok(false, "Shouldn't ever reach me"); };
+  sb.cow = { foopy: 2, __exposedProps__: { foopy: 'rw' } };
+  sb.payload = Cu.evalInSandbox('new Object()', xosb);
+  Cu.evalInSandbox(checkThrows.toSource(), sb);
+  Cu.evalInSandbox('checkThrows(fun.bind(null, payload));', sb);
+  Cu.evalInSandbox('checkThrows(fun.bind(payload));', sb);
+  Cu.evalInSandbox('checkThrows(function() { new fun(payload); });', sb);
+  Cu.evalInSandbox('checkThrows(function() { cow.foopy = payload; });', sb);
+  Cu.evalInSandbox('checkThrows(function() { Object.defineProperty(cow, "foopy", { value: payload }); });', sb);
+}
--- a/js/xpconnect/tests/unit/xpcshell.ini
+++ b/js/xpconnect/tests/unit/xpcshell.ini
@@ -35,16 +35,17 @@ support-files =
 [test_bug849730.js]
 [test_bug851895.js]
 [test_bug854558.js]
 [test_bug856067.js]
 [test_bug868675.js]
 [test_bug867486.js]
 [test_bug872772.js]
 [test_bug885800.js]
+[test_bug930091.js]
 [test_bug961054.js]
 [test_bug976151.js]
 [test_bug1001094.js]
 [test_bug1021312.js]
 [test_bug1033253.js]
 [test_bug1033920.js]
 [test_bug1033927.js]
 [test_bug1034262.js]
--- a/js/xpconnect/wrappers/ChromeObjectWrapper.cpp
+++ b/js/xpconnect/wrappers/ChromeObjectWrapper.cpp
@@ -1,15 +1,18 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /* vim: set ts=8 sts=4 et sw=4 tw=99: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "ChromeObjectWrapper.h"
+#include "WrapperFactory.h"
+#include "AccessCheck.h"
+#include "xpcprivate.h"
 #include "jsapi.h"
 
 using namespace JS;
 
 namespace xpc {
 
 // When creating wrappers for chrome objects in content, we detect if the
 // prototype of the wrapped chrome object is a prototype for a standard class
@@ -94,16 +97,85 @@ ChromeObjectWrapper::getPropertyDescript
     if (desc.object() || !wrapperProto)
         return true;
 
     // If not, try doing the lookup on the prototype.
     MOZ_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
     return JS_GetPropertyDescriptorById(cx, wrapperProto, id, desc);
 }
 
+static bool
+CheckPassToChrome(JSContext *cx, HandleObject wrapper, HandleValue v)
+{
+    // Primitives are fine.
+    if (!v.isObject())
+        return true;
+    RootedObject obj(cx, &v.toObject());
+
+    // Non-wrappers are fine.
+    if (!js::IsWrapper(obj))
+        return true;
+
+    // COWs are fine to pass back if and only if they have __exposedProps__,
+    // since presumably content should never have a reason to pass an opaque
+    // object back to chrome.
+    if (WrapperFactory::IsCOW(obj)) {
+        RootedObject target(cx, js::UncheckedUnwrap(obj));
+        JSAutoCompartment ac(cx, target);
+        RootedId id(cx, GetRTIdByIndex(cx, XPCJSRuntime::IDX_EXPOSEDPROPS));
+        bool found = false;
+        if (!JS_HasPropertyById(cx, target, id, &found))
+            return false;
+        if (found)
+            return true;
+    }
+
+    // Same-origin wrappers are fine.
+    if (AccessCheck::wrapperSubsumes(obj))
+        return true;
+
+    // Badness.
+    JS_ReportError(cx, "Permission denied to pass object to chrome");
+    return false;
+}
+
+static bool
+CheckPassToChrome(JSContext *cx, HandleObject wrapper, const CallArgs &args)
+{
+    if (!CheckPassToChrome(cx, wrapper, args.thisv()))
+        return false;
+    for (size_t i = 0; i < args.length(); ++i) {
+        if (!CheckPassToChrome(cx, wrapper, args[i]))
+            return false;
+    }
+    return true;
+}
+
+bool
+ChromeObjectWrapper::defineProperty(JSContext *cx, HandleObject wrapper,
+                                    HandleId id,
+                                    MutableHandle<JSPropertyDescriptor> desc) const
+{
+    if (!CheckPassToChrome(cx, wrapper, desc.value()))
+        return false;
+    return ChromeObjectWrapperBase::defineProperty(cx, wrapper, id, desc);
+}
+
+bool
+ChromeObjectWrapper::set(JSContext *cx, HandleObject wrapper,
+                         HandleObject receiver, HandleId id,
+                         bool strict, MutableHandleValue vp) const
+{
+    if (!CheckPassToChrome(cx, wrapper, vp))
+        return false;
+    return ChromeObjectWrapperBase::set(cx, wrapper, receiver, id, strict, vp);
+}
+
+
+
 bool
 ChromeObjectWrapper::has(JSContext *cx, HandleObject wrapper,
                          HandleId id, bool *bp) const
 {
     assertEnteredPolicy(cx, wrapper, id, GET);
     // Try the lookup on the base wrapper if permitted.
     if (AllowedByBase(cx, wrapper, id, js::Wrapper::GET) &&
         !ChromeObjectWrapperBase::has(cx, wrapper, id, bp))
@@ -155,16 +227,34 @@ ChromeObjectWrapper::get(JSContext *cx, 
     if (!wrapperProto)
         return true;
 
     // Try the prototype.
     MOZ_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
     return js::GetGeneric(cx, wrapperProto, receiver, id, vp.address());
 }
 
+bool
+ChromeObjectWrapper::call(JSContext *cx, HandleObject wrapper,
+                      const CallArgs &args) const
+{
+    if (!CheckPassToChrome(cx, wrapper, args))
+        return false;
+    return ChromeObjectWrapperBase::call(cx, wrapper, args);
+}
+
+bool
+ChromeObjectWrapper::construct(JSContext *cx, HandleObject wrapper,
+                               const CallArgs &args) const
+{
+    if (!CheckPassToChrome(cx, wrapper, args))
+        return false;
+    return ChromeObjectWrapperBase::construct(cx, wrapper, args);
+}
+
 // SecurityWrapper categorically returns false for objectClassIs, but the
 // contacts API depends on Array.isArray returning true for COW-implemented
 // contacts. This isn't really ideal, but make it work for now.
 bool
 ChromeObjectWrapper::objectClassIs(HandleObject obj, js::ESClassValue classValue,
                                    JSContext *cx) const
 {
   return CrossCompartmentWrapper::objectClassIs(obj, classValue, cx);
--- a/js/xpconnect/wrappers/ChromeObjectWrapper.h
+++ b/js/xpconnect/wrappers/ChromeObjectWrapper.h
@@ -27,21 +27,33 @@ class ChromeObjectWrapper : public Chrom
 {
   public:
     ChromeObjectWrapper() : ChromeObjectWrapperBase(0) {}
 
     /* Custom traps. */
     virtual bool getPropertyDescriptor(JSContext *cx, JS::Handle<JSObject*> wrapper,
                                        JS::Handle<jsid> id,
                                        JS::MutableHandle<JSPropertyDescriptor> desc) const MOZ_OVERRIDE;
+    virtual bool defineProperty(JSContext *cx, JS::Handle<JSObject*> wrapper,
+                                JS::Handle<jsid> id,
+                                JS::MutableHandle<JSPropertyDescriptor> desc) const MOZ_OVERRIDE;
+    virtual bool set(JSContext *cx, JS::Handle<JSObject*> wrapper,
+                     JS::Handle<JSObject*> receiver, JS::Handle<jsid> id,
+                     bool strict, JS::MutableHandle<JS::Value> vp) const MOZ_OVERRIDE;
+
     virtual bool has(JSContext *cx, JS::Handle<JSObject*> wrapper,
                      JS::Handle<jsid> id, bool *bp) const MOZ_OVERRIDE;
     virtual bool get(JSContext *cx, JS::Handle<JSObject*> wrapper, JS::Handle<JSObject*> receiver,
                      JS::Handle<jsid> id, JS::MutableHandle<JS::Value> vp) const MOZ_OVERRIDE;
 
+    virtual bool call(JSContext *cx, JS::Handle<JSObject*> wrapper,
+                      const JS::CallArgs &args) const MOZ_OVERRIDE;
+    virtual bool construct(JSContext *cx, JS::Handle<JSObject*> wrapper,
+                           const JS::CallArgs &args) const MOZ_OVERRIDE;
+
     virtual bool objectClassIs(JS::Handle<JSObject*> obj, js::ESClassValue classValue,
                                JSContext *cx) const MOZ_OVERRIDE;
 
     virtual bool enter(JSContext *cx, JS::Handle<JSObject*> wrapper, JS::Handle<jsid> id,
                        js::Wrapper::Action act, bool *bp) const MOZ_OVERRIDE;
 
     // NB: One might think we'd need to implement enumerate(), keys(), iterate(),
     // and getPropertyNames() here. However, ES5 built-in properties aren't