329746 - Content injection spoofing with a space before colon in HTTP header

Status: RESOLVED FIXED

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

Description

[Kohei Yoshino [:kohei]]

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.

Comment 1

[Kohei Yoshino [:kohei]]

I found Bug 80313. Is this a mistake on the implementation?

Comment 2

[Kohei Yoshino [:kohei]]

Attachment: data flow chart

IPA send us the data flow chart. I attach it.

Comment 3

[Kohei Yoshino [:kohei]]

Attachment: data flow chart 2

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

Comment 4

[Kohei Yoshino [:kohei]]

Nominate for blocking.

Comment 5

[Darin Fisher]

-> me, for investigation

Comment 6

[Kohei Yoshino [:kohei]]

Any updates?

Comment 7

[Darin Fisher]

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.

Comment 8

[Darin Fisher]

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.

Comment 9

[Darin Fisher]

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.

Comment 10

[Darin Fisher]

The patch for this bug will conflict with the patch for bug 328817, so I'm making this bug depend on that bug.

Comment 11

[Darin Fisher]

Attachment: testcase: python script (for first data flow chart)

Comment 12

[Darin Fisher]

Attachment: testcase: python script (for second data flow chart)

Comment 13

[Darin Fisher]

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.

Comment 14

[Darin Fisher]

Attachment: v1 patch

Comment 15

[Darin Fisher]

Safari also implements strict header parsing from what I can tell.

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 217369 [details] [diff] [review]
v1 patch

the table and nsHttp::IsValidToken were just moved without changes, right?

Comment 17

[Darin Fisher]

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

Right.

Comment 18

[Darin Fisher]

fixed-on-trunk

Comment 19

[Darin Fisher]

fixed1.8.1

Comment 20

[Daniel Veditz [:dveditz]]

Comment on attachment 217369 [details] [diff] [review]
v1 patch

approved for 1.8.0 branch, a=dveditz for drivers

Comment 21

[Darin Fisher]

fixed1.8.0.3

Comment 22

[Jay Patel [:jay]]

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.

Comment 23

[Kohei Yoshino [:kohei]]

IPA have confirmed that this vulnerability affects Firefox only.
It's OK to publish security advisory as of Firefox 1.5.0.4. Thanks!

Comment 24

[Kohei Yoshino [:kohei]]

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.

Comment 25

[Alexander Sack]

Attachment: 1.0.x patch

proposed backport