Bug 1143810 - Remove some XPConnect JSClass::setProperty hooks that are not needed anymore. r=bholley.
☠☠ backed out by c3638d994edd ☠ ☠
authorJason Orendorff <jorendorff@mozilla.com>
Fri, 13 Mar 2015 14:43:48 -0500
changeset 265290 92adb459d519c7d0c6fd08e234dafd3f6f660a14
parent 265289 11c951df53c9cde273ffc08e3c559530e627f32d
child 265291 7ebc76a450c31ef5b8c53482f3a9ad830bc96ee8
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1143810
milestone39.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 1143810 - Remove some XPConnect JSClass::setProperty hooks that are not needed anymore. r=bholley.
js/xpconnect/src/XPCWrappedNativeJSOps.cpp
js/xpconnect/tests/unit/test_xpcwn_tamperproof.js
js/xpconnect/tests/unit/xpcshell.ini
--- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
@@ -422,24 +422,16 @@ XPC_WN_OnlyIWrite_AddPropertyStub(JSCont
     // Allow only XPConnect to add/set the property
     if (ccx.GetResolveName() == id)
         return true;
 
     return Throw(NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN, cx);
 }
 
 static bool
-XPC_WN_OnlyIWrite_SetPropertyStub(JSContext *cx, HandleObject obj, HandleId id,
-                                  MutableHandleValue vp, ObjectOpResult &result)
-{
-    result.succeed();
-    return XPC_WN_OnlyIWrite_AddPropertyStub(cx, obj, id, vp);
-}
-
-static bool
 XPC_WN_CannotModifyPropertyStub(JSContext *cx, HandleObject obj, HandleId id,
                                 MutableHandleValue vp)
 {
     return Throw(NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN, cx);
 }
 
 static bool
 XPC_WN_CantDeletePropertyStub(JSContext *cx, HandleObject obj, HandleId id,
@@ -647,17 +639,17 @@ const XPCWrappedNativeJSClass XPC_WN_NoH
     "XPCWrappedNative_NoHelper",    // name;
     WRAPPER_FLAGS |
     JSCLASS_PRIVATE_IS_NSISUPPORTS, // flags
 
     /* Mandatory non-null function pointer members. */
     XPC_WN_OnlyIWrite_AddPropertyStub, // addProperty
     XPC_WN_CantDeletePropertyStub,     // delProperty
     nullptr,                           // getProperty
-    XPC_WN_OnlyIWrite_SetPropertyStub, // setProperty
+    nullptr,                           // setProperty
 
     XPC_WN_Shared_Enumerate,           // enumerate
     XPC_WN_NoHelper_Resolve,           // resolve
     XPC_WN_Shared_Convert,             // convert
     XPC_WN_NoHelper_Finalize,          // finalize
 
     /* Optionally non-null members start here. */
     nullptr,                         // call
@@ -1357,24 +1349,16 @@ XPC_WN_OnlyIWrite_Proto_AddPropertyStub(
     // Allow XPConnect to add the property only
     if (ccx.GetResolveName() == id)
         return true;
 
     return Throw(NS_ERROR_XPC_BAD_OP_ON_WN_PROTO, cx);
 }
 
 static bool
-XPC_WN_OnlyIWrite_Proto_SetPropertyStub(JSContext *cx, HandleObject obj, HandleId id,
-                                        MutableHandleValue vp, ObjectOpResult &result)
-{
-    result.succeed();
-    return XPC_WN_OnlyIWrite_Proto_AddPropertyStub(cx, obj, id, vp);
-}
-
-static bool
 XPC_WN_NoMods_Proto_Resolve(JSContext *cx, HandleObject obj, HandleId id, bool *resolvedp)
 {
     MOZ_ASSERT(js::GetObjectClass(obj) == &XPC_WN_NoMods_WithCall_Proto_JSClass ||
                js::GetObjectClass(obj) == &XPC_WN_NoMods_NoCall_Proto_JSClass,
                "bad proto");
 
     XPCWrappedNativeProto* self =
         (XPCWrappedNativeProto*) xpc_GetJSPrivate(obj);
@@ -1399,17 +1383,17 @@ XPC_WN_NoMods_Proto_Resolve(JSContext *c
 const js::Class XPC_WN_NoMods_WithCall_Proto_JSClass = {
     "XPC_WN_NoMods_WithCall_Proto_JSClass",    // name;
     WRAPPER_FLAGS,                             // flags;
 
     /* Mandatory non-null function pointer members. */
     XPC_WN_OnlyIWrite_Proto_AddPropertyStub,   // addProperty;
     XPC_WN_CantDeletePropertyStub,             // delProperty;
     nullptr,                                   // getProperty;
-    XPC_WN_OnlyIWrite_Proto_SetPropertyStub,   // setProperty;
+    nullptr,                                   // setProperty;
     XPC_WN_Shared_Proto_Enumerate,             // enumerate;
     XPC_WN_NoMods_Proto_Resolve,               // resolve;
     nullptr,                                   // convert;
     XPC_WN_Shared_Proto_Finalize,              // finalize;
 
     /* Optionally non-null members start here. */
     nullptr,                         // call;
     nullptr,                         // construct;
@@ -1424,17 +1408,17 @@ const js::Class XPC_WN_NoMods_WithCall_P
 const js::Class XPC_WN_NoMods_NoCall_Proto_JSClass = {
     "XPC_WN_NoMods_NoCall_Proto_JSClass",      // name;
     WRAPPER_FLAGS,                             // flags;
 
     /* Mandatory non-null function pointer members. */
     XPC_WN_OnlyIWrite_Proto_AddPropertyStub,   // addProperty;
     XPC_WN_CantDeletePropertyStub,             // delProperty;
     nullptr,                                   // getProperty;
-    XPC_WN_OnlyIWrite_Proto_SetPropertyStub,   // setProperty;
+    nullptr,                                   // setProperty;
     XPC_WN_Shared_Proto_Enumerate,             // enumerate;
     XPC_WN_NoMods_Proto_Resolve,               // resolve;
     nullptr,                                   // convert;
     XPC_WN_Shared_Proto_Finalize,              // finalize;
 
     /* Optionally non-null members start here. */
     nullptr,                         // call;
     nullptr,                         // construct;
@@ -1519,17 +1503,17 @@ static_assert(((WRAPPER_FLAGS >> JSCLASS
 
 const js::Class XPC_WN_Tearoff_JSClass = {
     "WrappedNative_TearOff",                   // name;
     WRAPPER_FLAGS |
     JSCLASS_HAS_RESERVED_SLOTS(XPC_WN_TEAROFF_RESERVED_SLOTS), // flags;
     XPC_WN_OnlyIWrite_AddPropertyStub,         // addProperty;
     XPC_WN_CantDeletePropertyStub,             // delProperty;
     nullptr,                                   // getProperty;
-    XPC_WN_OnlyIWrite_SetPropertyStub,         // setProperty;
+    nullptr,                                   // setProperty;
     XPC_WN_TearOff_Enumerate,                  // enumerate;
     XPC_WN_TearOff_Resolve,                    // resolve;
     XPC_WN_Shared_Convert,                     // convert;
     XPC_WN_TearOff_Finalize,                   // finalize;
 
     /* Optionally non-null members start here. */
     nullptr,                                   // call
     nullptr,                                   // construct
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/unit/test_xpcwn_tamperproof.js
@@ -0,0 +1,175 @@
+// Test that it's not possible to create expando properties on XPCWNs.
+// See <https://bugzilla.mozilla.org/show_bug.cgi?id=1143810#c5>.
+
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+
+function check_throws(f) {
+  try {
+    f();
+  } catch (exc) {
+    return;
+  }
+  throw new TypeError("Expected exception, no exception thrown");
+}
+
+/*
+ * Test that XPCWrappedNative objects do not permit expando properties to be created.
+ *
+ * This function is called twice. The first time, realObj is an nsITimer XPCWN
+ * and accessObj === realObj.
+ *
+ * The second time, accessObj is a scripted proxy with realObj as its target.
+ * So the second time we are testing that scripted proxies don't magically
+ * bypass whatever mechanism enforces the expando policy on XPCWNs.
+ */
+function test_tamperproof(realObj, accessObj, {method, constant, attribute}) {
+  // Assignment can't create an expando property.
+  check_throws(function () { accessObj.expando = 14; });
+  do_check_false("expando" in realObj);
+
+  // Strict assignment throws.
+  check_throws(function () { "use strict"; accessObj.expando = 14; });
+  do_check_false("expando" in realObj);
+
+  // Assignment to an inherited method name doesn't work either.
+  check_throws(function () { accessObj.hasOwnProperty = () => "lies"; });
+  check_throws(function () { "use strict"; accessObj.hasOwnProperty = () => "lies"; });
+  do_check_false(realObj.hasOwnProperty("hasOwnProperty"));
+
+  // Assignment to a method name doesn't work either.
+  let originalMethod;
+  if (method) {
+    originalMethod = accessObj[method];
+    accessObj[method] = "nope";  // non-writable data property, no exception in non-strict code
+    check_throws(function () { "use strict"; accessObj[method] = "nope"; });
+    do_check_true(realObj[method] === originalMethod);
+  }
+
+  // A constant is the same thing.
+  let originalConstantValue;
+  if (constant) {
+    originalConstantValue = accessObj[constant];
+    accessObj[constant] = "nope";
+    do_check_eq(realObj[constant], originalConstantValue);
+    check_throws(function () { "use strict"; accessObj[constant] = "nope"; });
+    do_check_eq(realObj[constant], originalConstantValue);
+  }
+
+  // Assignment to a readonly accessor property with no setter doesn't work either.
+  let originalAttributeDesc;
+  if (attribute) {
+    originalAttributeDesc = Object.getOwnPropertyDescriptor(realObj, attribute);
+    do_check_true("set" in originalAttributeDesc);
+    do_check_true(originalAttributeDesc.set === undefined);
+
+    accessObj[attribute] = "nope";  // accessor property with no setter: no exception in non-strict code
+    check_throws(function () { "use strict"; accessObj[attribute] = "nope"; });
+
+    let desc = Object.getOwnPropertyDescriptor(realObj, attribute);
+    do_check_true("set" in desc);
+    do_check_eq(originalAttributeDesc.get, desc.get);
+    do_check_eq(undefined, desc.set);
+  }
+
+  // Reflect.set doesn't work either.
+  if (this.Reflect && this.Reflect.set)
+    throw new Error("Congratulations on implementing Reflect.set! Here are some tests to uncomment.");
+  /*
+    if (method) {
+      do_check_false(Reflect.set({}, method, "bad", accessObj));
+      do_check_eq(realObj[method], originalMethod);
+    }
+    if (attribute) {
+      do_check_false(Reflect.set({}, attribute, "bad", accessObj));
+      do_check_eq(originalAttributeDesc.get, Object.getOwnPropertyDescriptor(realObj, attribute).get);
+    }
+  */
+
+  // Object.defineProperty can't do anything either.
+  let names = ["expando"];
+  if (method) names.push(method);
+  if (constant) names.push(constant);
+  if (attribute) names.push(attribute);
+  for (let name of names) {
+    let originalDesc = Object.getOwnPropertyDescriptor(realObj, name);
+    check_throws(function () {
+      Object.defineProperty(accessObj, name, {configurable: true});
+    });
+    check_throws(function () {
+      Object.defineProperty(accessObj, name, {writable: true});
+    });
+    check_throws(function () {
+      Object.defineProperty(accessObj, name, {get: function () { return "lies"; }});
+    });
+    check_throws(function () {
+      Object.defineProperty(accessObj, name, {value: "bad"});
+    });
+    let desc = Object.getOwnPropertyDescriptor(realObj, name);
+    if (originalDesc === undefined) {
+      do_check_eq(undefined, desc);
+    } else {
+      do_check_eq(originalDesc.configurable, desc.configurable);
+      do_check_eq(originalDesc.enumerable, desc.enumerable);
+      do_check_eq(originalDesc.writable, desc.writable);
+      do_check_eq(originalDesc.value, desc.value);
+      do_check_eq(originalDesc.get, desc.get);
+      do_check_eq(originalDesc.set, desc.set);
+    }
+  }
+
+  // Existing properties can't be deleted.
+  if (method) {
+    do_check_false(delete accessObj[method]);
+    check_throws(function () { "use strict"; delete accessObj[method]; });
+    do_check_eq(realObj[method], originalMethod);
+  }
+  if (constant) {
+    do_check_false(delete accessObj[constant]);
+    check_throws(function () { "use strict"; delete accessObj[constant]; });
+    do_check_eq(realObj[constant], originalConstantValue);
+  }
+  if (attribute) {
+    do_check_false(delete accessObj[attribute]);
+    check_throws(function () { "use strict"; delete accessObj[attribute]; });
+    desc = Object.getOwnPropertyDescriptor(realObj, attribute);
+    do_check_eq(originalAttributeDesc.get, desc.get);
+  }
+}
+
+function test_twice(obj, options) {
+  test_tamperproof(obj, obj, options);
+
+  let handler = {
+    getPrototypeOf(t) {
+      return new Proxy(Object.getPrototypeOf(t), handler);
+    }
+  };
+  test_tamperproof(obj, new Proxy(obj, handler), options);
+}
+
+function run_test() {
+  let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+  test_twice(timer, {
+    method: "init",
+    constant: "TYPE_ONE_SHOT",
+    attribute: "callback"
+  });
+
+  let principal = Cc["@mozilla.org/nullprincipal;1"].createInstance(Ci.nsIPrincipal);
+  test_twice(principal, {});
+
+  test_twice(Object.getPrototypeOf(principal), {
+    method: "subsumes",
+    constant: "APP_STATUS_INSTALLED",
+    attribute: "origin"
+  });
+
+  // Test a tearoff object.
+  Components.manager.autoRegister(do_get_file('../components/js/xpctest.manifest'));
+  let b = Cc["@mozilla.org/js/xpc/test/js/TestInterfaceAll;1"].createInstance(Ci.nsIXPCTestInterfaceB);
+  let tearoff = b.nsIXPCTestInterfaceA;
+  test_twice(tearoff, {
+    method: "QueryInterface"
+  });
+}
--- a/js/xpconnect/tests/unit/xpcshell.ini
+++ b/js/xpconnect/tests/unit/xpcshell.ini
@@ -104,10 +104,11 @@ head = head_watchdog.js
 head = head_watchdog.js
 [test_watchdog_toggle.js]
 head = head_watchdog.js
 [test_watchdog_default.js]
 head = head_watchdog.js
 [test_watchdog_hibernate.js]
 head = head_watchdog.js
 [test_writeToGlobalPrototype.js]
+[test_xpcwn_tamperproof.js]
 [test_xrayed_iterator.js]
 [test_xray_SavedFrame.js]
\ No newline at end of file