RESOLVED FIXED 94918
'self' in a CSP directive should match blob: and filesystem: URLs. 
https://bugs.webkit.org/show_bug.cgi?id=94918
Summary 'self' in a CSP directive should match blob: and filesystem: URLs.
Mike West
Reported 2012-08-24 03:15:42 PDT
blob:https://example.com/xxx is same-origin with https://example.com/. We should treat it as such when matching URLs against Content Security Policy directives.
Attachments
Patch (8.45 KB, patch)
2012-08-24 04:04 PDT, Mike West 		no flags
DetailsFormatted DiffDiff
Patch (15.71 KB, patch)
2012-08-25 15:22 PDT, Mike West 		no flags
DetailsFormatted DiffDiff
Patch (17.42 KB, patch)
2012-08-27 12:05 PDT, Mike West 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Mike West
Comment 1 2012-08-24 04:04:05 PDT
Created attachment 160387 [details] Patch
Adam Barth
Comment 2 2012-08-24 07:58:20 PDT
Comment on attachment 160387 [details] Patch I'm surprised this doesn't cause any tests to fail. Won't this mess up data URLs?
Adam Barth
Comment 3 2012-08-24 07:59:00 PDT
Rather than using SecurityOrigin, I wonder if we should use the hasInnerURL and extraceInnerURL functions (those names aren't 100% right).
Mike West
Comment 4 2012-08-24 08:08:07 PDT
(In reply to comment #2) > (From update of attachment 160387 [details]) > I'm surprised this doesn't cause any tests to fail. Won't this mess up data URLs? I'm pretty sure all the data tests simply check that the protocol-only source 'data:' works. In that case, the SecurityOrigin returns 'data:' as the protocol, so it works (he says, confidently, without having looked at either the tests or the code). > Rather than using SecurityOrigin, I wonder if we should use the hasInnerURL and extraceInnerURL functions (those names aren't 100% right). Neither 'shouldUseInnerURL' nor 'extractInnerURL' are public at the moment. I vaguely remember having a similar discussion when I tweaked 'blob:'s mixed-content behavior... *shrug* This patch has the advantage of not touching SecurityOrigin, but I'm happy to add those methods to SecurityOrigin's public signature if you'd like. :)
Mike West
Comment 5 2012-08-25 15:22:10 PDT
Created attachment 160572 [details] Patch
Mike West
Comment 6 2012-08-25 15:27:56 PDT
(In reply to comment #5) > Created an attachment (id=160572) [details] > Patch I've adjusted SecurityOrigin to include these two static functions for innerURL extraction. This patch also moves the innerURL magic out of CSPSource::match and into CSPSourceList::match, which should be more efficient, and adds a 'data:' URL test, as it doesn't look like we were explicitly testing it before.
Adam Barth
Comment 7 2012-08-27 11:29:37 PDT
Comment on attachment 160572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160572&action=review > Source/WebCore/page/ContentSecurityPolicy.cpp:247 > + KURL urlToMatch = SecurityOrigin::shouldUseInnerURL(url) ? SecurityOrigin::extractInnerURL(url) : url; urlToMatch -> effectiveURL > Source/WebCore/page/SecurityOrigin.cpp:195 > + if (SecurityOrigin::shouldUseInnerURL(url)) > + return adoptRef(new SecurityOrigin(SecurityOrigin::extractInnerURL(url))); "SecurityOrigin::" isn't needed here because SecurityOrigin::create is already in the SecurityOrigin namespace. > Source/WebCore/page/SecurityOrigin.cpp:225 > - if (shouldUseInnerURL(url) && SchemeRegistry::shouldTreatURLSchemeAsSecure(extractInnerURL(url).protocol())) > + if (SecurityOrigin::shouldUseInnerURL(url) && SchemeRegistry::shouldTreatURLSchemeAsSecure(SecurityOrigin::extractInnerURL(url).protocol())) ditto
Mike West
Comment 8 2012-08-27 12:05:48 PDT
Created attachment 160768 [details] Patch
WebKit Review Bot
Comment 9 2012-08-27 13:08:16 PDT
Comment on attachment 160768 [details] Patch Clearing flags on attachment: 160768 Committed r126785: <http://trac.webkit.org/changeset/126785>
WebKit Review Bot
Comment 10 2012-08-27 13:08:20 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.