Inline event handlers not whitelisted by hashes? #13

Closed
mikewest opened this Issue Oct 7, 2015 · 14 comments

Comments

Projects
None yet
5 participants
@mikewest
Member

mikewest commented Oct 7, 2015

From @kravietz on September 8, 2015 15:54

Reading through section 7.15 of the latest CSP2 editor's draft it seems like you can't whitelist an inline event handler with its SHA256 hash. Is this intended exclusion?

Just to provide a bit of background, the following script will only result in one pop-up, from the whitelisted alert in the <script> tag:

<!DOCUMENT html>
<html>
<head>
<meta http-equiv="Content-Security-Policy" content="default-src none; script-src 'sha256-qznLcsROx4GACP2dm0UCKCzCG-HiZ1guq6ZZDob_Tng='; img-src data:">
</head>
<body>
<img src="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg'/>"
onload="alert('Hello, world.');">
<script>alert('Hello, world.');</script>
</body>
</html>

The inline handler alert will not be shown and Chrome will display the following error message:

Refused to execute inline event handler because it violates the following Content Security Policy 
directive: "script-src 'sha256-qznLcsROx4GACP2dm0UCKCzCG-HiZ1guq6ZZDob_Tng='". 
Either the 'unsafe-inline' keyword, a hash ('sha256-...'), or a nonce ('nonce-...') is required to
enable inline execution.

Which on the other hand suggests that a hash could be used to whitelist it (but this may be just a Chrome catch-all message).

Copied from original issue: w3c/webappsec#468

@mikewest mikewest added the CSP label Oct 7, 2015

@mikewest

This comment has been minimized.

Show comment
Hide comment
@mikewest

mikewest Oct 7, 2015

Member

From @hillbrad on September 8, 2015 17:39

I would suggest that the Chrome-specific console text here is a bit
misleading.

Part of the difficulty here is that as a <script> block, one <script> is
roughly the same as another. But as an event handler, context is very
important. It's ok for "deleteAll()" to be attached to the big red button
that only appears after several confirmation steps, but not OK for an
injection to attach it to the "x" in the corner of an app install overlay.
So we would need to think a lot more about how to identify contexts, and
then you end up in a situation of potentially having to calculate running
hashes on much more of the DOM as it is parsed than is currently the case,
with potentially negative performance characteristics.

Maybe nonces are better for this use case, if we do need it?

And I'm not sure we do need it. On the application deployer side, where
the hash/nonce model is about processing/annotating legacy content to make
it CSP-compatible, it seems substantially as easy to "outline" these
handlers as it is to add a hash to them. See, e.g.
https://www.npmjs.com/package/noin

On Tue, Sep 8, 2015 at 8:54 AM Paweł Krawczyk notifications@github.com
wrote:

Reading through section 7.15 of the latest CSP2 editor's draft it seems
like you can't whitelist an inline event handler with its SHA256 hash. Is
this intended exclusion?

Just to provide a bit of background, the following script will only result
in one pop-up, from the whitelisted alert in the <script> tag:

<script>alert('Hello, world.');</script>

The inline handler alert will not be shown and Chrome will display the
following error message:

Refused to execute inline event handler because it violates the following Content Security Policy directive: "script-src 'sha256-qznLcsROx4GACP2dm0UCKCzCG-HiZ1guq6ZZDob_Tng='". Either the 'unsafe-inline' keyword, a hash ('sha256-...'), or a nonce ('nonce-...') is required to enable inline execution.

Which on the other hand suggests that a hash could be used to whitelist
it (but this may be just a Chrome catch-all message).


Reply to this email directly or view it on GitHub
w3c/webappsec#468.

Member

mikewest commented Oct 7, 2015

From @hillbrad on September 8, 2015 17:39

I would suggest that the Chrome-specific console text here is a bit
misleading.

Part of the difficulty here is that as a <script> block, one <script> is
roughly the same as another. But as an event handler, context is very
important. It's ok for "deleteAll()" to be attached to the big red button
that only appears after several confirmation steps, but not OK for an
injection to attach it to the "x" in the corner of an app install overlay.
So we would need to think a lot more about how to identify contexts, and
then you end up in a situation of potentially having to calculate running
hashes on much more of the DOM as it is parsed than is currently the case,
with potentially negative performance characteristics.

Maybe nonces are better for this use case, if we do need it?

