Bug 1726039 - Disallow setting private fields on MaybeCrossOrigin objects (HostEnsureCanPrivateElementAdd) r=peterv,jandem

Differential Revision: https://phabricator.services.mozilla.com/D122780

dom/base/MaybeCrossOriginObject.h

@@ -337,6 +337,9 @@ class MaybeCrossOriginObject : public Base,
   bool enumerate(JSContext* cx, JS::Handle<JSObject*> proxy,
                  JS::MutableHandleVector<jsid> props) const final;
 
+  // Cross origin objects should not participate in private fields.
+  virtual bool throwOnPrivateField() const override { return true; }
+
   /**
    * Spidermonkey-internal hook used by Object.prototype.toString.  Subclasses
    * need to implement this, because we don't know what className they want.

dom/bindings/DOMJSProxyHandler.h

@@ -108,12 +108,6 @@ class DOMProxyHandler : public BaseDOMProxyHandler {
            JS::Handle<JS::Value> v, JS::Handle<JS::Value> receiver,
            JS::ObjectOpResult& result) const override;
 
-  // Use the DOMProxyExpando object for private fields, rather than the proxy
-  // expando object.
-  virtual bool useProxyExpandoObjectForPrivateFields() const override {
-    return false;
-  }
-
   /*
    * If assigning to proxy[id] hits a named setter with OverrideBuiltins or
    * an indexed setter, call it and set *done to true on success. Otherwise, set

dom/bindings/RemoteObjectProxy.h

@@ -68,6 +68,10 @@ class RemoteObjectProxyBase : public js::BaseProxyHandler,
   const char* className(JSContext* aCx,
                         JS::Handle<JSObject*> aProxy) const final;
 
+  // Cross origin objects like RemoteWindowProxy should not participate in
+  // private fields.
+  virtual bool throwOnPrivateField() const override { return true; }
+
   bool isCallable(JSObject* aObj) const final { return false; }
   bool isConstructor(JSObject* aObj) const final { return false; }
 

js/public/Context.h

@@ -89,6 +89,23 @@ using FilenameValidationCallback = bool (*)(JSContext* cx,
                                             const char* filename);
 JS_PUBLIC_API void SetFilenameValidationCallback(FilenameValidationCallback cb);
 
+/**
+ * Install an context wide callback that implements the ECMA262 specification
+ * host hook `HostEnsureCanAddPrivateElement`.
+ *
+ * This hook, which should only be overriden for Web Browsers, examines the
+ * provided object to determine if the addition of a private field is allowed,
+ * throwing an exception and returning false if not.
+ *
+ * The default implementation of this hook, which will be used unless overriden,
+ * examines only proxy objects, and throws if the proxy handler returns true
+ * from the handler method `throwOnPrivateField()`.
+ */
+using EnsureCanAddPrivateElementOp = bool (*)(JSContext* cx, HandleValue val);
+
+JS_PUBLIC_API void SetHostEnsureCanAddPrivateElementHook(
+    JSContext* cx, EnsureCanAddPrivateElementOp op);
+
 } /* namespace JS */
 
 #endif  // js_Context_h

js/public/Proxy.h

@@ -315,6 +315,13 @@ class JS_PUBLIC_API BaseProxyHandler {
   // normal get/set/defineField paths.
   virtual bool useProxyExpandoObjectForPrivateFields() const { return true; }
 
+  // For some exotic objects (WindowProxy, Location), we want to be able to
+  // throw rather than allow private fields on these objects.
+  //
+  // As a simplfying assumption, if throwOnPrivateFields returns true,
+  // we should also return true to useProxyExpandoObjectForPrivateFields.
+  virtual bool throwOnPrivateField() const { return false; }
+
   /*
    * [[Call]] and [[Construct]] are standard internal methods but according
    * to the spec, they are not present on every object.

js/public/friend/ErrorNumbers.msg

@@ -382,6 +382,7 @@ MSG_DEF(JSMSG_ILLEGAL_PRIVATE_FIELD,   0, JSEXN_SYNTAXERR, "private fields aren'
 MSG_DEF(JSMSG_ILLEGAL_PRIVATE_NAME,    0, JSEXN_SYNTAXERR, "private names aren't valid in this context")
 MSG_DEF(JSMSG_INVALID_PRIVATE_NAME_PRECEDENCE,    0, JSEXN_SYNTAXERR, "invalid use of private name due to operator precedence")
 MSG_DEF(JSMSG_INVALID_PRIVATE_NAME_IN_UNARY_EXPR,    0, JSEXN_SYNTAXERR, "invalid use of private name in unary expression without object reference")
+MSG_DEF(JSMSG_ILLEGAL_PRIVATE_EXOTIC,  0, JSEXN_TYPEERR, "private fields or private methods aren't allowed on this exotic object")
 MSG_DEF(JSMSG_PRIVATE_FIELD_DOUBLE,    0, JSEXN_TYPEERR, "Initializing an object twice is an error with private fields")
 MSG_DEF(JSMSG_PRIVATE_BRAND_DOUBLE,    0, JSEXN_TYPEERR, "Initializing an object twice is an error with private methods")
 MSG_DEF(JSMSG_CURLY_AFTER_ASSERT,      0, JSEXN_SYNTAXERR, "missing '{' after assert")

js/src/jsapi.cpp

@@ -1865,6 +1865,11 @@ JS_PUBLIC_API void JS::SetFilenameValidationCallback(
   js::gFilenameValidationCallback = cb;
 }
 
+JS_PUBLIC_API void JS::SetHostEnsureCanAddPrivateElementHook(
+    JSContext* cx, JS::EnsureCanAddPrivateElementOp op) {
+  cx->runtime()->canAddPrivateElement = op;
+}
+
 /*** Standard internal methods **********************************************/
 
 JS_PUBLIC_API bool JS_GetPrototype(JSContext* cx, HandleObject obj,

js/src/proxy/Proxy.cpp

@@ -987,3 +987,26 @@ void ProxyObject::renew(const BaseProxyHandler* handler, const Value& priv) {
     setReservedSlot(i, UndefinedValue());
   }
 }
+
+// This implementation of HostEnsureCanAddPrivateElement is designed to work in
+// collaboration with Gecko to support the HTML implementation, which applies
+// only to Proxy type objects, and as a result we can simply provide proxy
+// handlers to correctly match the required semantics.
+bool DefaultHostEnsureCanAddPrivateElementCallback(JSContext* cx,
+                                                   HandleValue val) {
+  if (!val.isObject()) {
+    return true;
+  }
+
+  Rooted<JSObject*> valObj(cx, &val.toObject());
+  if (!IsProxy(valObj)) {
+    return true;
+  }
+
+  if (GetProxyHandler(valObj)->throwOnPrivateField()) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                              JSMSG_ILLEGAL_PRIVATE_EXOTIC);
+    return false;
+  }
+  return true;
+}

