1864385 - (CVE-2024-1551) Content-Type: multipart/x-mixed-replace allows Set-Cookie in response parts

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect, P2)

Description

[johanaxelcarlsson]

Summary

When Firefox encounters a response with the Content-Type of multipart/x-mixed-replace it will parse the response and update the document based on what is returned inside a given --BOUNDARY. Each response block

--BOUNDARY
Content-Type: text/html

data

It is allowed to set its own Content-Type.

I stumbled upon this Tweet https://x.com/ankursundara/status/1723410507389129092 where someone described an odd behavior in Firefox. The tweet mentions that you can also add Content-Security-Policy to each block, and this policy will be mixed in with the original policy given for the main response (this has some issues of its own but could be argued to work as expected).

I decided to test if any other headers could be altered inside these blocks. I found that contrary to Safari (Chrome handles x-mixed-replace differently), Firefox allows for Set-Cookie headers to be added inside each block, and they will get added to the current domain.

Issue

Any user that can set the Content-Type and control the response body of the request will be able to also set cookies on the domain.

I have found two examples of where this is an issue (just to prove the point):

1. Grafana (the monitoring platform) has a "proxy endpoint" where users can access requests to other services and have the response presented under grafana.example.com. To make this "safe" Grafana is stripping any Set-Cookie headers from the response, and also set a "Content-Security-Policy: sandbox" on the response. This makes the feature safe to use in most browsers. Using the Content-Type: multipart/x-mixed-replace and Set-Cookie trick here will allow to bypass this protection in Firefox.

2. The site https://webhook.site is a testing tool for webhooks. You can create a custom webhook like https://webhook.site/RANDOM where they allow you to configure the Content-Type and the response body of the response from your webhook test site. Even if you are only allowed to control the Content-Type and the body you can use the issue here to bypass this and inject Set-Cookie headers.

I tried to set some additional headers like Alt-Svc in the subparts but this did not work. The ones I got working in each sub block was

Content-Type
Content-Disposition
Content-Security-Policy
Set-Cookie

POC

I used a simple PHP site like this

<?php
header("Content-Type: multipart/x-mixed-replace; boundary=TEST");
?>
--TEST
Set-Cookie: testcookie=testvalue

<h1>Test</h1>
--TEST--

Then visit the site and see the <h1> render. Now look at the request in dev tools. You will see that the original request does not include any Set-Cookie header, but looking at the Storage tab, you can see the cookie has been set.

