Bug 233087 - CSP: Implement protections against nonce-hijacking
Summary: CSP: Implement protections against nonce-hijacking
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Patrick Griffis
URL:
Keywords: InRadar
Depends on: 233102
Blocks:
  Show dependency treegraph
 
Reported: 2021-11-13 10:16 PST by Patrick Griffis
Modified: 2021-12-10 11:02 PST (History)
11 users (show)

See Also:


Attachments
Patch (8.41 KB, patch)
2021-11-13 10:17 PST, Patrick Griffis
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (8.41 KB, patch)
2021-11-13 10:39 PST, Patrick Griffis
no flags Details | Formatted Diff | Diff
Patch (8.40 KB, patch)
2021-11-15 06:19 PST, Patrick Griffis
no flags Details | Formatted Diff | Diff
Patch (9.07 KB, patch)
2021-12-10 09:15 PST, Patrick Griffis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Griffis 2021-11-13 10:16:52 PST
CSP: Implement protections against nonce-hijacking
Comment 1 Patrick Griffis 2021-11-13 10:17:45 PST
Created attachment 444141 [details]
Patch
Comment 2 Patrick Griffis 2021-11-13 10:39:22 PST
Created attachment 444143 [details]
Patch
Comment 3 Patrick Griffis 2021-11-13 11:26:18 PST
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.
Comment 4 Patrick Griffis 2021-11-14 11:45:48 PST
(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.
Comment 5 Patrick Griffis 2021-11-15 06:19:28 PST
Created attachment 444244 [details]
Patch
Comment 6 Radar WebKit Bug Importer 2021-11-20 10:17:20 PST
<rdar://problem/85635546>
Comment 7 Brent Fulgham 2021-12-08 12:21:27 PST
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 8 Chris Dumez 2021-12-08 12:31:12 PST
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 };
Comment 9 Patrick Griffis 2021-12-10 09:15:10 PST
Created attachment 446741 [details]
Patch
Comment 10 Patrick Griffis 2021-12-10 09:27:06 PST
(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 11 Brent Fulgham 2021-12-10 09:32:07 PST
Comment on attachment 446741 [details]
Patch

r=me
Comment 12 EWS 2021-12-10 11:02:04 PST
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].