RESOLVED FIXED 99274
Warn when CSP headers don't separate directives with ';'. 
https://bugs.webkit.org/show_bug.cgi?id=99274
Summary Warn when CSP headers don't separate directives with ';'.
Mike West
Reported 2012-10-14 13:56:24 PDT
If a web author specifies "default-src 'self' img-src *", they're almost certainly making a mistake. We might want to throw a warning if we see a source list that contains a directive name. See https://twitter.com/soaj1664ashar/status/257468299372277760 for example. WDYT, Adam?
Attachments
Patch (15.00 KB, patch)
2012-10-15 01:26 PDT, Mike West 		no flags
DetailsFormatted DiffDiff
Patch (10.74 KB, patch)
2012-10-15 12:37 PDT, Mike West 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2012-10-14 23:00:17 PDT
Hum... Maybe if there's a source-expression that matches a directive name, we should issue a warning? Technically it could be an intranet host with that name, but that's pretty unlikely.
Mike West
Comment 2 2012-10-15 01:26:20 PDT
Created attachment 168647 [details] Patch
Mike West
Comment 3 2012-10-15 01:27:57 PDT
(In reply to comment #1) > Hum... Maybe if there's a source-expression that matches a directive name, we should issue a warning? Technically it could be an intranet host with that name, but that's pretty unlikely. That's what I was thinking as well. The attached patch is a stab at that. It also moves the directive names out of CSPDirectiveList::addDirective into static methods of ContentSecurityPolicy so I can reuse them. This seems to be the right thing to do, but I'm not entirely sure. I saw a few 'static char xxx[] = "...";' lines while grepping around... WDYT?
Adam Barth
Comment 4 2012-10-15 09:19:03 PDT
Comment on attachment 168647 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168647&action=review > Source/WebCore/page/ContentSecurityPolicy.h:87 > + static const char* connectSrc() { return "connect-src"; } > + static const char* defaultSrc() { return "default-src"; } > + static const char* fontSrc() { return "font-src"; } > + static const char* frameSrc() { return "frame-src"; } > + static const char* imgSrc() { return "img-src"; } > + static const char* mediaSrc() { return "media-src"; } > + static const char* objectSrc() { return "object-src"; } > + static const char* reportURI() { return "report-uri"; } > + static const char* sandbox() { return "sandbox"; } > + static const char* scriptSrc() { return "script-src"; } > + static const char* styleSrc() { return "style-src"; } > +#if ENABLE(CSP_NEXT) > + static const char* formAction() { return "form-action"; } > + static const char* pluginTypes() { return "plugin-types"; } > + static const char* scriptNonce() { return "script-nonce"; } > +#endif Rather thank making these functions, we should just make them static constants in the cpp file. > Source/WebCore/page/ContentSecurityPolicy.h:89 > + static bool isDirectiveName(const String& name); Should this be private?
Mike West
Comment 5 2012-10-15 12:37:34 PDT
Created attachment 168755 [details] Patch
Mike West
Comment 6 2012-10-15 22:07:42 PDT
Cool. Throwing this into the queue.
WebKit Review Bot
Comment 7 2012-10-15 22:25:37 PDT
Comment on attachment 168755 [details] Patch Clearing flags on attachment: 168755 Committed r131413: <http://trac.webkit.org/changeset/131413>
WebKit Review Bot
Comment 8 2012-10-15 22:25:41 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.