Disable DOM clobbering.

[mikewest]

https://research.securitum.com/xss-in-amp4email-dom-clobbering/ is a good example of the kinds of attacks enabled by the somewhat unexpected mapping of elements into the global namespace via the namedItem() getter on Window:

• https://html.spec.whatwg.org/#dom-window-nameditem
• https://html.spec.whatwg.org/#naming-form-controls:-the-name-attribute

We can't turn this off by default, as ~8% of pages depend on it in one way or another in Chrome's dataset, but it would be lovely if we could disable this footgun via (something like?) FP.

@clelland

[annevk]

See whatwg/html#2212 for prior discussion and the various things that need to be considered.

[koto]

DOM clobbering is also often a cause for concern in HTML sanitizers, a common source of bypasses is when a sanitizer gets confused when traversing the dirty DOM tree. Alternatively, the nodes that are in the end sanitized, may - instead of executing JS, confuse the application via DOM clobbering.

What these examples show is that perhaps - if blocking clobbering altogether turns out infeasible due to backwards compatibility - we could mark individual nodes themselves as not clobbering. Then a sanitizer could simply disable the clobbering for the whole dirty tree, and traverse it in a normal way, and - whatever the output is, the resulting nodes would not be able to clobber the rest of the application as well. This would be backwards compatible, and we could make the native sanitizer work like that from the start.

// @mozfreddyb

[bzbarsky]

I'm not quite sure I follow how the attack works. If the global variable involved is defined via var or let then that declaration will take precedence over the named properties object bits. So in the specific example in https://research.securitum.com/xss-in-amp4email-dom-clobbering/ what exactly is the setup with the AMP_MODE global that allows ids to override it? Unfortunately that post never shows or links to the full code involved...

DOM clobbering is also often a cause for concern in HTML sanitizers

Just to check, in this context does the term "clobbering" refer to named getters in general, [OverrideBuiltins] named getters, or any properties on objects that might shadow the prototype ("expandos")? Those are quite different problems that might require quite different solutions depending on what the actual issues are.

[koto]

I'm not quite sure I follow how the attack works. If the global variable involved is defined via var or let then that declaration will take precedence over the named properties object bits. So in the specific example in https://research.securitum.com/xss-in-amp4email-dom-clobbering/ what exactly is the setup with the AMP_MODE global that allows ids to override it? Unfortunately that post never shows or links to the full code involved...

I'm guessing it's https://github.com/ampproject/amphtml/blob/0c5e03f962ebee782476a0da9f68600ba92c3cc1/src/mode.js#L50

It usually is that you're attempting to read some global variables off window, or properties/functions of DOM elements.

DOM clobbering is also often a cause for concern in HTML sanitizers

Just to check, in this context does the term "clobbering" refer to named getters in general, [OverrideBuiltins] named getters, or any properties on objects that might shadow the prototype ("expandos")? Those are quite different problems that might require quite different solutions depending on what the actual issues are.

In the context of sanitizers, I guess the third option. The attacks have to be tailored to an exact sanitizer logic, but it roughly looks like this:

https://fastmail.blog/2015/12/20/sanitising-html-the-dom-clobbering-issue/

[bzbarsky]

I'm guessing it's https://github.com/ampproject/amphtml/blob/0c5e03f962ebee782476a0da9f68600ba92c3cc1/src/mode.js#L50

OK, but is the point that "AMP_MODE" is sometimes not actually defined on Window?

It usually is that you're attempting to read some global variables off window, or properties/functions of DOM elements.

Those are very different situations, which may have different compat constraints, for what it's worth.

In the context of sanitizers, I guess the third option.

If that is the case (that is, you are trying to sanitize a DOM fragment that untrusted script was allowed to touch), then that is quite far afield from the original issue filed in this bug (which is about the window's named properties object). The only possible solutions there are either separate worlds (in the Chrome sense) or membranes that only expose the default behavior (in the Firefox sense).

[koto]

I'm guessing it's https://github.com/ampproject/amphtml/blob/0c5e03f962ebee782476a0da9f68600ba92c3cc1/src/mode.js#L50

OK, but is the point that "AMP_MODE" is sometimes not actually defined on Window?

I don't know how AMP_MODE was set for AMP. I'm guessing win.AMP_MODE = xyz or var AMP_MODE just wasn't never called in that case.

It usually is that you're attempting to read some global variables off window, or properties/functions of DOM elements.

Those are very different situations, which may have different compat constraints, for what it's worth.

In the context of sanitizers, I guess the third option.

If that is the case (that is, you are trying to sanitize a DOM fragment that untrusted script was allowed to touch), then that is quite far afield from the original issue filed in this bug (which is about the window's named properties object). The only possible solutions there are either separate worlds (in the Chrome sense) or membranes that only expose the default behavior (in the Firefox sense).

To clarify for sanitizer case:

1. Sanitizers gets an untrusted string with HTML snippet. It happens to contain nodes named to clobber DOM API functions (like 'Node.removeChild`)
2. Sanitizers create a dirty DOM tree from that string
3. Sanitizer attempts to call the function on a dirty element, expecting to get a function from the prototype chain. It's clobbered, node removal doesn't succeed.
4. ...
5. Profit

I agree it's a different case than overriding properties on window. Both however are called DOM clobbering, as the source of clobbering comes from DOM and its behavior of treating id/name attributes specially.

[terjanq]

@bzbarsky

I am not sure about that specific example of AMP_MODE, but from my experience you can often see variables like:
window.CONFIG = window.CONFIG || {}

It usually happens when a global variable is shared between multiple scripts.

[bzbarsky]

Sanitizers gets an untrusted string with HTML snippet

OK, then we're definitely not dealing with the third option, thank you. So for sanitizers it sounds like [OverrideBuiltins] is the only issue, and that's only an issue for HTMLFormElement.

It's worth investigating how much people rely on that; the use counter mentioned in #349 (comment) does NOT count that.

[petamoriken]

related: https://github.com/w3c/webappsec-feature-policy/issues/251

[tomayac]

Likewise related: WICG/document-policy#32.

[securityMB]

Status update: recently a CL landed in Chromium that adds new UseCounters that will help us better understand DOM Clobbering patterns in the wild:

• kDOMClobberedShadowedDocumentPropertyAccessed - which tracks named properties on document and is triggered only when the property shadows a pre-existing property (for example: <img name=getElementById>).
• kDOMClobberedNotShadowedDocumentPropertyAccessed - which tracks named properties on document and is triggered only when the property does not shadow a pre-existing property.
• kDOMClobberedShadowedFormPropertyAccessed - tracks shadowed properties on <form>
• kDOMClobberedNotShadowedFormPropertyAccessed - tracks non-shadowed properties on <form>.

We will wait a couple of months to see the results and based on that we'll try to figure it out what exactly we can against various types of DOM Clobbering.

[mozfreddyb]

To clarify, this rules out any measurement for properties of window. Is this intentional?

[securityMB]

The UseCounter for window is already there. Also you can't shadow properties on window so there's no need to distinguish between these two cases as in these other examples.

That said, if you have an idea for another useful counter for window, let me know.

[annevk]

Disabling window[int] might be good too to prevent certain exfiltration attacks. That's also covered by COOP to some extent, but maybe the defense-in-depth is worth it.

[securityMB]

The use counter for kDOMClobberedWindowPropertyAccessed is most likely inflated (although it's difficult to say how much) as it triggers on assignments when it shouldn't, see this bug found by @ericcornelissen: https://crbug.com/377468494.