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

Status: NEW

(Core :: DOM: Security, defect, P3)

Description

[Muneaki Nishimura]

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.

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

echo("--boundary\r\n");
echo("Content-type: text/html\r\n");
echo("Content-Security-Policy: default-src 'self'\r\n"); // (2) Ignored (bad.php)
echo("\r\n");
echo("<script>alert(1);</script>\r\n");
echo("--boundary\r\n");
?>


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

[:Gijs (he/him)]

Not sure if the patch in 1295945 fixes this, too. baku?

Comment 2

[Andrea Marchesini [:baku]]

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

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.

Comment 3

[Daniel Veditz [:dveditz]]

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.

Comment 4

[Jason Duell]

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)

   https://groups.google.com/forum/#!searchin/mozilla.dev.platform/Chrome$20removed$20support$20for$20multipart$2Fx-mixed-replace$20documents|sort:relevance/mozilla.dev.platform/iuKw5doD5Ho/kS0iPE5Q5rMJ

  https://bugzilla.mozilla.org/show_bug.cgi?id=843508

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.

Comment 5

[Andrea Marchesini [:baku]]

I think we should fix this issue, or we should drop the support of multipart/x-mixed-replace completely.

Comment 6

[Christoph Kerschbaumer [:ckerschb}]

(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.

Comment 7

[Andrea Marchesini [:baku]]

Christoph, do we have somebody who can work on this?

Comment 8

[Christoph Kerschbaumer [:ckerschb}]

Attachment: bug_1296471_csp_part_multipartchannel.patch

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.