Last Comment Bug 694576 - (CVE-2012-0475) Ambiguous IPv6 in Origin (and Sec-WebSocket-Origin) request header
(CVE-2012-0475)
: Ambiguous IPv6 in Origin (and Sec-WebSocket-Origin) request header
Status: VERIFIED FIXED
[sg:moderate][qa-]
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: 9 Branch
: x86_64 Windows 7
: -- normal (vote)
: mozilla12
Assigned To: Patrick McManus [:mcmanus]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-14 08:57 PDT by Mone
Modified: 2015-08-09 14:40 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
affected
-
wontfix
-
wontfix
verified
wontfix
unaffected


Attachments
patch 0 (4.85 KB, patch)
2011-12-22 04:52 PST, Patrick McManus [:mcmanus]
bzbarsky: review-
Details | Diff | Review
patch 1 (4.76 KB, patch)
2011-12-22 09:11 PST, Patrick McManus [:mcmanus]
bzbarsky: review+
Details | Diff | Review

Description Mone 2011-10-14 08:57:41 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/535.1 (KHTML, like Gecko) Chrome/14.0.835.187 Safari/535.1

Steps to reproduce:

having a html page on a web server listening on a non-80 port, and accessing such page specifiyng as host the IPv6 of such webserver (obviously plus its port...):
If you open a cross-site XHR or WebSocket it will result in the browser sending an ambiguous Origin (or Sec-WebSocket-Origin) header if the said IPv6 contains at least 2 consecutive 16-bit fields of 0s. 

I think that at least at the moment this ambiguity does not pose security issues as I expect that most resources will not have a Origin permission list based on IPv6, but I'm flagging the hidden check anyway (better safe than sorry...)

I've seen the same behaviour on Firefox 7.0.1 and Aurora 9.0a2 



related specs/drafts:
CORS http://www.w3.org/TR/cors
WebSockets http://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-17
IPv6 http://tools.ietf.org/html/rfc5952
Origin http://tools.ietf.org/html/draft-ietf-websec-origin-06 & http://tools.ietf.org/html/draft-abarth-origin-09


Actual results:

Let's say that the web server is listening on port 8888 and can be reached with its IPv6 that is aa80:0000:0000:0000:19b7:231e:ff5d:34e3 (that compressed is aa80::19b7:231e:ff5d:34e3).
The Origin header generated by Firefox will be like this:

http://aa80::19b7:231e:ff5d:34e3:8888



Now let's say that the web server is listening on port 80 and can be reached with its IPv6 that is aa80:0000:0000:19b7:231e:ff5d:34e3:8888 (that compressed is aa80::19b7:231e:ff5d:34e3:8888). 
The Origin header generated by Firefox will be the same:

http://aa80::19b7:231e:ff5d:34e3:8888



Expected results:

The ambiguity is easily solved using square brakets so that the 2 Origins can be

http://[aa80::19b7:231e:ff5d:34e3]:8888

and 

http://[aa80::19b7:231e:ff5d:34e3:8888]


