Bug 153158 - CSP: 'blob:' URLs should not match 'self' in CSP source expression lists.
Summary: CSP: 'blob:' URLs should not match 'self' in CSP source expression lists.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: BlinkMergeCandidate, InRadar, WebExposed
Depends on: 153562
Blocks:
  Show dependency treegraph
 
Reported: 2016-01-15 15:10 PST by Daniel Bates
Modified: 2016-02-12 16:24 PST (History)
6 users (show)

See Also:


Attachments
Patch and Layout Tests (12.22 KB, patch)
2016-02-11 13:54 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (1.13 MB, application/zip)
2016-02-11 14:25 PST, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (1.09 MB, application/zip)
2016-02-11 14:41 PST, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-yosemite (925.08 KB, application/zip)
2016-02-11 14:46 PST, Build Bot
no flags Details
Patch and Layout Tests (12.37 KB, patch)
2016-02-12 10:37 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-yosemite (1.07 MB, application/zip)
2016-02-12 11:13 PST, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (1.04 MB, application/zip)
2016-02-12 11:17 PST, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-yosemite (885.73 KB, application/zip)
2016-02-12 11:35 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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.