search

whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.18k stars 2.71k forks source link

Consider a new flag `was_parser_inserted` to record initial `parser_inserted` state #4235

Open andypaicu opened 5 years ago

andypaicu commented 5 years ago

The parser-inserted flag is passed to parser metadata which is used in CSP to determine whether scripts are allowed to run. In the presence of strict-dynamic, any script that is not parser inserted is allowed to run by default(https://w3c.github.io/webappsec-csp/#script-pre-request step 4).

But in step 2 of prepare a script this flag is unset and it is set back in step 8 which means that invalid parser-inserted script blocks are considered as not parser-inserted. While this is done intentionally as noted right below step 2, this behavior does not work for the CSP check as it can open some vulnerabilities (https://github.com/w3c/webappsec-csp/issues/376).

I see some ways to address this: 1) Add another flag was-parser-inserted that is used instead of the variable was-parser-inserted throughout prepare a script but is actually stored on the element. Once this flag is set it is never unset so it keeps its initial state. On step 21 we use this flag for initializing the parser metadata.

2) Change the parser-inserted from a flag to a triple state that accounts for the state of parser-inserted-but-invalid (name WIP :D). At step 2 we set parser-inserted to parser-inserted-but-invalid if its parser-inserted and the script might stay in this state. This state mostly behaves the same as parser-inserted being false with the exception that it sets the parser metadata to parser-inserted at step 21.

What are you thoughts on this? I think option 1 is a bit cleaner personally. Is there perhaps a better option that I'm missing?

annevk commented 5 years ago

I'm inclined to agree, but I don't have this all internalized, maybe @domenic?

domenic commented 5 years ago

I've replied at https://github.com/w3c/webappsec-csp/issues/376. I don't understand the use case for distinguishing at this point; right now the spec tries to maintain a symmetry so the parser-inserted flag is only set for scripts whose contents are parser-inserted, and treats dynamic script bodies the same, no matter where the original element came from. This seems more correct, but let's discuss over there if there's something I'm missing, which would make us break this symmetry.