| Summary: | Implement CSP strict-dynamic for module scripts | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kate Cheney <katherine_cheney> | ||||||
| Component: | New Bugs | Assignee: | Kate Cheney <katherine_cheney> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bfulgham, cdumez, esprehn+autocc, ews-watchlist, kangil.han, mkwst | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Kate Cheney
2022-01-06 13:26:09 PST
Created attachment 448534 [details]
Patch
Comment on attachment 448534 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448534&action=review r=me > Source/WebCore/dom/ScriptElement.cpp:379 > + if (!contentSecurityPolicy.allowNonParserInsertedScripts(m_element.document().url(), m_startLineNumber, m_element.nonce(), sourceCode.source(), m_parserInserted)) Is there a specific CSP spec step we should reference for this decision? I know most of the CSP code doesn't add such comments, but I find them helpful when reviewing other areas of WebKit. > LayoutTests/http/tests/security/contentSecurityPolicy/strict-dynamic-module-script.html:6 > + <script src="/js-test-resources/testharnessreport.js" nonce='dummy'></script> Nit: Weird spacing here Comment on attachment 448534 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448534&action=review >> Source/WebCore/dom/ScriptElement.cpp:379 >> + if (!contentSecurityPolicy.allowNonParserInsertedScripts(m_element.document().url(), m_startLineNumber, m_element.nonce(), sourceCode.source(), m_parserInserted)) > > Is there a specific CSP spec step we should reference for this decision? I know most of the CSP code doesn't add such comments, but I find them helpful when reviewing other areas of WebKit. Do you mean the m_startLineNumber change or why we check for allowNonParserInsertedScripts here? If it is the latter, generally yes but not specifically to module scripts. Under https://www.w3.org/TR/CSP3/#strict-dynamic-usage the spec explains "Script requests which are triggered by non-"parser-inserted" script elements are allowed." requestModuleScript is a place we were formerly not checking a requested script's parser-inserted status. Comment on attachment 448534 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448534&action=review >>> Source/WebCore/dom/ScriptElement.cpp:379 >>> + if (!contentSecurityPolicy.allowNonParserInsertedScripts(m_element.document().url(), m_startLineNumber, m_element.nonce(), sourceCode.source(), m_parserInserted)) >> >> Is there a specific CSP spec step we should reference for this decision? I know most of the CSP code doesn't add such comments, but I find them helpful when reviewing other areas of WebKit. > > Do you mean the m_startLineNumber change or why we check for allowNonParserInsertedScripts here? > > If it is the latter, generally yes but not specifically to module scripts. Under https://www.w3.org/TR/CSP3/#strict-dynamic-usage the spec explains "Script requests which are triggered by non-"parser-inserted" script elements are allowed." requestModuleScript is a place we were formerly not checking a requested script's parser-inserted status. I meant the latter, so I think you've explained it. Created attachment 448604 [details]
Patch for landing
Committed r287756 (245819@main): <https://commits.webkit.org/245819@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 448604 [details]. |