The default bug view has changed. See this FAQ.
Bug 694576 (CVE-2012-0475)

Ambiguous IPv6 in Origin (and Sec-WebSocket-Origin) request header

VERIFIED FIXED in Firefox 12

Status

()

Core
Networking: HTTP
VERIFIED FIXED
6 years ago
2 years ago

People

(Reporter: Mone, Assigned: mcmanus)

Tracking

9 Branch
mozilla12
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(firefox9- affected, firefox10- wontfix, firefox11- wontfix, firefox12 verified, firefox-esr10 wontfix, status1.9.2 unaffected)

Details

(Whiteboard: [sg:moderate][qa-])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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)
Patrick, any thoughts on security severity here?
Assignee: nobody → nobody

Updated

6 years ago
Assignee: nobody → nobody
(Assignee)

Comment 2

6 years ago
(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)
Whiteboard: sg:moderate
(Assignee)

Comment 3

6 years ago
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..
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....
Thanks Patrick!
Component: General → Networking: HTTP
Product: Firefox → Core
QA Contact: general → networking.http
Whiteboard: sg:moderate → [sg:moderate]
(Assignee)

Comment 6

6 years ago
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
Yes, but GetHost explicitly strips out the [] right now.  See nsStandardURL::Host....

Comment 8

5 years ago
BZ or Patrick - either of you want to own this?
Compare code at http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#548
(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).

Updated

5 years ago
Assignee: nobody → mcmanus
(Assignee)

Comment 11

5 years ago
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?
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?
(Assignee)

Comment 13

5 years ago
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.
Attachment #583760 - Flags: review?(bzbarsky)
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.
Attachment #583760 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 15

5 years ago
my bad - I misread that as doing something else.

easy to fixup - thanks.
(Assignee)

Comment 16

5 years ago
Created attachment 583824 [details] [diff] [review]
patch 1
Attachment #583760 - Attachment is obsolete: true
Attachment #583824 - Flags: review?(bzbarsky)
Comment on attachment 583824 [details] [diff] [review]
patch 1

r=me.  Thanks!
Attachment #583824 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 18

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c02f25c177e3

Updated

5 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
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?
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox10: --- → affected
status-firefox11: --- → affected
status-firefox12: --- → fixed
status-firefox9: --- → affected
tracking-firefox10: --- → ?
tracking-firefox11: --- → ?
tracking-firefox9: --- → ?
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
(Assignee)

Comment 20

5 years ago
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.
(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.
tracking-firefox10: ? → -
tracking-firefox11: ? → -
tracking-firefox9: ? → -
status1.9.2: --- → unaffected
status-firefox-esr10: --- → wontfix
status-firefox10: affected → wontfix
status-firefox11: affected → wontfix
Mone, would you be able to verify this fixed in Firefox 12.0b3?
Whiteboard: [sg:moderate] → [sg:moderate][qa-]
(Reporter)

Comment 23

5 years ago
Hi All,

Yes I confirm that the issue is solved on Firefox 12, at least since a2 (2012-03-13), thanks everybody :)
Marking verified based on comment 23.
Status: RESOLVED → VERIFIED
status-firefox12: fixed → verified
Alias: CVE-2012-0475
Group: core-security
See Also: → bug 1192666
You need to log in before you can comment on or make changes to this bug.