'self' in a CSP directive should match blob: and filesystem: URLs.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Mike West]

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.

[Mike West]

Created attachment 160387 [details]
Patch

[Adam Barth]

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]

Rather than using SecurityOrigin, I wonder if we should use the hasInnerURL and extraceInnerURL functions (those names aren't 100% right).

[Mike West]

(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]

Created attachment 160572 [details]
Patch

[Mike West]

(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 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]

Created attachment 160768 [details]
Patch

[WebKit Review Bot]

Comment on attachment 160768 [details]
Patch

Clearing flags on attachment: 160768

Committed r126785: <http://trac.webkit.org/changeset/126785>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.