And I'm not sure we do need it. On the application deployer side, where
the hash/nonce model is about processing/annotating legacy content to make
it CSP-compatible, it seems substantially as easy to "outline" these
handlers as it is to add a hash to them. See, e.g.
https://www.npmjs.com/package/noin

On Tue, Sep 8, 2015 at 8:54 AM Paweł Krawczyk notifications@github.com
wrote:

Reading through section 7.15 of the latest CSP2 editor's draft it seems
like you can't whitelist an inline event handler with its SHA256 hash. Is
this intended exclusion?

Just to provide a bit of background, the following script will only result
in one pop-up, from the whitelisted alert in the <script> tag:

<script>alert('Hello, world.');</script>

The inline handler alert will not be shown and Chrome will display the
following error message:

Refused to execute inline event handler because it violates the following Content Security Policy directive: "script-src 'sha256-qznLcsROx4GACP2dm0UCKCzCG-HiZ1guq6ZZDob_Tng='". Either the 'unsafe-inline' keyword, a hash ('sha256-...'), or a nonce ('nonce-...') is required to enable inline execution.

Which on the other hand suggests that a hash could be used to whitelist
it (but this may be just a Chrome catch-all message).


Reply to this email directly or view it on GitHub
w3c/webappsec#468.

@mikewest

This comment has been minimized.

Show comment
Hide comment
@mikewest

mikewest Oct 7, 2015

Member
  1. Chrome's error message is bad. I should fix that.
  2. I didn't really intend for nonces or hashes to enable inline event handlers. It's something I suppose is worth reconsidering for CSP3, but the behavior for CSP2 (which is shipping in Chrome and Firefox) excludes them. As Brad notes, that's not at all a straightforward discussion.
Member

mikewest commented Oct 7, 2015

  1. Chrome's error message is bad. I should fix that.
  2. I didn't really intend for nonces or hashes to enable inline event handlers. It's something I suppose is worth reconsidering for CSP3, but the behavior for CSP2 (which is shipping in Chrome and Firefox) excludes them. As Brad notes, that's not at all a straightforward discussion.
@mikewest

This comment has been minimized.

Show comment
Hide comment
@mikewest

mikewest Oct 7, 2015

Member

From @kravietz on September 8, 2015 18:41

Thanks, these are very helpful notes. I have already filed a Chrome bug report about this message, but wanted to confirm here what should be the correct browser's policy.

As for inline event handlers I do see quite a lot of them, especially if you include a Google AdSense code. While perhaps the correct approach should be to rewrite AdSense code not to use inline handlers, right now it is a real problem if you want to enable CSP on real world websites.

Member

mikewest commented Oct 7, 2015

From @kravietz on September 8, 2015 18:41

Thanks, these are very helpful notes. I have already filed a Chrome bug report about this message, but wanted to confirm here what should be the correct browser's policy.

As for inline event handlers I do see quite a lot of them, especially if you include a Google AdSense code. While perhaps the correct approach should be to rewrite AdSense code not to use inline handlers, right now it is a real problem if you want to enable CSP on real world websites.

@mikewest

This comment has been minimized.

Show comment
Hide comment
@mikewest

mikewest Oct 7, 2015

Member

From @hillbrad on September 8, 2015 18:49

Do those event handlers have to be inline? Is there a reason the necessary
elements can't be tagged by id or class and the event handlers added
imperatively by a script block?

On Tue, Sep 8, 2015 at 11:41 AM Paweł Krawczyk notifications@github.com
wrote:

Thanks, these are very helpful notes. I have already filed a Chrome bug
report about this message, but wanted to confirm here what should be the
correct browser's policy.

As for inline event handlers I do see quite a lot of them, especially if
you include a Google AdSense code. While perhaps the correct approach
should be to rewrite AdSense code not to use inline handlers, right now it
is a real problem if you want to enable CSP on real world websites.


Reply to this email directly or view it on GitHub
w3c/webappsec#468 (comment).

Member

mikewest commented Oct 7, 2015

From @hillbrad on September 8, 2015 18:49

Do those event handlers have to be inline? Is there a reason the necessary
elements can't be tagged by id or class and the event handlers added
imperatively by a script block?

On Tue, Sep 8, 2015 at 11:41 AM Paweł Krawczyk notifications@github.com
wrote:

Thanks, these are very helpful notes. I have already filed a Chrome bug
report about this message, but wanted to confirm here what should be the
correct browser's policy.

As for inline event handlers I do see quite a lot of them, especially if
you include a Google AdSense code. While perhaps the correct approach
should be to rewrite AdSense code not to use inline handlers, right now it
is a real problem if you want to enable CSP on real world websites.


