Bug 153158

Summary: CSP: 'blob:' URLs should not match 'self' in CSP source expression lists.
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebCore Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, buildbot, commit-queue, mkwst, rniwa, webkit-bug-importer
Priority: P2 Keywords: BlinkMergeCandidate, InRadar, WebExposed
Version: WebKit Local Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=154122
Bug Depends on: 153562    
Bug Blocks:    
Attachments:
Description Flags
Patch and Layout Tests
none
Archive of layout-test-results from ews101 for mac-yosemite
none
Archive of layout-test-results from ews104 for mac-yosemite-wk2
none
Archive of layout-test-results from ews113 for mac-yosemite
none
Patch and Layout Tests
none
Archive of layout-test-results from ews103 for mac-yosemite
none
Archive of layout-test-results from ews106 for mac-yosemite-wk2
none
Archive of layout-test-results from ews115 for mac-yosemite none

Description Daniel Bates 2016-01-15 15:10:01 PST
We should merge <https://src.chromium.org/viewvc/blink?view=rev&revision=197354>.

'blob:' URLs should not match 'self' in CSP source expression lists.

Chrome is currently treating `'self'` as including `blob:` URLs. That's
against the spec (and Firefox), which requires whitelisting `blob:`
explicitly:

https://w3c.github.io/webappsec/specs/content-security-policy/#source-list-guid-matching.

This patch fixes our implementation.

Mozilla discussion here: https://bugzilla.mozilla.org/show_bug.cgi?id=1150957
Comment 1 Radar WebKit Bug Importer 2016-01-27 20:46:04 PST
<rdar://problem/24383264>
Comment 2 Daniel Bates 2016-02-11 13:54:08 PST
Created attachment 271081 [details]
Patch and Layout Tests
Comment 3 Build Bot 2016-02-11 14:25:34 PST
Comment on attachment 271081 [details]
Patch and Layout Tests

Attachment 271081 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/816263

New failing tests:
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-blocks-eval.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-block-aborts-all-subsequent-imports.html
Comment 4 Build Bot 2016-02-11 14:25:36 PST
Created attachment 271087 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 5 Build Bot 2016-02-11 14:41:28 PST
Comment on attachment 271081 [details]
Patch and Layout Tests

Attachment 271081 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/816306

New failing tests:
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-blocks-eval.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-block-aborts-all-subsequent-imports.html
Comment 6 Build Bot 2016-02-11 14:41:30 PST
Created attachment 271091 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 7 Build Bot 2016-02-11 14:46:06 PST
Comment on attachment 271081 [details]
Patch and Layout Tests

Attachment 271081 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/816308

New failing tests:
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-blocks-eval.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-block-aborts-all-subsequent-imports.html
Comment 8 Build Bot 2016-02-11 14:46:07 PST
Created attachment 271093 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 9 Daniel Bates 2016-02-11 16:22:09 PST
(In reply to comment #7)
> Comment on attachment 271081 [details]
> Patch and Layout Tests
> 
> Attachment 271081 [details] did not pass mac-debug-ews (mac):
> Output: http://webkit-queues.webkit.org/results/816308
> 
> New failing tests:
> http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-
> importScripts-redirect-cross-origin-blocked.html
> http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-blocks-
> eval.html
> http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp.html
> http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-
> importScripts-block-aborts-all-subsequent-imports.html

These failures are because this patch depends on the patch for bug #153562. Without the patch for bug #153562, the script URL of a Web Worker is checked against the script-src directive as opposed to the child-src directive. As a workaround for this bug (bug # 153158) these tests needed to put 'self' or http://127.0.0.1:8000 to allow the load of a blob URL Web Worker script. The failure of these tests indicate that the proposed patch (attachment #271081 [details]) works as intended as a blob URL matches neither 'self' nor http://127.0.0.1:8000.
Comment 10 Brent Fulgham 2016-02-12 09:01:11 PST
Comment on attachment 271081 [details]
Patch and Layout Tests

View in context: https://bugs.webkit.org/attachment.cgi?id=271081&action=review

> Source/WebCore/ChangeLog:16
> +        (WebCore::ContentSecurityPolicySourceList::matches):

Could you add some text explaining why the SecurityOrigin::extractInnerlURL is no longer appropriate here?

> Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp:-105
> -    if (m_allowSelf && m_policy.urlMatchesSelf(effectiveURL))

I don't understand this change. Were we wrong to be considering the SecurityOrigin rules here?
Comment 11 Daniel Bates 2016-02-12 10:30:32 PST
(In reply to comment #10)
> Comment on attachment 271081 [details]
> Patch and Layout Tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=271081&action=review
> 
> > Source/WebCore/ChangeLog:16
> > +        (WebCore::ContentSecurityPolicySourceList::matches):
> 
> Could you add some text explaining why the SecurityOrigin::extractInnerlURL
> is no longer appropriate here?
> 

Will add following text:

Do not make a distinction between URLs that contain a nested URL (e.g. blob://http://www.example.com/...) and URLs that do not contain a nested URL. The URL of the requested resource should be matched against the source list sources expressions.

> > Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp:-105
> > -    if (m_allowSelf && m_policy.urlMatchesSelf(effectiveURL))
> 
> I don't understand this change. Were we wrong to be considering the
> SecurityOrigin rules here?

Yes, we should not have made use of SecurityOrigin to differentiate between URLs that contain a nested URL and URLs that do not.
Comment 12 Daniel Bates 2016-02-12 10:37:38 PST
Created attachment 271186 [details]
Patch and Layout Tests

Updated patch based on Brent Fulgham's feedback.
Comment 13 Brent Fulgham 2016-02-12 10:51:36 PST
Comment on attachment 271186 [details]
Patch and Layout Tests

r=me
Comment 14 Build Bot 2016-02-12 11:13:52 PST
Comment on attachment 271186 [details]
Patch and Layout Tests

Attachment 271186 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/820286

New failing tests:
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-blocks-eval.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-block-aborts-all-subsequent-imports.html
Comment 15 Build Bot 2016-02-12 11:13:54 PST
Created attachment 271191 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 16 Build Bot 2016-02-12 11:17:50 PST
Comment on attachment 271186 [details]
Patch and Layout Tests

Attachment 271186 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/820295

New failing tests:
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-blocks-eval.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-block-aborts-all-subsequent-imports.html
Comment 17 Build Bot 2016-02-12 11:17:52 PST
Created attachment 271192 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 18 Build Bot 2016-02-12 11:35:06 PST
Comment on attachment 271186 [details]
Patch and Layout Tests

Attachment 271186 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/820426

New failing tests:
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-blocks-eval.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp.html
http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-block-aborts-all-subsequent-imports.html
Comment 19 Build Bot 2016-02-12 11:35:09 PST
Created attachment 271195 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 20 Daniel Bates 2016-02-12 16:24:27 PST
Comment on attachment 271186 [details]
Patch and Layout Tests

Clearing flags on attachment: 271186

Committed r196528: <http://trac.webkit.org/changeset/196528>
Comment 21 Daniel Bates 2016-02-12 16:24:31 PST
All reviewed patches have been landed.  Closing bug.