Content injection spoofing with a space before colon in HTTP header

RESOLVED FIXED in mozilla1.8.1

Status

()

Core
Networking: HTTP
P1
critical
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: kohei, Assigned: Darin Fisher)

Tracking

({fixed1.8.1, testcase, verified1.8.0.4})

Trunk
mozilla1.8.1
fixed1.8.1, testcase, verified1.8.0.4
Points:
---
Bug Flags:
blocking1.7.14 ?
blocking-aviary1.0.9 ?
blocking1.9a1 +
blocking1.8.1 +
blocking1.8.0.4 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:high])

Attachments

(6 attachments)

(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: N/A

Note: This bug was originally reported to IPA (Information-technology Promotion Agency, Japan) and forwarded to us at Mozilla Japan. I'm NOT original reporter. Don't mention my name in the security advisory. For more information about IPA, visit http://www.ipa.go.jp/about/english/ 

IPA#62734622

Attackers can forge a Web page on another domain by sending the HTTP response including Transfer-Encoding header with a space between header name and colon (:). This could be used for the real phishing to steal sensitive data, such as user password or cookies. 

Windows XP Professional SP2
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060129 Firefox/1.6a1


Reproducible: Sometimes

Steps to Reproduce:
1. Set up Firefox to use Microsoft Internet Security & Acceleration Server 2004 (ISA Server) for HTTP proxy
2. Provide the proxy with HTTP response with the following conditions:
 A. "Content-Length" header is included
 B. "Transfer-Encoding : chunked" is included (note a space between "Encoding" and ":")
 C. response length with chunk size data of B is shorter than the value of A

Actual Results:  
"Transfer-Encoding SP" header (Transfer-Encoding header with a space) is not defined by HTTP/1.1. Therefore, ISA Server will send data of the number of bytes specified by Content-Length header to the browser. Transfer-Encoding header with a space will be transferred as is. This is appropriate behavior compliant with HTTP/1.1.

Received the response via proxy, Firefox recognizes "Transfer-Encoding SP" header as "Transfer-Encoding" header and understands that the response body is chunked encoded.

The rest of response determined by the condition C is incorrectly recognized as the response of following HTTP request.


Expected Results:  
The rest of response should not be recognized as the response of following HTTP request.

The malicious Web site admins can target Firefox users who use HTT/1.1 proxy to steal cookies or build web pages including fake login form for arbitrary domains.

Workaround: Don't use ISA Server

More info coming.
(Reporter)

Comment 1

12 years ago
I found Bug 80313. Is this a mistake on the implementation?
(Reporter)

Comment 2

12 years ago
Created attachment 214406 [details]
data flow chart

IPA send us the data flow chart. I attach it.
(Reporter)

Comment 3

12 years ago
Created attachment 214408 [details]
data flow chart 2

Here is another chart. 
This bug can also be reproduced with Content-Length header.
(Reporter)

Comment 4

12 years ago
Nominate for blocking.
Flags: blocking1.8.1?
Flags: blocking1.8.0.2?
(Assignee)

Comment 5

11 years ago
-> me, for investigation
Assignee: nobody → darin
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.9a1+
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.3?
Flags: blocking1.8.0.2?
Flags: blocking1.7.14?
Flags: blocking-aviary1.0.9?
(Reporter)

Comment 6

11 years ago
Any updates?
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.8.1
(Assignee)

Comment 7

11 years ago
I don't think spaces around the colon have anything to do with this problem.  Firefox prefers the last Content-Length header over the first, and it will prefer "Tranfer-Encoding: chunked" over a given Content-Length.
(Assignee)

Comment 8

11 years ago
OK, nevermind.  I understand now.  This problem occurs because Firefox tolerates headers that are malformed.  ISA proxy on the other hand, ignores the malformed headers, and therefore this bug gets introduced.  I recall that we added support for sloppy header parsing for consistency with IE because many websites serve malformed headers.  Some, even send '=' instead of ':'.  I'm not sure how to proceed.  If we make Firefox conform strictly to RFC 2616, then we risk breaking web site compatibility.  If we do not, then we risk security.
(Assignee)

Comment 9

11 years ago
Here's the code that Firefox uses to parse response headers:
http://lxr.mozilla.org/mozilla/source/netwerk/protocol/http/src/nsHttpHeaderArray.cpp#131

The comment documents the parsing rules that we implement.
Flags: blocking1.8.0.3? → blocking1.8.0.3+
Whiteboard: [sg:high]
(Assignee)

Comment 10

11 years ago
The patch for this bug will conflict with the patch for bug 328817, so I'm making this bug depend on that bug.
Depends on: 328817
(Assignee)

Comment 11

11 years ago
Created attachment 217360 [details]
testcase: python script (for first data flow chart)
(Assignee)

Comment 12

11 years ago
Created attachment 217361 [details]
testcase: python script (for second data flow chart)
(Assignee)

Comment 13

11 years ago
I have IE, version 6.0.2900.2180.xpsp_sp2_gdr.050301-1519, and it does not appear to be vulnerable to this problem.  It seems like Microsoft must have tightened up the header parsing in IE somewhere along the way, or maybe ruslan was wrong:
https://bugzilla.mozilla.org/show_bug.cgi?id=29219#c14

It seems that the best fix might be to make our header parsing strict again given the ubiquity of IE 6.
(Assignee)

Comment 14

11 years ago
Created attachment 217369 [details] [diff] [review]
v1 patch
Attachment #217369 - Flags: superreview?(bzbarsky)
Attachment #217369 - Flags: review?(cbiesinger)
(Assignee)

Comment 15

11 years ago
Safari also implements strict header parsing from what I can tell.
Attachment #217369 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 217369 [details] [diff] [review]
v1 patch

the table and nsHttp::IsValidToken were just moved without changes, right?
Attachment #217369 - Flags: review?(cbiesinger) → review+
(Assignee)

Comment 17

11 years ago
> the table and nsHttp::IsValidToken were just moved without changes, right?

Right.
(Assignee)

Comment 18

11 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Attachment #217369 - Flags: approval1.8.0.3?
Attachment #217369 - Flags: approval-branch-1.8.1?(bzbarsky)
Attachment #217369 - Flags: approval-branch-1.8.1?(bzbarsky) → approval-branch-1.8.1+
(Assignee)

Comment 19

11 years ago
fixed1.8.1
Keywords: fixed1.8.1
Comment on attachment 217369 [details] [diff] [review]
v1 patch

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #217369 - Flags: approval1.8.0.3? → approval1.8.0.3+
(Assignee)

Comment 21

11 years ago
fixed1.8.0.3
Keywords: fixed1.8.0.3

Comment 22

11 years ago
v.fixed on 1.8.0 branch: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.8.0.2) Gecko/20060420 Firefox/1.5.0.2, no "hacked" popups with either of Darin's testcases, the second headers sent are not processed.
Keywords: fixed1.8.0.3 → verified1.8.0.3

Updated

11 years ago
Keywords: testcase
Whiteboard: [sg:high] → [sg:high] coordinate advisory release with IPA
(Reporter)

Comment 23

11 years ago
IPA have confirmed that this vulnerability affects Firefox only.
It's OK to publish security advisory as of Firefox 1.5.0.4. Thanks!
Whiteboard: [sg:high] coordinate advisory release with IPA → [sg:high]
(Reporter)

Comment 24

11 years ago
IPA told us that the original reporter is Kazuho Oku (Cybozu Labs).
FYI: http://labs.cybozu.co.jp/en/archives/2005-08-16-1.html

dveditz: Please mention his name in the upcoming advisory.
Group: security

Comment 25

11 years ago
Created attachment 225476 [details] [diff] [review]
1.0.x patch

proposed backport

Updated

11 years ago
Attachment #225476 - Flags: review?(caillon)
You need to log in before you can comment on or make changes to this bug.