Reply to this email directly or view it on GitHub
w3c/webappsec#468 (comment).

@mikewest

This comment has been minimized.

Show comment
Hide comment
@mikewest

mikewest Oct 7, 2015

Member

From @kravietz on September 8, 2015 18:52

I'm sure they don't have to but in case of embedding a third party code such as Google AdSense you don't have much influence on how it's written. A workaround I came up with is to load AdSense code in a IFRAME from an URL that has a separate, more liberal CSP.

Member

mikewest commented Oct 7, 2015

From @kravietz on September 8, 2015 18:52

I'm sure they don't have to but in case of embedding a third party code such as Google AdSense you don't have much influence on how it's written. A workaround I came up with is to load AdSense code in a IFRAME from an URL that has a separate, more liberal CSP.

@mikewest

This comment has been minimized.

Show comment
Hide comment
@mikewest

mikewest Oct 7, 2015

Member

From @kravietz on September 10, 2015 5:46

As for identifying context for whitelisting inline handlers I think it would be quite easy thing to do. For example for such handler:

<button id="action1" onclick="doSubmit()">

This might be whitelisted with the following hash expression:

sha256-jzgBGA4UWFFmpOBq0JpdsySukE1FrEN5bUpoK8Z29fY=#action1#click
Member

mikewest commented Oct 7, 2015

From @kravietz on September 10, 2015 5:46

As for identifying context for whitelisting inline handlers I think it would be quite easy thing to do. For example for such handler:

<button id="action1" onclick="doSubmit()">

This might be whitelisted with the following hash expression:

sha256-jzgBGA4UWFFmpOBq0JpdsySukE1FrEN5bUpoK8Z29fY=#action1#click
@metromoxie

This comment has been minimized.

Show comment
Hide comment
@metromoxie

metromoxie Oct 28, 2015

Contributor

FWIW, this was a very specific choice we made in the design because there are quite a few corner cases around inline handlers. Of course, I can't recall any of them right now :-)

Contributor

metromoxie commented Oct 28, 2015

FWIW, this was a very specific choice we made in the design because there are quite a few corner cases around inline handlers. Of course, I can't recall any of them right now :-)

@mikewest

This comment has been minimized.

Show comment
Hide comment
@mikewest

mikewest Feb 18, 2016

Member

Based on some feedback from folks implementing CSP, it would make things much easier to deploy if we allowed hashes to whitelist inline handlers. The risk boils down to an attacker taking a whitelisted action, and moving it to another element or handler. Honestly, that seems low-risk enough to not worry too much about the syntactical strangeness that #click#load#etc would introduce.

Member

mikewest commented Feb 18, 2016

Based on some feedback from folks implementing CSP, it would make things much easier to deploy if we allowed hashes to whitelist inline handlers. The risk boils down to an attacker taking a whitelisted action, and moving it to another element or handler. Honestly, that seems low-risk enough to not worry too much about the syntactical strangeness that #click#load#etc would introduce.

@mikewest

This comment has been minimized.

Show comment
Hide comment
@mikewest

mikewest Feb 18, 2016

Member

Also, Firefox is apparently already doing this for style (and script?) according to https://code.google.com/p/chromium/issues/detail?id=546106.

@dveditz, WDYT?

Member

mikewest commented Feb 18, 2016

Also, Firefox is apparently already doing this for style (and script?) according to https://code.google.com/p/chromium/issues/detail?id=546106.

@dveditz, WDYT?

@michaelficarra

This comment has been minimized.

Show comment
Hide comment
@michaelficarra

michaelficarra Feb 21, 2016

Contributor

👎 I think there is a very real security risk here. A developer attempting to whitelist an inline script/style for a particular use may not be aware that it could be abused to apply malicious behaviour/styling to another element. And even if they were aware of this, it would be difficult to evaluate all the ways that this behaviour/styling could be used maliciously on every other element on the page.

Contributor

michaelficarra commented Feb 21, 2016

👎 I think there is a very real security risk here. A developer attempting to whitelist an inline script/style for a particular use may not be aware that it could be abused to apply malicious behaviour/styling to another element. And even if they were aware of this, it would be difficult to evaluate all the ways that this behaviour/styling could be used maliciously on every other element on the page.

@mikewest

This comment has been minimized.

Show comment
Hide comment
@mikewest

mikewest Feb 22, 2016

Member