js/src/vm/Interpreter-inl.h

@@ -575,6 +575,18 @@ static MOZ_ALWAYS_INLINE bool CheckPrivateFieldOperation(JSContext* cx,
     }
   }
 
+  // Invoke the HostEnsureCanAddPrivateElement ( O ) host hook here
+  // if the code is attempting to attach a new private element (which
+  // corresponds to the ThrowHas Throw Condition).
+  if (condition == ThrowCondition::ThrowHas) {
+    if (JS::EnsureCanAddPrivateElementOp op =
+            cx->runtime()->canAddPrivateElement) {
+      if (!op(cx, val)) {
+        return false;
+      }
+    }
+  }
+
   if (!HasOwnProperty(cx, val, idval, result)) {
     return false;
   }

js/src/vm/Runtime.cpp

@@ -77,6 +77,9 @@ const JSSecurityCallbacks js::NullSecurityCallbacks = {};
 static const JSWrapObjectCallbacks DefaultWrapObjectCallbacks = {
     TransparentObjectWrapper, nullptr};
 
+extern bool DefaultHostEnsureCanAddPrivateElementCallback(JSContext* cx,
+                                                          HandleValue val);
+
 static size_t ReturnZeroSize(const void* p) { return 0; }
 
 JSRuntime::JSRuntime(JSRuntime* parentRuntime)
@@ -100,6 +103,7 @@ JSRuntime::JSRuntime(JSRuntime* parentRuntime)
       DOMcallbacks(nullptr),
       destroyPrincipals(nullptr),
       readPrincipals(nullptr),
+      canAddPrivateElement(&DefaultHostEnsureCanAddPrivateElementCallback),
       warningReporter(nullptr),
       selfHostedLazyScript(),
       geckoProfiler_(thisFromCtor()),

js/src/vm/Runtime.h

