Last Comment Bug 329746 - Content injection spoofing with a space before colon in HTTP header
: Content injection spoofing with a space before colon in HTTP header
Status: RESOLVED FIXED
[sg:high]
: fixed1.8.1, testcase, verified1.8.0.4
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: All All
: P1 critical (vote)
: mozilla1.8.1
Assigned To: Darin Fisher
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on: 328817
Blocks:
  Show dependency treegraph
 
Reported: 2006-03-08 01:19 PST by Kohei Yoshino [:kohei]
Modified: 2006-06-13 15:23 PDT (History)
4 users (show)
dveditz: blocking1.7.14?
dveditz: blocking‑aviary1.0.9?
dveditz: blocking1.9a1+
dveditz: blocking1.8.1+
dveditz: blocking1.8.0.4+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
data flow chart (75.70 KB, image/png)
2006-03-08 01:26 PST, Kohei Yoshino [:kohei]
no flags Details
data flow chart 2 (74.88 KB, image/png)
2006-03-08 01:36 PST, Kohei Yoshino [:kohei]
no flags Details
testcase: python script (for first data flow chart) (725 bytes, text/plain)
2006-04-05 17:49 PDT, Darin Fisher
no flags Details
testcase: python script (for second data flow chart) (655 bytes, text/plain)
2006-04-05 17:49 PDT, Darin Fisher
no flags Details
v1 patch (10.43 KB, patch)
2006-04-05 18:55 PDT, Darin Fisher
cbiesinger: review+
bzbarsky: superreview+
bzbarsky: approval‑branch‑1.8.1+
dveditz: approval1.8.0.4+
Details | Diff | Splinter Review
1.0.x patch (9.44 KB, patch)
2006-06-13 14:46 PDT, Alexander Sack
asac: review? (caillon)
Details | Diff | Splinter Review

Description Kohei Yoshino [:kohei] 2006-03-08 01:19:04 PST
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] 2006-03-08 01:24:12 PST
I found Bug 80313. Is this a mistake on the implementation?
Comment 2 Kohei Yoshino [:kohei] 2006-03-08 01:26:36 PST
Created attachment 214406 [details]
data flow chart

IPA send us the data flow chart. I attach it.
Comment 3 Kohei Yoshino [:kohei] 2006-03-08 01:36:30 PST
Created attachment 214408 [details]
data flow chart 2

Here is another chart. 
This bug can also be reproduced with Content-Length header.
Comment 4 Kohei Yoshino [:kohei] 2006-03-08 01:39:57 PST
Nominate for blocking.
Comment 5 Darin Fisher 2006-03-08 06:32:51 PST
-> me, for investigation
Comment 6 Kohei Yoshino [:kohei] 2006-03-19 08:41:06 PST
Any updates?
Comment 7 Darin Fisher 2006-03-30 15:08:53 PST
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 2006-03-30 15:12:49 PST
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 2006-03-30 15:15:47 PST
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 2006-04-05 17:44:25 PDT
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 2006-04-05 17:49:13 PDT
Created attachment 217360 [details]
testcase: python script (for first data flow chart)
Comment 12 Darin Fisher 2006-04-05 17:49:42 PDT
Created attachment 217361 [details]
testcase: python script (for second data flow chart)
Comment 13 Darin Fisher 2006-04-05 18:20:10 PDT
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 2006-04-05 18:55:40 PDT
Created attachment 217369 [details] [diff] [review]
v1 patch
Comment 15 Darin Fisher 2006-04-05 18:58:19 PDT
Safari also implements strict header parsing from what I can tell.
Comment 16 Christian :Biesinger (don't email me, ping me on IRC) 2006-04-07 10:38:38 PDT
Comment on attachment 217369 [details] [diff] [review]
v1 patch

the table and nsHttp::IsValidToken were just moved without changes, right?
Comment 17 Darin Fisher 2006-04-07 10:42:01 PDT
> the table and nsHttp::IsValidToken were just moved without changes, right?

Right.
Comment 18 Darin Fisher 2006-04-07 10:52:54 PDT
fixed-on-trunk
Comment 19 Darin Fisher 2006-04-07 17:35:30 PDT
fixed1.8.1
Comment 20 Daniel Veditz [:dveditz] 2006-04-17 12:03:49 PDT
Comment on attachment 217369 [details] [diff] [review]
v1 patch

approved for 1.8.0 branch, a=dveditz for drivers
Comment 21 Darin Fisher 2006-04-17 13:53:56 PDT
fixed1.8.0.3
Comment 22 Jay Patel [:jay] 2006-04-20 15:43:21 PDT
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] 2006-05-09 22:15:24 PDT
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] 2006-05-24 22:29:36 PDT
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 2006-06-13 14:46:53 PDT
Created attachment 225476 [details] [diff] [review]
1.0.x patch

proposed backport

Note You need to log in before you can comment on or make changes to this bug.