New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 707486 link

Starred by 8 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature



Sign in to add a comment

Consider disallowing named access on Window in isolated worlds

Project Member Reported by dcheng@chromium.org, Apr 1 2017

Issue description

In a recently disclosed Project Zero bug (https://bugs.chromium.org/p/project-zero/issues/detail?id=1225&desc=6), there were logic bugs in content scripts that didn't realize that DOM element names could leak into the global namespace.

We should consider changing this. I'm not sure if we can disallow everything (do content scripts need to do window.frames["name"]), or if we can only disallow a subset of things. Either way, it seems worth considering.
 

Comment 1 by a...@google.com, Apr 2 2017

Cc: taviso@google.com a...@google.com
Cc: rdevlin....@chromium.org
Named getters in frames don't seem particularly harmful; the getter on Window (V8Window::namedPropertyGetterCustom) is particularly interesting because Window is the global object. It's kinda like PHP's register_globals.

I'm uneasy about making the content script contexts API surface not a superset of the main world's, but if this is an issue that occurs in other popular extensions, it might be worthwhile.

Do we have UseCounter or similar data for the APIs that extensions use? Do we know whether content scripts do rely on this feature?
Like jbroman@, I'd prefer not to restrict content scripts in this way.  The way the extensions were relying on this in the exploit was more than a little incorrect.  I don't think it's realistic to try and prevent any footguns from leaking into extension JS, and at the end of the day, we can only do so much.  I don't think the complexity and weirdness/unpredictability to developers this would introduce is worth the gains.
jbroman: the problem is window.frames getter is the same as Window's getter. So if we disallow named access on Window, named access on Window.frames will stop working too.

Nonetheless, I'll be adding a UMA for this. I doubt we'll be able to remove it instantaneously, but we can flag it as deprecated and warn in the dev console and see how things trend.

While it's certainly true that it's not possible to prevent all footguns, but this is a pretty easy one to overlook. The point of isolated worlds is to be isolated, and this arguably violates that.
Ah, whoops, I missed that.

DOMWindow* frames() const { return self(); }

>_>

Comment 6 by a...@google.com, Apr 3 2017

This is likely outside of the scope of this issue, but note that the capability to break the DOM element name -> global namespace bindings would also be useful for regular webapps. 

In particular, the existing behavior has led to vulnerabilities in several client-side HTML sanitizers (DOM Purify: https://www.slideshare.net/x00mario/in-the-dom-no-one-will-hear-you-scream; Caja), and is a problem in situations where an attacker has markup injection but without the capability to execute scripts (e.g. in an application with a locked-down CSP). So it would be nice if we could eventually expose a switch to break this to the web as well.
Cc: jochen@chromium.org yukishiino@chromium.org mkwst@chromium.org haraken@chromium.org
Yes, for the web itself, we would expose this toggle via a feature policy. But we would likely need to solve the problem of window.frames to consider that shippable at all.

Comment 8 by mkwst@chromium.org, Apr 4 2017

Cc: abarth@chromium.org
I'd love to kill this behavior for the web in general. It is surprising for modern developers, and not terribly useful for ancient developers.

I added metrics in https://www.chromestatus.com/metrics/feature/timeline/popularity/1824, but they're not terribly useful because a) the numbers are very high, and b) they don't tell us whether the change would actually break things.

For Isolated Worlds, I agree with dcheng@; it's totally reasonable for the isolated world to have access to the DOM via normal getters, but it's pretty unreasonable for the page to have control over the isolated world's environment. I would be surprised to learn that this was an intentional design choice at the time, rather than just an oversight. We could ask abarth@, I suppose. :)
So I think the main issue with doing this is that there's no alternative for accessing child browsing contexts by name. In order to even consider doing this, we'd need to have a viable alternative.

Crazy idea.... can we hide the named accessor on the global proxy itself and only expose it when the global proxy itself is returned by some other attribute (e.g. window/self/frames/parent/top)? Maybe this could be implemented by wrapping a proxy around it when it's returned by an attribute getter, and the proxy object would forward to the normal named accessor?

e.g.

var iframe = document.createElement('iframe');
iframe.name = 'abc';
document.body.appendChild(iframe);
abc;  // should be undefined
window["abc"];  // should be iframe.contentWindow
self["abc"];  // should be iframe.contentWindow
> So I think the main issue with doing this is that there's no alternative for accessing child browsing contexts by name.

Isn't there? I mean, the following works, right?

```
function getFrameByName(name) {
  for (f of document.querySelectorAll('iframe')) {
    if (f.name == name || f.id == name) 
      return f;  
  }
  return undefined;
}
```
It works as long as the child browsing context doesn't change its browsing context name by assigning a different value to window.name.
Re #9: I'm not sure, but I think that, in your example,

(line 1-3 are snipped)
4: abc;  // should be undefined
5: window["abc"];  // should be iframe.contentWindow
6: self["abc"];  // should be iframe.contentWindow

the global object used to look for "abc" on line 4 is the global object (i.e. inner global) because it must be the same object even after navigation, right (again, I'm not sure if this is true)?

The object used on line 5 and 6 is the global proxy (i.e. outer global).

If so, we may be technically able to install the named accessor on the global proxy.  Just an idea.
This is being discussed for Firefox in <https://bugzilla.mozilla.org/show_bug.cgi?id=1353150>

Comment 14 by bzbar...@mit.edu, Apr 12 2017

For what it's worth, the behavior we have in Gecko on tip right now is as follows:

1) The named properties object claims to have no properties when the current Realm is a web extension content script.  Yes, this means you can't access child browsing contexts by name from the content script.  If we run into reports of extension compatibility problems we'll consider mitigations for this.  The comment 12 suggestion of doing something via the WindowProxy might be viable.  This is the behavior change that was introduced in https://bugzilla.mozilla.org/show_bug.cgi?id=1353150

2) Named getters on things that are not the named properties object work in general, but never have the [OverrideBuiltins] behavior if the current Realm is more privileged than the object being accessed.  This behavior has been around for a long time.
Project Member

Comment 15 by sheriffbot@chromium.org, Apr 12 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Recharge-Cold
Status: Available (was: Untriaged)
We should still figure out if we can fix this. Maybe we could try adding use counters for how often named access is used from content scripts.

Sign in to add a comment