@@ -454,6 +454,8 @@ struct JSRuntime {
   js::MainThreadData<JSDestroyPrincipalsOp> destroyPrincipals;
   js::MainThreadData<JSReadPrincipalsOp> readPrincipals;
 
+  js::MainThreadData<JS::EnsureCanAddPrivateElementOp> canAddPrivateElement;
+
   /* Optional warning reporter. */
   js::MainThreadData<JS::WarningReporter> warningReporter;
 

js/xpconnect/tests/mochitest/test_private_field_dom.html

@@ -151,22 +151,21 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=????
     }
 
     if (A instanceof Function) {
+      function assertThrewInstance(f, error) {
+        var threw = true;
+        try {
+          f();
+          threw = false;
+        } catch (e) {
+          // info("Caught " + e.name);
+          is(e instanceof error, true, "Correct Error thrown");
+        }
+        is(threw, true, "Error was thrown");
+      }
+
       function testNode(node) {
         info("Testing node " + node.nodeName);
 
-        function assertThrewInstance(f, error) {
-          var threw = true;
-          try {
-            f();
-            threw = false;
-          } catch (e) {
-            // info("Caught " + e.name);
-            is(e instanceof error, true, "Correct Error thrown");
-          }
-          is(threw, true, "Error was thrown");
-        }
-
-
         assertThrewInstance(() => A.g(node), TypeError);
         assertThrewInstance(() => A.s(node, 'node'), TypeError);
         // info("Stamping Node");
@@ -191,8 +190,14 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=????
       const test_contents = document.getElementById('test_contents');
       testNodeRecursive(test_contents);
 
-      // Test window and prototype chain.
-      var w = window;
+      info("Checking Window");
+      // Window itself isn't allowed to host private fields, because it's
+      // a cross-origin object
+      assertThrewInstance(() => A.g(window), TypeError)
+
+      info("Checking Window Prototype Chain")
+      // However, it's prototype chain can.
+      w = Object.getPrototypeOf(window);
       while (w) {
         testNode(w);
         w = Object.getPrototypeOf(w);

js/xpconnect/tests/unit/test_private_field_xrays.js

@@ -2,7 +2,7 @@
 
 ChromeUtils.importESModule("resource://gre/modules/Preferences.sys.mjs");
 
-add_task(async function() {
+add_task(async function () {
   let webnav = Services.appShell.createWindowlessBrowser(false);
 
   let docShell = webnav.docShell;
@@ -10,7 +10,16 @@ add_task(async function() {
   docShell.createAboutBlankContentViewer(null, null);
 
   let window = webnav.document.defaultView;
-  let unwrapped = Cu.waiveXrays(window);
+
+  let iframe = window.eval(`
+    iframe = document.createElement("iframe");
+    iframe.id = "iframe"
+    document.body.appendChild(iframe)
+    iframe`);
+
+
+  let unwrapped = Cu.waiveXrays(iframe);
+
 
   class Base {
     constructor(o) {
@@ -18,32 +27,21 @@ add_task(async function() {
     }
   };
 
-  var A;
-  try {
-    A = eval(`
-    (function() {
-      class A extends Base {
-        #x = 12;
-        static gx(o) {
-          return o.#x;
-        }
 
-        static sx(o, v) {
-          o.#x = v;
-        }
-      };
-      return A})()`);
-  } catch (e) {
-    Assert.equal(e instanceof SyntaxError, true);
-    Assert.equal(
-        /private fields are not currently supported/.test(e.message), true);
-    // Early return if private fields aren't enabled.
-    return;
-  }
+  class A extends Base {
+    #x = 12;
+    static gx(o) {
+      return o.#x;
+    }
 
-  new A(window);
-  Assert.equal(A.gx(window), 12);
-  A.sx(window, 'wrapped');
+    static sx(o, v) {
+      o.#x = v;
+    }
+  };
+
+  new A(iframe);
+  Assert.equal(A.gx(iframe), 12);
+  A.sx(iframe, 'wrapped');
 
   // Shouldn't tunnel past xray.
   Assert.throws(() => A.gx(unwrapped), TypeError);
@@ -51,10 +49,10 @@ add_task(async function() {
 
   new A(unwrapped);
   Assert.equal(A.gx(unwrapped), 12);
-  Assert.equal(A.gx(window), 'wrapped');
+  Assert.equal(A.gx(iframe), 'wrapped');
 
-  A.sx(window, 'modified');
+  A.sx(iframe, 'modified');
   Assert.equal(A.gx(unwrapped), 12);
   A.sx(unwrapped, 16);
-  Assert.equal(A.gx(window), 'modified');
+  Assert.equal(A.gx(iframe), 'modified');
 });

js/xpconnect/wrappers/WrapperFactory.h

@@ -41,6 +41,9 @@ class CrossOriginObjectWrapper : public js::Wrapper {
   bool dynamicCheckedUnwrapAllowed(JS::Handle<JSObject*> obj,
                                    JSContext* cx) const override;
 
+  // Cross origin objects should not participate in private fields.
+  virtual bool throwOnPrivateField() const override { return true; }
+
   static const CrossOriginObjectWrapper singleton;
 };
 

testing/web-platform/meta/js/behaviours/HostEnsureCanAddPrivateElement.window.js.ini

@@ -1,36 +1,12 @@
 [HostEnsureCanAddPrivateElement.window.html]
-  [Same Origin: WindowProxy]
-    expected: FAIL
-
   [Cross Origin (port): WindowProxy]
-    expected: FAIL
+    expected: FAIL # SecurityError
 
   [Cross Origin (remote): WindowProxy]
-    expected: FAIL
+    expected: FAIL # SecurityError
 
   [Same Origin + document.domain WindowProxy]
-    expected: FAIL
-
-  [Same Origin: Location]
-    expected: FAIL
-
-  [Cross Origin (remote): Location]
-    expected: FAIL
-
-  [Cross Origin: Location]
-    expected: FAIL
-
-  [Same Origin + document.domain: Location]
-    expected: FAIL
-
-  [(After document.domain set) Same Origin + document.domain: Location]
-    expected: FAIL
+    expected: FAIL # SecurityError
 
   [(After document.domain set) Same Origin + document.domain WindowProxy does carry private fields after navigation]
-    expected: FAIL
-
-  [(After document.domain set) Local navigation (setdomain) Location]
-    expected: FAIL
-
-  [(After document.domain set) Local navigation (setdomain) WindowProxy does carry private fields after navigation]
-    expected: FAIL
+    expected: FAIL # SecurityError