Closed
Bug 1229639
Opened 10 years ago
Closed 9 years ago
Content Security Policy: matching %-encoded path fails
Categories
(Core :: DOM: Security, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla51
| Tracking | Status | |
|---|---|---|
| firefox51 | --- | fixed |
People
(Reporter: kontakt, Assigned: hchang)
References
()
Details
(Whiteboard: [domsecurity-backlog1][domsecurity-active])
Attachments
(3 files)
Screenshot
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20151201004002
Steps to reproduce:
Visit https://demos.scheurle.info/firefox/csp-url-matching/ .
Actual results:
Of the 2 images whitelistet in the page's CSP only the first is loaded. The second one, having a path containing %-encoded international characters, gets blocked.
Expected results:
Both images should be allowed to load.
|
Reporter | |
Updated•10 years ago
|
Severity: normal → major
Component: Untriaged → DOM: Security
OS: Unspecified → Mac OS X
Product: Firefox → Core
Hardware: Unspecified → x86
|
||
Comment 1•10 years ago
|
||
I can reproduce that problem, we have to investigate closer and fix.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [domsecurity-backlog]
|
|
||
Comment 2•10 years ago
|
||
Thanks Chris for filing those issues.
|
|
||
Updated•9 years ago
|
Priority: -- → P1
|
|
||
Updated•9 years ago
|
Priority: P1 → P3
Whiteboard: [domsecurity-backlog] → [domsecurity-backlog1]
|
Assignee | |
Updated•9 years ago
|
Assignee: nobody → hchang
Whiteboard: [domsecurity-backlog1] → [domsecurity-backlog1][domsecurity-active]
|
|
Assignee | |
Comment 3•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/69526/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69526/
Attachment #8778173 -
Flags: review?(ckerschb)
|
|
Assignee | |
Comment 4•9 years ago
|
||
Still requires a test case but just submit a patch for review first.
|
|
Assignee | |
Updated•9 years ago
|
Status: NEW → ASSIGNED
|
|
||
Comment 5•9 years ago
|
||
| mozreview-review | ||
Comment on attachment 8778173 [details]
Bug 1229639 - Part 1: Match CSP host source with percent-decoded URI.
https://reviewboard.mozilla.org/r/69526/#review67610
Thanks for fixing; please provide a testcase before landing;
::: dom/security/nsCSPUtils.cpp:662
(Diff revision 1)
> // http://www.w3.org/TR/CSP11/#source-list-paths-and-redirects
> if (!aWasRedirected && !mPath.IsEmpty()) {
> // converting aUri into nsIURL so we can strip query and ref
> // example.com/test#foo -> example.com/test
> // example.com/test?val=foo -> example.com/test
> nsCOMPtr<nsIURL> url = do_QueryInterface(aUri);
It's unfortunate that nsIURL does not provide this functionality for us. Given that, I think your solution is the best option we have.
Please make sure to remove PercentDecodeStr() from nsCSPParser.h file as well.
Attachment #8778173 -
Flags: review?(ckerschb) → review+
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
||
Comment 8•9 years ago
|
||
| mozreview-review | ||
Comment on attachment 8788406 [details]
Bug 1229639 - Part 2: Test case.
https://reviewboard.mozilla.org/r/76910/#review75088
looks good, r=me.
I suppose you have tested: https://demos.scheurle.info/firefox/csp-url-matching/ - right?
::: dom/security/test/csp/test_bug1229639.html:14
(Diff revision 1)
> +<body>
> +<p id="display"></p>
> +<div id="content" style="display: none">
> +
> +
> +</div>
nit: remove whitespace
::: dom/security/test/csp/test_bug1229639.html:30
(Diff revision 1)
> +
> +examiner.prototype = {
> + observe: function(subject, topic, data) {
> + if (data === 'http://mochi.test:8888/tests/dom/security/test/csp/%24.js') {
> + is(topic, "specialpowers-http-notify-request");
> + SimpleTest.finish();
Please call window.examiner.remove() before SimpleTest.finish();
Attachment #8788406 -
Flags: review?(ckerschb) → review+
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 10•9 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8788406 [details]
Bug 1229639 - Part 2: Test case.
https://reviewboard.mozilla.org/r/76910/#review75088
Thanks for the review :) Besides, I did test https://demos.scheurle.info/firefox/csp-url-matching/ and the image could be loaded successfully.
|
||
Comment 11•9 years ago
|
||
Pushed by hchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e8f963c5afd2
Part 1: Match CSP host source with percent-decoded URI. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/f0df0335a049
Part 2: Test case. r=ckerschb
|
||
Comment 12•9 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/e8f963c5afd2
https://hg.mozilla.org/mozilla-central/rev/f0df0335a049
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•