1095859 - (CVE-2014-8639) Cookie injection by Proxy with 407 response

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[iliwoy]

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

Steps to reproduce:

1. Set up a HTTP proxy in Firefox.
2. Navigate to https://mail.google.com.
    Chrome actually sends a CONNECT request:
        CONNECT mail.google.com:443 HTTP/1.1
	Host: mail.google.com
3. The proxy responds a 407 message, including Set-Cookie headers.
    HTTP/1.1 407 Proxy Authentication Required
    Proxy-Authenticate: Basic realm=”auth"
    Set-Cookie: SID=malicious; domain=.mail.google.com; 


Actual results:

Firefox mistakenly accepts the malicious cookies.


Expected results:

Firefox should ignore such Set-Cookie headers.

This bug occurs in ./netwerk/protocol/http/nsHttpChannel.cpp, line 1244.

1225     if (mTransaction->ProxyConnectFailed()) {
1226         // Only allow 407 (authentication required) to continue
1227         if (httpStatus != 407)
1228             return ProcessFailedProxyConnect(httpStatus);
1229         // If proxy CONNECT response needs to complete, wait to process connection
1230         // for Strict-Transport-Security.
1231     } else {
1232         // Given a successful connection, process any STS data that's relevant.
1233         rv = ProcessSTSHeader();
1234         MOZ_ASSERT(NS_SUCCEEDED(rv), "ProcessSTSHeader failed, continuing load.");
1235     }
1236 
1237     MOZ_ASSERT(!mCachedContentIsValid);
1238 
1239     ProcessSSLInformation();
1240 
1241     // notify "http-on-examine-response" observers
1242     gHttpHandler->OnExamineResponse(this);
1243 
1244     SetCookie(mResponseHead->PeekHeader(nsHttp::Set_Cookie));

Comment 1

[iliwoy]

If the response dose not contain Proxy-Authenticate header, like this:

    HTTP/1.1 407 Proxy Authentication Required
    Set-Cookie: SID=malicious; domain=.mail.google.com;

The cookie will be injected silently (without authentication dialog).

Comment 2

[Daniel Veditz [:dveditz]]

Jason: who should get this bug?

Comment 3

[Patrick McManus [:mcmanus]]

great bug. thanks.

I'll patch this up - processAltSvc should be similarly restricted - like the logic that protects ProcessSecurityHeaders() except they need to continue to happen after OnExamineResponse.

Comment 4

[Patrick McManus [:mcmanus]]

Attachment: proxy tweak

Comment 5

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

Comment on attachment 8524233 [details] [diff] [review]
proxy tweak

Looks good. Thanks!

Comment 6

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d0b45d3dc7d6

Comment 7

[Patrick McManus [:mcmanus]]

https://wiki.mozilla.org/Security/Bug_Approval_Process says to go ahead and land sec-moderate - so i did!

Comment 8

[Patrick McManus [:mcmanus]]

Comment on attachment 8524233 [details] [diff] [review]
proxy tweak


Approval Request Comment
[Feature/regressing bug #]: Firefox 1.0 :)
[User impact if declined]: cookie injection attack against https sites (write only - which is a mitigation) with active network attacker.
[Describe test coverage new/current, TBPL]: new xpcshell tbpl coverage
[Risks and why]: modest - the gecko change is just a few lines, making old cookie behavior conditional
[String/UUID change made/needed]: none

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 10

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8524233 [details] [diff] [review]
proxy tweak

Seeing as we've shipped this forever, this is sec-moderate, and we're down to the last 34 Beta, I think we can wait a few more weeks. Let's consider this fix for 35. Beta-

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

Should we take this on ESR?

Comment 12

[Patrick McManus [:mcmanus]]

Comment on attachment 8524233 [details] [diff] [review]
proxy tweak

yes - we should take it on esr. I overlooked that, thanks

[Approval Request Comment] -
If this is not a sec:{high,crit} bug, please state case for ESR consideration: see above
User impact if declined: see above
Fix Landed on Version:36
Risk to taking this patch (and alternatives if risky): see above
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 13

[Lawrence Mandel [:lmandel] (use needinfo)]

As I don't think we need to take this in 34, I suggest that we take the fix in Firefox 35 and ESR 31.4.

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8524233 [details] [diff] [review]
proxy tweak

Approving for Aurora, ESR approval will have to wait until after we've got 31.3 esr underway.

Comment 15

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9614d1f5e967

Comment 16

[Patrick McManus [:mcmanus]]

 https://hg.mozilla.org/releases/mozilla-esr31/rev/a1c4c727b1f4

Comment 17

[Ryan VanderMeulen [:RyanVM]]

sec-moderates generally aren't being taken for B2G v2.0 without good reason at this point, but can you please request b2g34 approval for v2.1 still?

Comment 18

[Patrick McManus [:mcmanus]]

Comment on attachment 8524233 [details] [diff] [review]
proxy tweak

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]

see comments 17, 12, 8 for justifications on other branches.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/0b8e4d5f6714

Comment 20

[:Gijs (he/him)]

People are reporting that ff35 breaks setups using proxies for authentication (bug 1121895), and, based on having this in the release notes, suspect this broke it.

Comment 21

[Patrick McManus [:mcmanus]]

This can be opened based on 
https://www.mozilla.org/de/security/advisories/mfsa2015-04/

Comment 22

[Al Billings [:abillings - ex-MoCo]]

Patrick, please don't open bugs without checking with me and Dan. Once we release an advisory and fix for an issue, we generally wait at least six weeks to open the bug barring an immediate need.

Comment 23

[Christiane Ruetten [INACTIVE]]

Did the patch make it into the 2.1 release?

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Yes, per comment 19.