@michaelficarra: Would you be less sad if the developer had to opt-into this behavior with something like unsafe-apply-hashes-to-inline-event-handlers (or something shorter :) )?

Member

mikewest commented Feb 22, 2016

@michaelficarra: Would you be less sad if the developer had to opt-into this behavior with something like unsafe-apply-hashes-to-inline-event-handlers (or something shorter :) )?

@michaelficarra

This comment has been minimized.

Show comment
Hide comment
@michaelficarra

michaelficarra Feb 24, 2016

Contributor

@mikewest Yes. Or something longer 😉.

Contributor

michaelficarra commented Feb 24, 2016

@mikewest Yes. Or something longer 😉.

@mikewest mikewest closed this in a2bb43f Apr 14, 2016

dstockwell pushed a commit to dstockwell/chromium that referenced this issue Apr 28, 2016

CSP: Allow hashed inline event handlers only with 'unsafe-hashed-attr…
…ibutes'

The original implementation didn't gate this on any particular signal in
the policy. Based on the discussion at w3c/webappsec-csp#13,
we should be a bit more cautious. So, 'unsafe-hashed-attributes' it is.

BUG=546106

Review-Url: https://codereview.chromium.org/1923273002
Cr-Commit-Position: refs/heads/master@{#390418}
@dracos

This comment has been minimized.

Show comment
Hide comment
@dracos

dracos Nov 7, 2017

"Do those event handlers have to be inline? Is there a reason the necessary elements can't be tagged by id or class and the event handlers added imperatively by a script block?" – I have stumbled across this issue because I wish to use <link rel="preload" href="style.css" as="style"> on a Content-Security-Policy-using site. All the docs (and the spec) for rel="preload" say I should then add onload="this.rel='stylesheet'" in order to activate the preload once loaded. But I cannot use inline event handlers on this site.

The best thing I can come up with is <link id="preload_base_css" rel="preload" href="style.css" as="style"> <script nonce="nonce">document.getElementById('preload_base_css').onload = function(){this.rel='stylesheet'}; but in testing (though I think with a few lines between the link and the script), if the CSS is already cached, say, it looks like it is possible for the link's onload event to trigger before the handler is attached, which makes it a bit useless. It might be okay like that, but I don't know if it's possible to guarantee the handler will be registered before it fires if it's not on the element itself?

Anyway, just saying this might be useful for that scenario, unless there's also a way I can add my CSP nonce to the <link> and have that permit inline event handlers or something else I haven't considered.

dracos commented Nov 7, 2017

"Do those event handlers have to be inline? Is there a reason the necessary elements can't be tagged by id or class and the event handlers added imperatively by a script block?" – I have stumbled across this issue because I wish to use <link rel="preload" href="style.css" as="style"> on a Content-Security-Policy-using site. All the docs (and the spec) for rel="preload" say I should then add onload="this.rel='stylesheet'" in order to activate the preload once loaded. But I cannot use inline event handlers on this site.

The best thing I can come up with is <link id="preload_base_css" rel="preload" href="style.css" as="style"> <script nonce="nonce">document.getElementById('preload_base_css').onload = function(){this.rel='stylesheet'}; but in testing (though I think with a few lines between the link and the script), if the CSS is already cached, say, it looks like it is possible for the link's onload event to trigger before the handler is attached, which makes it a bit useless. It might be okay like that, but I don't know if it's possible to guarantee the handler will be registered before it fires if it's not on the element itself?

Anyway, just saying this might be useful for that scenario, unless there's also a way I can add my CSP nonce to the <link> and have that permit inline event handlers or something else I haven't considered.

@andypaicu

This comment has been minimized.

Show comment
Hide comment
@andypaicu

andypaicu Apr 5, 2018

Collaborator

I have sent an e-mail on public-webappsec but in case someone follows this thread more closely, I will also leave a comment here.

I have put together an explainer for this and I would like to hear your thoughts.

https://docs.google.com/document/d/1_nYS4gWYO2Oh8rYDyPglXIKNsgCRVhmjHqWlTAHst7c/edit?usp=sharing

Collaborator

andypaicu commented Apr 5, 2018

I have sent an e-mail on public-webappsec but in case someone follows this thread more closely, I will also leave a comment here.

I have put together an explainer for this and I would like to hear your thoughts.

https://docs.google.com/document/d/1_nYS4gWYO2Oh8rYDyPglXIKNsgCRVhmjHqWlTAHst7c/edit?usp=sharing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment