Closed Bug 1122653 Opened 9 years ago Closed 9 years ago

Content-Security-Policy must allow path information, as defined by CSP 1.0 spec

Categories

(Core :: Security, defect)

35 Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1122445

People

(Reporter: ryan, Unassigned)

Details

Attachments

(4 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.95 Safari/537.36

Steps to reproduce:

Prior to FF 35, a CSP directive WITH path information was allowed:

Content-Security-Policy: default-src 'self'; script-src https://example.com/js/

FF 34 and below followed the CSP 1.0 spec of ignoring the "/js/" and treating the CSP as though it had been:

Content-Security-Policy: default-src 'self'; script-src https://example.com


Actual results:

Beginning FF 35, having a CSP header like:

Content-Security-Policy: default-src 'self'; script-src https://example.com/js/

now completely blocks a resource like:

https://example.com/js/myresource.js

as a CSP violiation (which it isn't).


Expected results:

FF should *go back* to following the CSP 1.0 spec of ignoring path information in CSP headers if it is not ready to understand that path information.

See section 5.1 here:

http://www.w3.org/TR/CSP/

specifically Example 4:

"The user agent will ignore the path and act as if the policy contained a script-src directive with value https://example.com."

The purpose of this is to allow backwards compatibility with CSP 2.0
> The purpose of this is to allow backwards compatibility with CSP 2.0

Ryan,

Starting with Firefox 35 we implement the current CSP 1.1 spec draft for this.  See bug 808292 and the draft at <https://w3c.github.io/webappsec/specs/content-security-policy/>.

You're right that your https://example.com/js/ example should not block https://example.com/js/myresource.js.  But your screenshot shows a different situation.  Specifically, the policy in the error message has a path ending in "uy0cakkwr_k/" but the URL that was blocked has "UY0CAKkwR_k/" (and similar case differences for the "AB....QmU" bit).

Is the case difference an issue on our end, or an issue with the policy the page in question is sending?  A link to the page showing the problem would be really useful if that's possible.
Attached image Localhost_Console.png
Attached image Localhost_Inspector.png
Comment on attachment 8550727 [details]
Localhost_Console.png

Reports all lower case in console.
Comment on attachment 8550729 [details]
Localhost_Inspector.png

HTML source in mixed case.
Comment on attachment 8550728 [details]
Localhost_OriginalHeaders.png

Original headers in mixed case to match HTML mixed case.
We are correctly sending mixed case in both our Content-Security-Policy header, and the HTML.  Please find attached clarification:

[ Network ]

1. Localhost_OriginalHeaders.png -- Highlighted correct mixed casing is contained in the original headers.

[ Console ]

2. Localhost_Console.png -- Console contains all lower case.

[ Inspector ]

3. Localhost_Inspector.png -- HTML source in mixed case.

....

Conclusion:

- The server is sending mixed case in both places we expect, response headers (#1) and HTML (#3).  

- "Somewhere" in FF that appears to be getting translated to lower case before showing up in the console (#2), which may be indicative of the underlying problem?
Thank you for checking that!

So looking at nsCSPHostSrc in nsCSPUtils.cpp, it does this:

517 nsCSPHostSrc::appendPath(const nsAString& aPath)
518 {
519   mPath.Append(aPath);
520   ToLowerCase(mPath);

This code dates back to bug 951457.

Then mPath is used in the code added in bug 808292, but that's using case-sensitive comparisons....

That ToLowerCase casll in appendPath just looks wrong to me.  Christoph, is there a reason that's there?  And do we not have tests in bug 808292 with non-lowercase paths?

[Tracking Requested - why for this release]: Can break pages due to incorrect CSP enforcement.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(mozilla)
(In reply to Boris Zbarsky [:bz] from comment #10)
> Thank you for checking that!
> 
> So looking at nsCSPHostSrc in nsCSPUtils.cpp, it does this:
> 
> 517 nsCSPHostSrc::appendPath(const nsAString& aPath)
> 518 {
> 519   mPath.Append(aPath);
> 520   ToLowerCase(mPath);
> 
> This code dates back to bug 951457.
> 
> Then mPath is used in the code added in bug 808292, but that's using
> case-sensitive comparisons....
> 
> That ToLowerCase casll in appendPath just looks wrong to me.  Christoph, is
> there a reason that's there?  And do we not have tests in bug 808292 with
> non-lowercase paths?

Boris, you are absolutely right, that 'ToLowerCase' is wrong. We are about to fix the problem in:
https://bugzilla.mozilla.org/show_bug.cgi?id=1122445#c3

I think we can mark this one as a duplicate.
Flags: needinfo?(mozilla)
Yeah, indeed.  Not sure how I missed that one....
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: