CSP header in a part of multipart HTTP response body is ignored




DOM: Security
2 years ago
2 years ago


(Reporter: Muneaki Nishimura, Unassigned)



Firefox Tracking Flags

(Not tracked)


(Whiteboard: [domsecurity-backlog1])


(1 attachment)



2 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/601.7.7 (KHTML, like Gecko) Version/9.1.2 Safari/601.7.7

Steps to reproduce:

multipart/x-mixed-replace type of HTTP data supports 2 ways of HTTP header setting.
One is to set a header in HTTP header line (1) and another one is in each  part of multipart response body (2).

CSP header set in (1) is properly applied because of Bug 1223743 but CSP in (2) is still ignored.

(1) http://mallory.csrf.jp/x-mixed-replace/csp2/good.php
(2) http://mallory.csrf.jp/x-mixed-replace/csp2/bad.php

Following PHP code can reproduce this issue.

header("Content-Type: multipart/x-mixed-replace;boundary=boundary");
header("Content-Security-Policy: default-src 'self'"); // (1) Not ignored (good.php)

echo("Content-type: text/html\r\n");
echo("Content-Security-Policy: default-src 'self'\r\n"); // (2) Ignored (bad.php)

Actual results:

CSP header in (2) is ignored.

Expected results:

CSP header in (2) should also be applied.
Note that Apple's Safari properly handles CSP in both (1)(2).

Comment 1

2 years ago
Not sure if the patch in 1295945 fixes this, too. baku?
Group: firefox-core-security → core-security
Component: Untriaged → DOM: Security
Flags: needinfo?(amarchesini)
Product: Firefox → Core
See Also: → bug 1295945
Flags: needinfo?(amarchesini)
Flags: needinfo?(jduell.mcbugs)
echo("Content-type: text/html\r\n");
echo("Content-Security-Policy: default-src 'self'\r\n"); // (2) Ignored (bad.php)

This second block is processed as content and it's not processed as HTTP headers. You can see this block in view-source.
The bug is not about how we manage the headers in the document, but how we manage multipart connections.
This doesn't need to be hidden -- better to have site authors aware that our CSP protection is incomplete. This doesn't provide information to an attacker to cause CSP to fail.
Group: core-security
This smells like WONTFIX to me.  1) We're the last browser supporting multipart/x-mixed-replace, which is apparently used by less than 0.00001% of page loads (thought those numbers were gathered by Chrome, and some sites use UA sniffing and deliver x-mixed to firefox only, so take with grain of salt). 2) AFAICT this isn't a sec bug, it just means CSP headers won't work in multipart blocks (which is presumably only used by a fraction of those .000001% of page loads)



If this does needs fixing we'll have to look at how necko provides access to headers that appear in multipart chunks instead of the main body header.  I'm not even sure we have an API for that (IIRC multipart causes multiple sets of OnStart/OnData/OnStop calls, so I'd expect that a header in a sub-part ought to show up in the OnStart for that block) and see if either it's not providing that header, or if the loss is happening in the CORS code somehow.

But let's make sure this actually is worth fixing.
Flags: needinfo?(jduell.mcbugs)
I think we should fix this issue, or we should drop the support of multipart/x-mixed-replace completely.
(In reply to Muneaki Nishimura from comment #0)
> (1) http://mallory.csrf.jp/x-mixed-replace/csp2/good.php
> (2) http://mallory.csrf.jp/x-mixed-replace/csp2/bad.php

FWIW, the first part (1), which works now was fixed within:
> Bug 1223743 - CSP: Check baseChannel for CSP when loading multipart channel

The second part (2) will most likely be fixed within Bug 1295945. We have a testcase for (1)
> dom/security/test/csp/test_multipartchannel.html
but not for (2), potentially it's worth tweaking the test so we have coverage for both scenarios.
Priority: -- → P3
Whiteboard: [domsecurity-backlog1]
Christoph, do we have somebody who can work on this?
Flags: needinfo?(ckerschb)
Created attachment 8783512 [details] [diff] [review]

Baku, here is a mochitest, but as jduell mentions, probably not worth fixing at the moment. I put in the backlog for now. I don't think it's high priority to get this fixed right now. If you think otherwise, please let me know.
Flags: needinfo?(ckerschb)
You need to log in before you can comment on or make changes to this bug.