1652868 - Make sure Private Fields don't violate the security assumptions around Window, WindowProxy and Location Objects

Status: RESOLVED DUPLICATE of bug 1726039

(Core :: JavaScript Engine, task, P2)

Description

[Matthew Gaudet (he/him) [:mgaudet]]

Window, WindowProxy, and Location are special objects that have security properties in the Web Platform.

Since our implementation of private field semantics differs from the specification language, I want to make sure that we are able to ensure that we do not get into a situation where the special implementation of private fields ends up violating some web platform security model.

In more concrete terms: Private Fields as specified use the semantics of a Weak Map: Which is to say, for a given JS class:

class A { 
  #x = 10; 
};
var obj = new A; 

we should not think of #x as a property of obj, but rather that there is a hidden WeakMap in A that corresponds to the field #x, and each object with a field is used as a key, and the value of the field is the value in the map.

Because JavaScript, we are able to stamp fields into arbitrary objects, including web platform provided objects, using the following pattern:

class Base { 
  constructor(o) { 
    return o;
  }
};

class B extends Base { 
  #x = 12
  static gx(obj) { return obj.#x; } 
};

new B(window);

Conceptually this is simply creating an entry in the B.#x WeakMap of window -> 12.

Now, in our implementation we did not use the WeakMap approach, as it would be slow. Instead, private fields are indeed implemented as properties, however, those properties are hidden, and follow the required observability semantics of the class fields proposal.

Proxies provide a new challenge. Basic JavaScript proxies, as implemented in JS have no slots, nor need for them. However, Private fields produces a new need for them. My patches (under review) for Proxies adds a new Expando object to hold private fields to Proxies. DOM Proxies use their pre-existing expando object.

I'm trying to figure out how to verify that we don't have information leaks or other security invariant violations with private fields.

For example, after reading the following, from the previously linked blog post

As you navigate a browsing context, the associated Window object changes. But the whole time, the WindowProxy object stays the same. Ergo, one WindowProxy object is a proxy for many Window objects.

I find myself wondering if we couldn't end up in some situation where private fields can cause a denial of service by leaking memory across navigations. What I imagine is something like the following (bear with me as I am extremely under-skilled in the web platform, which is why I'm opening this bug and seeking help)

class Base { 
  constructor(o) { 
    return o;
  }
};

class C extends Base { 
  #holder;
  static sx(obj, v) { obj.#holder = v; }
};

new C(window);
C.sx(SomeHugeArrayBuffer);
// Navigate Window -> someHugearraybuffer is leaked. 

Looking for some assistance in trying to figure out what's going on as I try to science dog my way through this concern.

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

ni? peterv as we had sort of discussed this previously, but I am not sure how clear I was, and having a centralized topic to discuss this asynchronously would be useful as we reason this out.

Please feel free to help me redirect this question where it needs to go. I really don't have enough of a clue, and I'm trying to avoid this being a bad issue sometime in the future :)

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

Waldo has kindly filled in some of the gaps for me on Matrix. I think what I have right now is incorrect, and will need changing. I do need to write a few test cases to make sure I'm communicating correctly.

Comment 3

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: test.html

So, I wrote a small test case based on what I can piece together (thanks in particular to this SO answer) (touch a.html and b.html in the same directory to make the test case work)

This test case produces a different result on Safari, Chrome, and Firefox Nightly. The core of the test is asking if a private field stamped onto a window remains after navigation.

• Safari says: Yes, the private field exists after navigation
• Chrome says: No, the private field no longer exists after navigation
• Firefox stops one step previous, and throws Uncaught DOMException: Permission denied to access property #x on cross-origin object when trying to stamp the private field on the window.

From a weak-map model perspective, it seems like at the very least, Firefox is incorrect here, though, failing safe. To me, Chrome's behaviour is incorrect, given the subsequent assertion about the equivalency of windowA and windowB

Comment 4

[Tom S [:evilpie]]

This was also discussed in https://github.com/tc39/ecma262/pull/688.

Comment 5

[Anne (:annevk)]

I managed to page this back in somewhat, but I wish bz was still here.

In the example in comment 0, it seems that the instance of C becomes eligible for GC post-navigation as there are no references to it and thus nobody should be able to look up the private state associated with window (which is a WindowProxy instance, to be clear). I don't know if we do that correctly, but in principle that would avoid leaks. I also do not see a security concern there, unless it becomes possible to reify these private fields some day in a way that you could leak state across origins. Per https://github.com/tc39/proposal-private-declarations/issues/11 it seems that is unlikely.

In the case from comment 3, Safari has the behavior I would expect. Here the instance of A is not eligible for GC post-navigation as the parent environment holds onto it. And that also has a strong reference to the child's WindowProxy instance. I don't see an opportunity for leaks or security issues there either. But I do think it would be prudent to add tests that cover cross-site (i.e., different registrable domain) navigation of the child (starting from same-origin, same-site, and cross-site documents). The WindowProxy instance should still remain the same and private fields should still work the same, but that should catch anything unexpected happening due to Fission or equivalent projects in other browsers.

I suppose Firefox's behavior stems from how we do security membranes for objects and those (now) also guarding private fields, though why it would be triggered for a same-origin case is unclear to me. I don't know for sure what the correct solution there is and what is correct for extensions and privileged script. It seems that from the design of private fields only those minting them can access them, so intuitively I would not expect the membrane to add value here, but I could be mistaken.

(Apologies for not getting to this on Monday. My backlog was a little bigger than anticipated.)

Comment 6

[Mark S. Miller]

Safari's behavior is correct. The others are incorrect. We need test262 tests demonstrating this.

Comment 7

[Anne (:annevk)]

Yeah, to be clear, I agree with that as far as web-observable behavior goes. I think it also holds for the other things we do, but I'm less confident.

Comment 8

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: WIP: Bug 1652868 - WPT Test for private fields and their carried status for WindowProxies and Location objects.

Taking the previously posted on the bug test.html, and converting it to a WPT
test. Curiously however, the results are different in Firefox when run as a WPT
test vs. just the bare HTML test as I had written previously. In WPT, the test
case fails checking for the private-field on the window proxy post-navigation,
which was Chrome's behaviour when I was testing by hand.

Previously Firefox would throw, complaining about cross-origin access, but it
wasn't entirely clear to me why. I don't claim to understand the rules around
origins and file:// urls tho, but I suspect it could be related -- I imagine
WPT is serving the (blank in this patch -- surely there's a better way) a.html
and b.html from the same origin.

Still, this test continues to show the incorrect behaviour; but the test case
also need to be expanded to cover same-site, and cross-site documents.

Comment 9

[Anne (:annevk)]

Ah, if you were using file: URLs before I think that explains the difference (i.e., what was unclear to me before) as each URL is essentially its own origin there. So once you cover same-site and cross-site you should run into the same kind of failures. (https://github.com/web-platform-tests/wpt/blob/master/common/get-host-info.sub.js is very useful for creating such tests, FWIW.)

Comment 10

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: WIP: Bug 1652868 - Make WindowProxy use ProxyExpando for storing private fields.

To recap briefly for those who weren't involved: Private Fields are actually
modelled as hidden properties on objects; this works out fairly well right up
until one encounters proxies. At that point, one is forced to acknowledge that
the specification of Private Fields is thought in terms of a 'hidden WeakMap',
associated with a class, where the key is the object that 'has' a given private
field.

This means that if you stamp a private field on a Proxy, it does -not- tunnel
through to the target object. The problem is, Firefox is riddled with transparent
proxies, for example CCWs, where you want the private field to land on the target,
not the proxy.

To handle this, the Proxy implementation was made configurable. Classes which need
transparent behaviour return false from useProxyExpandoObjectForPrivateFields, and
classes which desire private fields to end up on the proxy itself, return true.

My hope had been that this would be sufficient to handle, at least the simplest
version (single origin) of the private field navigation, as I had identified via
debugging that the behaviour I'd seen on my original test case was private field
definition tunneling through the WindowProxy (as viewed as nsOuterWindowProxy).

Unfortunately, this seems insufficient, and I'm not entirely sure how to proceed
due to a lack of insight into the actuall design. What I can tell right now is that
at some point TransplantObjectNukingXrayWaivers is invoked, and at this point, under
the covers an object and the WindowProxy carrying the private field are swapped.

This means however that a subsequent check of the window proxy post-navigation has
lost the private field that we intended to be preserved.

Comment 11

[Matthew Gaudet (he/him) [:mgaudet]]

So, I am science dogging my way through this; but I'm in need of some assistance from someone who understands our WindowProxy objects and their life-cycle; I'm going to seek some clarification.

Comment 12

[Phabricator Automation]

Comment on attachment 9227755 [details]
WIP: Bug 1652868 - WPT Test for private fields and their carried status for WindowProxies and Location objects.

Revision D118216 was moved to bug 1717710. Setting attachment 9227755 [details] to obsolete.

Comment 13

[Phabricator Automation]

Comment on attachment 9228215 [details]
WIP: Bug 1652868 - Make WindowProxy use ProxyExpando for storing private fields.

Revision D118414 was moved to bug 1717710. Setting attachment 9228215 [details] to obsolete.

Comment 14

[Matthew Gaudet (he/him) [:mgaudet]]

This will be fixed by disallowing the concerning cases in this bug via Bug 1726039, so resolving this as a dupe.