Consider disallowing named access on Window in isolated worlds |
||||||
Issue descriptionIn 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.
,
Apr 3 2017
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?
,
Apr 3 2017
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.
,
Apr 3 2017
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.
,
Apr 3 2017
Ah, whoops, I missed that.
DOMWindow* frames() const { return self(); }
>_>
,
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.
,
Apr 4 2017
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.
,
Apr 4 2017
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. :)
,
Apr 5 2017
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
,
Apr 5 2017
> 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;
}
```
,
Apr 5 2017
It works as long as the child browsing context doesn't change its browsing context name by assigning a different value to window.name.
,
Apr 5 2017
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.
,
Apr 6 2017
This is being discussed for Firefox in <https://bugzilla.mozilla.org/show_bug.cgi?id=1353150>
,
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.
,
Apr 12 2018
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
,
Apr 12 2018
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 |
||||||
Comment 1 by a...@google.com
, Apr 2 2017