(at least Chrome behave like this)
Comment 1 Johnny Stenback (:jst, jst@mozilla.com) 2011-11-03 14:02:55 PDT
Patrick, any thoughts on security severity here?
Comment 2 Patrick McManus [:mcmanus] 2011-11-04 05:34:50 PDT
(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #1)
> Patrick, any thoughts on security severity here?

there are a couple things about the report I want to verify, but at first pass its sg:moderate.

can you change the component to core:networking:websockets ? (If I try and do that it tells me I can't set the sec group and threatens to drop the current sec classification)
Comment 3 Patrick McManus [:mcmanus] 2011-11-04 05:38:31 PDT
ah, I just realized the report says all Origin headers - not just WS. That makes more sense to me (and is something I wanted to verify). so the component should be http not websockets.

that comes from 

  nsCString asciiOrigin;
  rv = nsContentUtils::GetASCIIOrigin(mPrincipal, asciiOrigin);
  NS_ENSURE_SUCCESS(rv, rv);

maybe bz has some insight into whether [] is the correct format..
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-11-04 07:42:40 PDT
Spec is at http://tools.ietf.org/html/draft-abarth-origin-09#section-6.1 and says:

 serialized-origin   = scheme "://" host [ ":" port ]
                     ; <scheme>, <host>, <port> productions from RFC3986

So yes, the [] would be correct, I believe.

Is that something that nsStandardURL::GetHost should handle, or do we need to somehow figure out in nsContentUtils that the host is an IPv6 literal?  Seems like the former would be better....
Comment 5 Johnny Stenback (:jst, jst@mozilla.com) 2011-11-04 07:52:05 PDT
Thanks Patrick!
Comment 6 Patrick McManus [:mcmanus] 2011-11-07 09:05:45 PST
http://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsStandardURL.cpp#419(In reply to Boris Zbarsky (:bz) from comment #4)

> Is that something that nsStandardURL::GetHost should handle, or do we need

hmm.. it looks like sethost was already trying to do it.. 

http://mxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsStandardURL.cpp#419
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-11-07 09:09:05 PST
Yes, but GetHost explicitly strips out the [] right now.  See nsStandardURL::Host....
Comment 8 Josh Aas 2011-11-11 11:32:04 PST
BZ or Patrick - either of you want to own this?
Comment 9 Christian :Biesinger (don't email me, ping me on IRC) 2011-11-11 16:40:09 PST
Compare code at http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#548
Comment 10 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-11-13 01:09:39 PST
(In reply to Josh Aas (Mozilla Corporation) from comment #8)
> BZ or Patrick - either of you want to own this?

I don't, particularly.

Christian, would it make sense for GetHost to just always do that?  If not, we need to make the content utils code do something like this (e.g. by moving the code you linked to to nsNetUtils).
Comment 11 Patrick McManus [:mcmanus] 2011-12-01 14:26:03 PST
I guess I see 4 paths, all kinda undesirable.

1] nsStandardURL:Host (used by getAsciiHost) has stripped [] back to hg rev 1. There are obviously lots of callers; I'm not eager to change that behavior - regressions would be likely.

2] likewise, nsIURI has a bunch of downstream uses so adding a getAsciiHostPort to that which does the right thing doesn't seem real desirable for anyone that wants to implement nsIURI.

3] I guess I could add getAsciiHostPort() to nsIStandardURL (or better a whole new IDL that nsStandardURL also implements) and contentutils could QI that and use the existing code as a backup. Seems kinda expensive.

4] Finally, contentutils is already doing the concat so it really ought to absorb responsibility for the semantics (unless you argue that new methods/interfaces are needed because it shouldn't be in that business at all) much like nsHttpHandler::GenerateHostPort() does - so similar logic as seen in generatehostport can just be applied in contentutils when doing the port concat.

I favor 4 as simple and efficient, though it muddles ownership of these things further. I guess failing that I favor a version of 3 with a new IDL and a QI. I'm happy to write the code for either path.

Boris what do you think?
Comment 12 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-01 14:53:38 PST
The problem with #4 is that then ContentUtils needs methods to detect when the hostname is actually an IP address, right?  I guess you're suggesting that it just look for ':' in the hostname?  For that matter, would it need to look for '%' that way as well?

I have no a priori problem with contentutils doing something like that, as long as the code only lives in one shared place.  I don't want multiple versions of code like that around....  So contentutils would either pass the host string or the nsIURI to some nsNetUtil method and out would pop the right thing.

Whether 2 is better than 4...  We should look at our other GetAsciiHost callers and see how many of them are actually implementing GetAsciiHostPort, I think.

As far as 2 vs 3, 3 seems strictly worse since it'll give wrong behavior for some URIs, right?
Comment 13 Patrick McManus [:mcmanus] 2011-12-22 04:52:34 PST
Created attachment 583760 [details] [diff] [review]
patch 0

The code to do host/port concatenation was duplicated in nsContentUtils and nsHttpHandler, with only the latter handling the [] ipv6 literal case.

This patch moves the httphandler algorithm into nsNetUtils and changes nsContentUtils and nsHttpHandler to both point at that.
Comment 14 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-22 08:44:26 PST
Comment on attachment 583760 [details] [diff] [review]
patch 0

This doesn't do the right thing when the port is set in the URI and set to the default port, does it?  You need to keep those compares to the default port and reset the port to -1 if it's the default, I think.
Comment 15 Patrick McManus [:mcmanus] 2011-12-22 08:54:10 PST
my bad - I misread that as doing something else.

easy to fixup - thanks.
Comment 16 Patrick McManus [:mcmanus] 2011-12-22 09:11:15 PST
Created attachment 583824 [details] [diff] [review]
patch 1
Comment 17 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-12-22 09:14:13 PST
Comment on attachment 583824 [details] [diff] [review]
patch 1

r=me.  Thanks!
Comment 18 Patrick McManus [:mcmanus] 2011-12-22 18:24:40 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/c02f25c177e3
Comment 19 Ed Morley [:emorley] 2011-12-23 19:58:20 PST
https://hg.mozilla.org/mozilla-central/rev/c02f25c177e3

Patrick, I've set the affected/tracking flags, but I'll let you decide whether this wants beta/aurora approval requested if that's ok?
Comment 20 Patrick McManus [:mcmanus] 2011-12-25 06:07:13 PST
I'm not going to a? though someone from sec-team can if they disagree.

to exploit you need to be a server with an origin ACL list using IPV6 literals of a particular pattern (and a particular matching pattern of the port you are running your server on - this is not in the attackers control). There is no evidence that has happened in the wild.

so I'd rather the change get the chance to bake in aurora/beta in the usual way.
Comment 21 Alex Keybl [:akeybl] 2012-01-03 12:48:36 PST
(In reply to Patrick McManus [:mcmanus] from comment #20)
> I'm not going to a? though someone from sec-team can if they disagree.
> 
> to exploit you need to be a server with an origin ACL list using IPV6
> literals of a particular pattern (and a particular matching pattern of the
> port you are running your server on - this is not in the attackers control).
> There is no evidence that has happened in the wild.
> 
> so I'd rather the change get the chance to bake in aurora/beta in the usual
> way.

Given our intent to let this ride, minusing for tracking.
Comment 22 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-29 15:36:06 PDT
Mone, would you be able to verify this fixed in Firefox 12.0b3?
Comment 23 Mone 2012-03-30 01:02:25 PDT
Hi All,

Yes I confirm that the issue is solved on Firefox 12, at least since a2 (2012-03-13), thanks everybody :)
Comment 24 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-30 10:33:14 PDT
Marking verified based on comment 23.

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