(this is my POC page https://joaxcar.com/firefox/cookie.php)

/Johan

Comment 1

[Valentin Gosu [:valentin] (he/him)]

We can probably special case Set-Cookie and maybe CSP headers for multipart channels.
Then we need a plan for bug 1276918. Or to align with Chrome&Safari with WPT tests.

Comment 2

[Daniel Veditz [:dveditz]]

Any user that can set the Content-Type and control the response body of the request [...]

That sounds terrifying enough right there! I can't believe sites have set things up to allow that on purpose

Is it defined what is supposed to happen with x/multipart-replace ? Assuming a web server is in control of the responses it generates, the ability to set cookies from an HTML content part could be expected and useful. And yes, the reporter has shown that's not a good assumption in at least those cases. Is the expected behavior defined enough to call this a Firefox security bug?

Rather than special-casing cookies or other features, we should figure out if this is a feature we support or not. If we support it then get its behavior standardized. If not, kill it. Do we have any telemetry on how much it's used for html parts?

Comment 3

[Valentin Gosu [:valentin] (he/him)]

I see the plan is to disable it for document loads in bug 1276918

Comment 4

[Frederik Braun [:freddy]]

Cookies can still be set on non-document loads, no?

Comment 5

[edgul]

Attachment: Bug 1864385 - Block set-cookie from multipart/x-mixed-replace with pref. r?valentin

Comment 6

[edgul]

Attachment: Bug 1864385 - Added test for set-cookie blocking from multipart/x-mixed-replace r?valentin

Depends on D198197

Comment 7

[edgul]

Attachment: Bug 1864385 - Block csp setting from multipart/x-mixed-replace r?valentin

Depends on D198197

Comment 8

[edgul]

I'm a bit confused. Do we really want to disable CSP setting from multipart/x-mixed-replace? It looks like this exact "feature" was added for sec reasons for bug 1416045.

Valentin? Christoph?

Comment 9

[Frederik Braun [:freddy]]

Mhh, I don't think so. It would be better if we could have each response (each part of the multi parts) append to the CSP instead of overwriting it. We have CSP_AppendCSPFromHeader() in nsCSPUtils.cpp for that. See also bug 1864434.

Comment 10

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Frederik Braun [:freddy] from comment #9)

Mhh, I don't think so. It would be better if we could have each response (each part of the multi parts) append to the CSP instead of overwriting it. We have CSP_AppendCSPFromHeader() in nsCSPUtils.cpp for that. See also bug 1864434.

I just discussed this with Freddy, we should not ignore the CSP, because we explicitly added that behavior within bug 1223743. However, I think we should rather append and merge CSPs as Freddy pointed out.

Comment 11

[Valentin Gosu [:valentin] (he/him)]

Thanks for letting us know. In that case let's keep tracking the CSP issues in bug 1864434, and we can move forward with disabling Set-Cookie for multipart.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/920b05879c35
Block set-cookie from multipart/x-mixed-replace with pref. r=valentin,necko-reviewers

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for causing build bustage in nsMultiMixedConv.cpp:
https://hg.mozilla.org/integration/autoland/rev/415e31510249f154bcd66c06a314a94e3cab50dc

Push with failures
Failure log

[task 2024-01-16T18:43:15.031Z] 18:43:15    ERROR -  /builds/worker/checkouts/gecko/netwerk/streamconv/converters/nsMultiMixedConv.cpp(987,25): error: no member named 'network_cookie_prevent_set_cookie_from_multipart' in namespace 'mozilla::StaticPrefs'
[task 2024-01-16T18:43:15.032Z] 18:43:15     INFO -    987 |       if (!StaticPrefs::network_cookie_prevent_set_cookie_from_multipart() &&
[task 2024-01-16T18:43:15.032Z] 18:43:15     INFO -        |            ~~~~~~~~~~~~~^

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ace77950984d
Block set-cookie from multipart/x-mixed-replace with pref. r=valentin,necko-reviewers

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ace77950984d

Comment 16

[Daniel Bodea [:danibodea]]

I would like to reproduce and verify this fix, but I lack a proper understanding of the issue in question. Can you help us (QA) by giving us a set of steps to reproduce and actual and expected results so that we can correctly validate this fix? Thank you!

Comment 17

[edgul]

Simply visit the site mentioned in Comment 0, https://joaxcar.com/firefox/cookie.php, which appears to still be hosting the file as described.
Then view your cookies for the site (shift + F9). This should open devtools to storage with cookies for the site selected in the left panel.
There should be no cookies in the main panel.

Comment 18

[Daniel Bodea [:danibodea]]

Thank you for the clarification!
I have reproduced this issue in Release v122.00 and verified the fix in Beta v123.0b6 and Nightly v124.0a1 in Windows10 and MacOS11.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Does this need an ESR uplift also? It grafts cleanly. Please nominate ASAP if yes.

Comment 20

[edgul]

Comment on attachment 9372086 [details]
Bug 1864385 - Block set-cookie from multipart/x-mixed-replace with pref. r?valentin

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Comment 0 outlines this pretty well. It shouldn't be possible to set cookies from the multipart section.
• User impact if declined: The vulnerability will continue to exist.
• Fix Landed on Version: Nightly and Beta
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Seems to work on Beta and Nightly. Not a common use case and really shouldn't be used.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9372086 [details]
Bug 1864385 - Block set-cookie from multipart/x-mixed-replace with pref. r?valentin

Approved for 115.8esr.

Comment 22

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/ec81a0da67e2

Comment 23

[Daniel Bodea [:danibodea]]

This fix has also been verified in ESR v115.8.0esr on Windows 10 and MacOS 11. No cookie can not be found in the Storage section of the Developer Tools.

Comment 24

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 25

[BugBot [:suhaib / :marco/ :calixte]]

a month ago, edgul placed a reminder on the bug using the whiteboard tag [reminder-test 2024-02-17] .

edgul, please refer to the original comment to better understand the reason for the reminder.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/709337e620c7
Added test for set-cookie blocking from multipart/x-mixed-replace r=valentin,necko-reviewers

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/709337e620c7