Description

mikewest

opened on Nov 27, 2017

Issue body actions

CSP attempts to defend against some kinds of dangling-markup attacks by preventing the execution of scripts that include "<script" in an attribute name or value. Discarding attributes during parsing makes it possible to trivially work around this defense, as described in https://crbug.com/740615. As dropping repeated attributes is also the root cause of the dangling-markup risk in the first place, it might be reasonable to dig into it a little more deeply to see if something more fundamental can be done.

One idea we (briefly) discussed at TPAC is to prevent nonced script execution for <script> elements which had duplicate-attribute parse errors. An approach to this that I'm exploring in https://chromium-review.googlesource.com/c/chromium/src/+/566822 does more or less the following:

1. Records duplicate-attribute parse errors on the tag during tokenization.
2. Persists that error bit onto the Node generated during tree-building.
3. Reads that error bit during "Is element nonceable?".

WDYT?

/cc @whatwg/security (@ckerschb, @dbates-wk, @patrickkettner in particular as folks working on CSP)

Activity

1. mikewest

   added a commit that references this issue on Nov 27, 2017

   Elements with duplicated attributes are not nonceable.

   6493935
2. annevk

   added

   security/privacyThere are security or privacy implicationsThere are security or privacy implications

   topic: parser

   on Nov 27, 2017
3. annevk commented on Nov 27, 2017

   

   annevk

   on Nov 27, 2017

   Member

   More actions

   cc @whatwg/html-parser
4. bzbarsky commented on Nov 28, 2017

   

   bzbarsky

   on Nov 28, 2017

   Contributor

   More actions

   @hsivonen
5. andypaicu commented on Nov 9, 2018

   

   andypaicu

   on Nov 9, 2018

   Contributor

   More actions

   Unfortunately this is still a pretty big hole in nonce based CSP policies because it opens very simple avenues for injection points to become dangling-markup nonce-stealing attacks.

   Does the above approach detailed by Mike seem reasonable. What are peoples thoughts?
6. gsnedders commented on Nov 9, 2018

   

   gsnedders

   on Nov 9, 2018

   Member

   More actions

   To me this seems like a good idea given the potential threats, though it definitely adds some complexity (given the need to propagate the error). That said, my opinion probably shouldn't for a huge amount here. 🙂
7. hsivonen commented on Nov 12, 2018

   

   hsivonen

   on Nov 12, 2018

   Member

   More actions

   Propagating a "this script is questionable enough that it should not run" bit to the DOM is doable from the parser perspective. Gecko already does this for scripts that were truncated by early EOF.

   Let's not overdo it, though. Let's make it just one bit that says the there was at least one duplicate attribute without trying to retain knowledge of which attribute was duplicated and how all the way through the stack. Or even just one bit saying the script isn't eligible to run (in case we want to add other reasons later or combine it right away with the truncation-by-early-EOF case).

   Reacted by sideshowbarker and Anne van Kesteren
8. aarongable

   added a commit that references this issue on Dec 3, 2018

   Reland of "CSP: Block nonces for elements with duplicate attributes."

   20c6560
9. andypaicu commented on Dec 5, 2018

   

   andypaicu

   on Dec 5, 2018

   Contributor

   More actions

   I've raised the 2 referenced PR's, please have a look and let me know what you think.
10. ryandel8834

    added a commit that references this issue on Aug 13, 2022

    Elements with duplicated attributes are not nonceable.

    6ca1aba
11. evilpie commented on Jan 26, 2024

    

    evilpie

    on Jan 26, 2024

    Member

    More actions

    This is still a problem, that I discovered while implementing the nonceable check in Firefox.

Issue actions