Make `unsafe-hashed-attributes' less unsafe

[briansmith]

If I understand correctly, based on the example at https://w3c.github.io/webappsec-csp/#unsafe-hashed-attributes-usage, the digest is calculated only over the value of the attribute, and the attribute name and the element name are ignored. This means that whenever unsafe-hashed-attributes is used, an attacker can duplicate the value into any event handler.

My guess is that this could be made safer by hashing the element name and the attribute name in addition to the attribute value.

One example syntax would be, in the case of the example given, <button id="action" onclick="doSubmit()">, instead of hashing just the string "doSubmit()" (without quotes), "button onclick doSubmit()" (without quotes) would be hashed, where a single space is used as a delimiter to separate the fields, since spaces can't appear in element or attribute names.

And/or, the spec should mention that a danger of this mechanism is that it enables an attacker to create an inline event handler for any event on any type of element that executes the whitelisted code. Therefore, one should be careful that the whitelisted code isn't dangerous when evaluated on different elements or for different events.

Regardless, the usage example should mention what exactly the hash is calculated on, as I was only able to figure it out by guessing and checking.

[mikewest]

If I understand correctly...

You've understood correctly. And I agree that the description is a little obtuse. Basically, HTML passes in the attribute value via https://html.spec.whatwg.org/#event-handler-content-attributes, and it's eventually routed through https://w3c.github.io/webappsec-csp/#match-element-to-source-list which does the comparison. So the spec is technically correct, but could use some clarification text somewhere.

My guess is that this could be made safer by hashing the element name and the attribute name in addition to the attribute value.

Yup. There's another bug with similar suggestions (something like sha256-hashgoeshere#element@attribute: https://github.com/w3c/webappsec-csp/issues/13 (which is easier to read in the original https://github.com/w3c/webappsec/issues/468). I don't have real objections to that, but I think that might be overkill for the use cases I know about (which are mass quantities of old (and sometimes static?) pages on google.com that folks like @arturjanc want to convert to use 'strict-dynamic', but can't because of inline script).

/cc @hillbrad and @devd who had opinions.

[arturjanc]

This is a reasonable proposal and I am not opposed to doing something like this, but I also don't believe it makes the application noticeably more secure against markup injections / XSS.

As currently in the spec, any script whitelisted with a hash will be allowed to execute. This means that if the document wants to allow <div onclick="foo()"></div> it will need to permit the execution of foo() which means that an attacker could inject <script>foo()</script> and execute the script without any user interaction (or do the same via an event handler such as img#onload which also executes without interaction).

The main benefit here is that it would tie the executed script to a user action even in the event of an XSS. But even in that case, an attacker will almost always be able to get that script to run; if the attacker needs a button#onclick=foo() instead of directly calling foo() she can reuse styles already available in the application and create a button spanning the full height/width of the page with visibility:hidden, or by "tiling" the page with a large number of small, invisible buttons -- if a user clicks anywhere within the document, the script will execute.

Like @mikewest said, I'm not really sure if this is enough of a barrier to justify the extra complexity. But I do think there should at least be a warning in the spec against allowing hashes for event handlers to bless patterns like <a onclick="deleteAccount()">Remove my account</a>

[briansmith]

Like @mikewest said, I'm not really sure if this is enough of a barrier to justify the extra complexity. But I do think there should at least be a warning in the spec against allowing hashes for event handlers to bless patterns like Remove my account

This seems reasonable to me.

[mikewest]

Added a note in https://w3c.github.io/webappsec-csp/#unsafe-hashed-attributes-usage. Thanks to you both!

[koto]

The approach of whitelisting event handlers for execution was already tried and was subsequently broken e.g. by this.

To start with trivial examples, virtually every application exposes a logout functionality upon clicking a button. With unsafe-hashed-attributes this can simply be a body of a script element, preventing users to use the actual site. Similarly, many event handlers delete a given entity.

To step away a bit, unsafe-hashed-attributes aims to authenticate given code snippets by announcing a digest over a non-attacker-controlled channel, demonstrating the snippet provenance. The chosen scheme however makes a mistake of not adhering to Horton's principle - it authenticates what's being said (i.e. the code), not what was being meant (i.e. when that code should run). This enables the attacker to abuse the authentication scheme with code reuse - auto-logging-out is a simple example of that. In crypto such approach causes problems years difficult to fix for years ahead. There is no known to me way of applying this scheme safely, especially given that the intention is to whitelist all existing handlers automatically.

[arturjanc]

I think the misunderstanding here comes from assuming that all code running in a given origin, both

1. Belongs to a single application (and gets the same CSP), and
2. Is present in inline event handlers.

In practice, both of these assumptions are not true for many sites. For example, help pages and static content often contain event handlers with analytics and similar self-contained functionality which is separate from the main application; scripts on such pages do not invoke code which affects the application, but executes in the same origin. Because of this, the set of blessed script values often does not include things useful to an attacker (and note that particularly in the per-page CSP use case enabled by 'unsafe-hashed-attributes' the policy would only apply to the single document, rather than for the whole origin).

Also, the fact that logout functionality exists in many sites also doesn't mean that it's present in an inline event handler; developers can progressively refactor the more sensitive application-specific scripts into external JS blocks and remove the inline event handler. The difference here is that 'unsafe-hashed-attributes' would make it possible to adopt policies without 'unsafe-inline' (as is currently done by ~88% of sites with CSP) but with a much more constrained set of allowed inline code. There's no guarantee that this will be completely safe for all applications, but it's better than status quo; and in some cases, like static content, it actually comes quite close to solving the problem in that it doesn't give the attacker the capability to execute scripts which affect other content living in the origin.

I'd encourage you to think about 'unsafe-hashed-attributes' as a tool which solves a very important problem, but -- like all things in CSP -- requires some competence on part of the developer to deploy it safely. There are many ways to deploy it safely, and some to do it _un_safely, but even in the latter case (e.g. when blessing all existing inline event handlers in a complex dynamic app and bundling them into a huge policy served for the whole origin) it still has positive effects like forcing new code to be outlined and restricts the attacker to running code present in the XSS-able document.

@devd @briansmith I haven't forgotten about your questions and will reply soon (bad flight back from the US).

[koto]

I see. So the idea is to specify the hashes for the inline event handlers in an origin-scoped CSP (as opposed to a document-specific one)? That's not as bad then, but I'm still not convinced it's a right tool.

We all agree that it's better than unsafe-inline, the problematic part from my p.o.v. is that it's much harder to get it right ('right' meaning that it successfully stops the attacks by a determined attacker) - and it's difficult for adopters to understand how it may be bypassed, which will probably lead to majority of deployments being vulnerable.

It's always a tradeoff when introducing an unsafe- keyword, and in practice developers will just use whatever is the easiest, which is why right now majority of CSPs use unsafe-inline as you say - and I think it's a safe assumption this is bad. Offering easy to adopt, insecure by default, but hard to get right settings may simply continue to have such bad effect. And this particular keyword may offer reasonable attack countermeasures only when the whitelist is small, and does not include any "transactional" JS. Which may be the intention of the editors, but will likely be abused by adopters like all previous unsafe keywords.

Since the intention is to apply it to static pages, maybe this can be resolved differently? Since the middleware would anyway be required to gather the hashes, and the code is fairly uniform (analytics) it's not unreasonable for the middleware to rewrite the inline handlers into separate script blocks and hash those? This should be fairly easy:

<a onclick="analytics(123)">

to

 <a id=aaa><script>document.all.aaa.onclick=analytics(123)</script>

Most analytics likely already uses data- attributes to carry the parameters anyway, and they are generally migrating away from inline handlers and towards CSP-compatible code.

[arturjanc]

So the idea is to specify the hashes for the inline event handlers in an origin-scoped CSP (as opposed to a document-specific one)?

I think it's the opposite, i.e. for static documents (which are currently often exempt from CSP in the subset of applications using nonces) the CSP could be set per-document, i.e. whitelist only the inline event handlers present in that document. This seems preferable to bundling all values of inline event handlers from the origin into a large site-wide CSP.

Re: majority of deployments being vulnerable, this is already the case, by far ;-) Moving to a world where the attacker can invoke only the minority of code left in inline event handlers -- which currently forces 'unsafe-inline' or prevents the page or site from deploying CSP -- seems strictly better, even in an unsafe deployment where the developer doesn't take care to review the scripts which get blessed by 'unsafe-hashed-attributes'. Especially considering that the alternative is the lack of any restrictions at all...

Note that this is qualitatively similar to CSP2 hashes for inline scripts in that blessing a given inline script to run can be just as dangerous as an inline event handler because the script can exhibit unpredictable behavior when called multiple times, or can rely on data from the DOM which can be controlled by the attacker. We can argue that this isn't great, but it's a tool which enables deployment of CSP and consequently can often stop the attacker from getting unconstrained JS execution in the vulnerable origin, though it might not block application-specific attacks (just like CSP generally doesn't block phishing via malicious login forms in the same origin, exfiltration of data via CSS, or other scriptless attacks).

Re: automated refactorings, I like the idea and would really love for this to be possible, but developers have so far run into several issues which make it very difficult to do in general, as evidenced in part by the large majority of CSP deployments with 'unsafe-inline'. I will write down some of the problems we've seen with this approach and post it separately, soon.