1724896 - (CVE-2021-29991) Firefox incorrectly interprets newlines in HTTP headers in HTTP/3, allowing for header splitting

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect, P1)

Description

[Neal Poole]

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

Steps to reproduce:

1. Set up a system at https://fb.mvfst.net/15 which speaks HTTP/3, and returns an HTTP/3 header with a newline in it.
2. Browse to it in Firefox with the Network tab open

Actual results:

The page loads properly and the Network tab indicates that Firefox parsed the header with a newline as two separate headers, similar to how it would have been parsed in HTTP/1.1

Expected results:

The page should fail to load, because the QPACK encoding for headers should allow Firefox to recognize that a single header containing a newline is present, and newlines are not allowed in headers.

Comment 1

[Neal Poole]

We originally came across this issue while investigating a report of HTTP header splitting submitted via our bug bounty program. The server in question supports HTTP/1.1, HTTP/2, and HTTP/3. We found the issue only reproduced with HTTP/1.1, and with HTTP/3 in Firefox.

Comment 2

[Daniel Veditz [:dveditz]]

[Tracking Requested - why for this release]:
Response splitting attacks can lead to critical security problems on websites. The sites that are the early adopters for http/3 tend to be the big sites popular with our users.

With a Fx90 build I get http/2 from that server (and no bug); in the just-released Firefox 91 I'm served http/3 and can confirm the problem (also see it in Beta). Tagging as a "regression" in that sense. Oddly, in Nightly I'm seeing http/2 even though all the relevant prefs appear to be the same as in my other test profiles -- will keep poking at that.

Update: When I refresh in Nightly and other builds I do get http/3 and can confirm the bug everywhere. Our first connection to a server doesn't use http/3 because we need to get the Alt-svc header -- I was testing too efficiently trying to hit each relevant version quickly :-)

Comment 3

[Daniel Veditz [:dveditz]]

Removing the regression keyword. Apparently issue on my end: http/3 has been enabled in Release since Fx89.

Comment 4

[Neal Poole]

If you go to https://fb.mvfst.net/ first and then https://fb.mvfst.net/15 it may work better to repro. My understanding is that browsers typically connect to an HTTP/2 or HTTP/1.1 server and "discover" the server supports HTTP/3 through the Alt-Svc header: if you go to /15 directly you may not get anything if the HTTP/2 request fails due to the splitting.

Comment 5

[Dragana Damjanovic [:dragana]]

Attachment: Sanitize headers

Comment 6

[Dragana Damjanovic [:dragana]]

Comment on attachment 9235700 [details]
Sanitize headers

Security Approval Request

• How easily could an exploit be constructed based on the patch?: The fix really show the issue.
  Comment 2 was information about the attacks.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which older supported branches are affected by this flaw?: from 90
• If not all supported branches, which bug introduced the flaw?: Enabling HTTP/3 in 90 release
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: The patch will apply cleanly to 93, 92 and 91
• How likely is this patch to cause regressions; how much testing does it need?: Very unlikely. Our HTTP/2 stack already behaves in the same way as he change in this patch.
  Also he code change is in Rust code which makes it a bit safer.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 9235700 [details]
Sanitize headers

sec-approval=dveditz

Please don't land until the Release Managers are ready to build the point release. The patch makes the problem obvious and we don't want to leave too long a gap between landing and shipping.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 9235700 [details]
Sanitize headers

Beta/Release Uplift Approval Request

• User impact if declined: Users of sites that use http/3 are vulnerable to HTTP response splitting vulnerabilities
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Follow steps in comment 4, then open dettools and looks at the response headers for the /15 document. You should see a single header that looks like

injectheader: Value x: new

rather that two separate headers

injectheader: Value
x: new

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is straightforward rust code to replacing problematic control characters with a space. This will make the http/3 implementation behave like http/2 where this behavior is well-tested.
• String changes made/needed:

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/autoland/rev/b03420dc286b17bd8823d7e86ba5790050bc80e8

Comment 10

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

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

Comment 11

[Neal Poole]

If there is going to be any public attribution of the reporter for the issue, can we use the name Youssef Sammouda? That's the researcher who submitted the report to our bug bounty program that led us to identify the behavior.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9235700 [details]
Sanitize headers

Approved for 92.0b4.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2187c8b0e6f4

Comment 14

[Daniel Cicas [:dcicas], Release QA]

Hello,

I managed to reproduce this issue with Fx 91.0 release.

I can confirm that this issue is fixed on Fx 92.0b4 and Fx 93.0a1 on Win 10, mac OS 11.5 and Ubuntu 18.

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 9235700 [details]
Sanitize headers

approved for 91.0.1 (release + esr)

Comment 16

[Julien Cristau [:jcristau]]

for 91.0.1:
https://hg.mozilla.org/releases/mozilla-release/rev/a2096e046618

for 91.0.1esr (FIREFOX_ESR_91_0_X_RELBRANCH):
https://hg.mozilla.org/releases/mozilla-esr91/rev/4f3e0758a0ced30fa1cf4a4577f74aa71770aae7

for 91.1esr (default):
https://hg.mozilla.org/releases/mozilla-esr91/rev/a5e047f1b996f8caa67a1d5ff2c03e889e9b9538

Comment 17

[Daniel Cicas [:dcicas], Release QA]

Hello,

I verified this fix on Fx 91.0.1 Fx 91.0.1esr and Fx 91.1esr (https://treeherder.mozilla.org/jobs?repo=mozilla-esr91&revision=a5e047f1b996f8caa67a1d5ff2c03e889e9b9538&selectedTaskRun=VK7wIrFeQ_au5V_Sx-T59Q.0) on win 10, mac OS 11 and Ubuntu 18.04.

Comment 18

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

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 19

[Neal Poole]

https://www.mozilla.org/en-US/security/advisories/mfsa2021-37/ mentions my name instead of Youssef. Any chance we can update that?

Comment 20

[Dragana Damjanovic [:dragana]]

Julien, do you know the answer to question in comment 19?

Comment 21

[Julien Cristau [:jcristau]]

Updated: https://github.com/mozilla/foundation-security-advisories/commit/e6fae793fba581f31f5bb8ca7bb3246580c076f5