| Summary: | CSP: Implement protections against nonce-hijacking | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Patrick Griffis <pgriffis> | ||||||||||
| Component: | WebCore Misc. | Assignee: | Patrick Griffis <pgriffis> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | bfulgham, cdumez, changseok, cmarcelo, esprehn+autocc, ews-watchlist, gyuyoung.kim, kangil.han, katherine_cheney, rniwa, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Bug Depends on: | 233102 | ||||||||||||
| Bug Blocks: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Patrick Griffis
2021-11-13 10:16:52 PST
Created attachment 444141 [details]
Patch
Created attachment 444143 [details]
Patch
Comment on attachment 444143 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444143&action=review > LayoutTests/imported/w3c/web-platform-tests/content-security-policy/script-src/nonce-enforce-blocked-expected.txt:2 > +FAIL Unnonced scripts generate reports. Type error Note that this is a different bug in CSP reporting code that I'll be looking into fixing. The previous error was an assert much earlier where unsafe code was ran which no longer does. (In reply to Patrick Griffis from comment #3) > Note that this is a different bug in CSP reporting code that I'll be looking > into fixing. Fixed in bug 233102, I'll update expectations here as PASS after that lands. Created attachment 444244 [details]
Patch
Comment on attachment 444244 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444244&action=review r=me > Source/WebCore/dom/Element.cpp:334 > + const auto& name = attribute.localName(); If you did "localName().convertToASCIILowercase()", you could do the following four string comparisons without doing case insensitive compares. > Source/WebCore/dom/Element.cpp:335 > + const auto& value = attribute.value(); Ditto for value Comment on attachment 444244 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444244&action=review > Source/WebCore/dom/Element.h:769 > + bool isNonceable() const; Please move this up. We don't put getters in the middle of data members. > Source/WebCore/html/parser/HTMLConstructionSite.cpp:517 > + setAttributes(element, attributes, false, m_parserContentPolicy); false here is really unclear. This should use an enum class, not a bool. e.g.: enum class HasDuplicateAttribute : bool { No, Yes }; Created attachment 446741 [details]
Patch
(In reply to Chris Dumez from comment #8) > Comment on attachment 444244 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=444244&action=review > > > Source/WebCore/dom/Element.h:769 > > + bool isNonceable() const; > > Please move this up. We don't put getters in the middle of data members. > > > Source/WebCore/html/parser/HTMLConstructionSite.cpp:517 > > + setAttributes(element, attributes, false, m_parserContentPolicy); > > false here is really unclear. This should use an enum class, not a bool. > e.g.: > enum class HasDuplicateAttribute : bool { No, Yes }; Before committing this just wanted to make sure my changes look good. Comment on attachment 446741 [details]
Patch
r=me
Committed r286860 (245093@main): <https://commits.webkit.org/245093@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 446741 [details]. |