376756 - (PR07-01) When there are duplicate HTTP headers, the first one should win

Status: RESOLVED DUPLICATE of bug 655389

(Core :: Networking: HTTP, defect)

Description

[Daniel Veditz [:dveditz]]

CPNI (formerly NISCC) sends us this report:

PR07-01: Mozilla Firefox duplicate header overwrite bug (draft)

Description:

Whenever a web server responds to a HTTP request with duplicate headers (i.e.: CRLF injection attack), Firefox incorrectly ignores the first header and considers the last instance of a duplicate header, rather than responding to the original HTTP header only.

For instance, let's suppose the target web application is vulnerable to CRLF injection:

HTTP request:

GET /homedir/%0aLocation:%20http://evil.tld HTTP/1.0
Connection: close

HTTP response:

HTTP/1.1 302 Moved Temporarily
Server: Apache/1.3.31 (Unix)
Location: /homedir/
Location: http://evil.tld
Connection: close
Content-Type: text/html


Notice the duplicate 'Location:' header. Instead of only considering the first and original header, Firefox takes into account the last header.

From the security point of view, this can represent an issue in situations in which web applications are vulnerable to CRLF injection since headers such "Location:" and "Set-cookie:" can be overwritten without needing to perform a reponse splitting attack [http://www.packetstormsecurity.org/papers/general/whitepaper_httpresponse.pdf].


The following steps have been provided to replicate this issue:

1. We make our own test web server with netcat running the shell script "run-server.sh" (netcat [http://netcat.sourceforge.net/] is required for the script to work)
2. We then point Mozilla Firefox to the host running "run-server.sh" on port 55555. I.e.: http://192.168.1.10:55555/
3. Firefox will then ignore the first "Location:" header and take into account the second one instead. The result is that we are redirected to www.procheckup.com (second "Location:" header) instead of www.google.com (first "Location:" header). The following is the HTTP response returned by the test server when connecting to it:

	HTTP/1.1 302 Found
	Server: Test-server/1.0
	Location: http://www.google.com/
	Location: http://www.procheckup.com/
	Connection: close
	Content-Length: 0


Notes:

- This header overwrite behaviour was tested successfully on Mozilla Firefox 2.0.0.2 under Windows XP SP2.
- The following web browsers *don't* seeem to suffer from this header overwrite behaviour (they only take into account the first instance of a HTTP header): Internet Explorer 7.0.5730.11 (Windows XP SP2), Opera 9.02 (Windows XP SP2)

Comment 1

[Daniel Veditz [:dveditz]]

Our behavior for Set-Cookie is correct, there can be multiple ones of those. For headers that are not allowed to have multiple settings the reporter is correct that we always take the last presented value. This appears quite deliberate, it would be very easy to drop the 'else' clause and never set these headers more than once.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/protocol/http/src/nsHttpHeaderArray.cpp&rev=1.16&mark=87-89#65

Why do we do it this way? The spec is silent on the issue but the IE and Opera behavior seems to make sense.

It should be noted that we're not trying to protect against a malicious server--if the server wants to lie it will just lie--but against a broken server that's not correctly handling a request.

Comment 2

[Jesse Ruderman]

I'm not sure I understand why letting the first header win is better than letting the last header win.  If the server sends the line-break-injectable header last, letting the first header win is better.  But if the server sends the injectable header first, letting the last header win is better.

Can we just reject the page entirely if it contains bogusly duplicated, disagreeing headers?

Comment 3

[Boris Zbarsky [:bzbarsky]]

> Can we just reject the page entirely

I thought duplicated Content-Type headers were somewhat common -- once from the dumb server side app framework and once from the actual CGI script.  I would be interested in what IE's and Opera's behavior is with duplicated Content-Type.  Do they still use the first one they see?  Or the last?

Comment 4

[Jason Duell]

I'm adding this to my list of likely network bugs to work on.  No guarantee
when I'll actually get to it.

Comment 5

[Anne (:annevk)]

I know we changed some of this, e.g. duplicate Location headers in the context of a redirect is a network error (unless their values happen to match).

Is this documented somewhere? We should really start to coordinate the hairy details of Necko with other implementers, perhaps in another venue then ietf-http-wg if they are hostile towards it.

Comment 6

[Mark Nottingham]

We'll be talking about the browser profile doc in Toronto.

Comment 7

[Jason Duell]

This bug was fixed when we we started differentiating between SetHeader() (from Addons and internal code: the last caller wins) and SetHeaderFromNet (when we get HTTP response headers)

  http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHeaderArray.cpp#55

SetHeaderFromNet uses lists of well-known 'singleton' (can't merge) and 'suspectDuplicate' (error if we see more than a single instance: includes Location and Content-Length and Content-Disposition).  If a header can't be merged and it's not a suspectDuplicate, we ignore later instances (so the 1st wins).

I'd welcome any feedback (in a new bug) if our list of singletons/suspectDuplicate headers needs any tweaking:

  http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHeaderArray.h?rev=aceb8d1e6eba#120

  http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHeaderArray.h?rev=aceb8d1e6eba#169