Closed Bug 1095859 (CVE-2014-8639) Opened 10 years ago Closed 10 years ago

Cookie injection by Proxy with 407 response


(Core :: Networking, defect)

Not set



Tracking Status
firefox33 --- wontfix
firefox34 --- wontfix
firefox35 --- fixed
firefox36 --- fixed
firefox-esr31 35+ fixed
b2g-v1.4 --- wontfix
b2g-v2.0 --- wontfix
b2g-v2.0M --- wontfix
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed


(Reporter: iliwoy, Assigned: mcmanus)



(Keywords: sec-moderate, Whiteboard: [adv-main35+][adv-esr31.4+][b2g-adv-main2.2-])


(1 file)

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
    Chrome actually sends a CONNECT request:
        CONNECT HTTP/1.1
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;; 

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     }
1237     MOZ_ASSERT(!mCachedContentIsValid);
1239     ProcessSSLInformation();
1241     // notify "http-on-examine-response" observers
1242     gHttpHandler->OnExamineResponse(this);
1244     SetCookie(mResponseHead->PeekHeader(nsHttp::Set_Cookie));
If the response dose not contain Proxy-Authenticate header, like this:

    HTTP/1.1 407 Proxy Authentication Required
    Set-Cookie: SID=malicious;;

The cookie will be injected silently (without authentication dialog).
Jason: who should get this bug?
Component: Untriaged → Networking
Ever confirmed: true
Flags: needinfo?(jduell.mcbugs)
Keywords: sec-moderate
Product: Firefox → Core
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.
Flags: needinfo?(jduell.mcbugs)
Attached patch proxy tweakSplinter Review
Assignee: nobody → mcmanus
Attachment #8524233 - Flags: review?(valentin.gosu)
Comment on attachment 8524233 [details] [diff] [review]
proxy tweak

Looks good. Thanks!
Attachment #8524233 - Flags: review?(valentin.gosu) → review+ says to go ahead and land sec-moderate - so i did!
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
Attachment #8524233 - Flags: approval-mozilla-beta?
Attachment #8524233 - Flags: approval-mozilla-aurora?
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-
Attachment #8524233 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Should we take this on ESR?
Flags: needinfo?(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 for more info.
Flags: needinfo?(mcmanus)
Attachment #8524233 - Flags: approval-mozilla-esr31?
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 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.
Attachment #8524233 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8524233 - Flags: approval-mozilla-esr31? → approval-mozilla-esr31+
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?
Closed: 10 years ago
Flags: needinfo?(mcmanus)
Resolution: --- → FIXED
Comment on attachment 8524233 [details] [diff] [review]
proxy tweak

NOTE: Please see to better understand the B2G approval process and landings.

[Approval Request Comment]

see comments 17, 12, 8 for justifications on other branches.
Flags: needinfo?(mcmanus)
Attachment #8524233 - Flags: approval-mozilla-b2g34?
Attachment #8524233 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
Whiteboard: [adv-main35+][adv-esr31.4+]
Alias: CVE-2014-8639
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.
This can be opened based on
Group: core-security
Depends on: 1121895
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.
Did the patch make it into the 2.1 release?
Flags: needinfo?(ryanvm)
Whiteboard: [adv-main35+][adv-esr31.4+] → [adv-main35+][adv-esr31.4+][b2g-adv-main2.2?]
Yes, per comment 19.
Flags: needinfo?(ryanvm)
Whiteboard: [adv-main35+][adv-esr31.4+][b2g-adv-main2.2?] → [adv-main35+][adv-esr31.4+][b2g-adv-main2.2-]
You need to log in before you can comment on or make changes to this bug.