213012 - relative URLs can violate cookie path restrictions

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[Daniel Veditz [:dveditz]]

Netscape received the following report from Martin O'Neal. In short, a URL like
http://x.com/victim/%2E%2E/evil/hack.html is incorrectly able to read cookies
with the path set to "/victim".

If a literal ".." is used then Mozilla coalesces the path before matching
against cookies, with the result that "/victim" will not match "/evil". Some
servers (Netscape's NES, for example) will not accept %2E%2E as a relative path,
but others (e.g. Apache) do. We should not depend on the server implementation
for our security, the coalescing code should treat %2e as a '.'

On the other hand, our Same-origin policy means /evil/hack.html could open
/victim/target.html in a frame and take the cookies through the DOM. But the
described hack does not require javascript.

----------Report follows-----------------
Issue details:
-- Overview --

The cookie specifications detail a path argument 
that can be used to restrict the areas of a host 
that will be exposed to a cookie. By using 
standard encoding techniques this functionality 
can be subverted, potentially exposing the cookie 
to scrutiny and use in further attacks.


-- Analysis --

The cookie standard is formally defined in 
RFC2965 [1]. This makes reference to the optional 
path argument that allows a cookie originator to 
specify “the subset of URLs on the origin server 
to which this cookie applies” [1].

Many of the user agents appear to function by 
simply matching the initial part of the requested 
URL, so by using a combination of relative paths 
and standard encoding techniques the path 
restriction functionality can be subverted. 

Where this becomes useful is in conducting 
attacks against the session cookies of an 
application that does not suffer from any 
exploitable input validation flaws, but that 
shares the same server environment with one that 
does.


-- Example --

For this example we shall imagine that our secure 
application shares a host with some sample files 
that were installed at the same time as the web 
server. Obviously, this would never happen in a 
live production environment (pauses to insert 
tongue firmly in cheek).

The secure application is located within 
the “/secure” folder and sets the cookie path 
argument to “/secure” which is intended to 
restrict the cookie information from being 
exposed elsewhere on the same host.

The attacker knows that the secure application 
has no useable vulnerabilities in itself and can 
also see that the cookie that it sets has the 
path restricted. They also know that the sample 
files have an exploitable XSS flaw that would 
give them access to the all-important session 
cookies (if they can get a valid user to access 
it; a completely different problem to solve).

By using a crafted URL they can bypass the path 
restriction intended by the originator and get 
the valid users browser to expose the session 
cookie to the sample application. 


-- Recommendations --

The cookie path functionality of the affected 
user agents should be revised to ensure that they 
work as intended and cannot be bypassed by 
encoding techniques.


-- Escalation --

>From initial analysis, it appears this style of 
flaw might be widespread across various agents. 
If this is confirmed as being so, then we will 
most likely need to coordinate a public release 
across vendors.

Comment 1

[[e-mail]]

I'm wondering if this is really a cookie bug, or something we should fix in the
various implementations of nsIURI... currently, cookies just calls
nsIURI::GetPath and then matches that against a stored string. So, if the nsIURI
impls coalesced the path in this case, I don't think there'd be a problem. Comments?

thanks for filing this dveditz!

Comment 2

[Bradley Baetz (:bbaetz)]

I'm getting a feeling of deja vu here - haven't we seen this before? theres bug
114018, but I thought we had something on the .. thing too. Maybe I'm
misremembering.

Comment 3

[Darin Fisher]

dveditz and i thought about making net_CoalesceDirsAbs (in nsURLHelper.cpp)
treat %2E equivalently to a simple unescaped '.'

Comment 4

[Daniel Veditz [:dveditz]]

Should we also watch out for %2F==/ just in case? The two servers we tested
wouldn't act on that equivalence, and it *is* in the restricted set (unlike '.')
so arguably we shouldn't have to.

Comment 5

[Daniel Veditz [:dveditz]]

bbaetz: bug 114018 doesn't seem relevant, did you mean another?

Comment 6

[Andreas Otte]

Now that I have access to this bug (bugzilla is strange sometimes) repeating
what I mailed before:

RFC 2396 says:

   2.3. Unreserved Characters

   Data characters that are allowed in a URI but do not have a reserved
   purpose are called unreserved.  These include upper and lower case
   letters, decimal digits, and a limited set of punctuation marks and
   symbols.

      unreserved  = alphanum | mark

      mark        = "-" | "_" | "." | "!" | "~" | "*" | "'" | "(" | ")"

   Unreserved characters can be escaped without changing the semantics
   of the URI, but this should not be done unless the URI is being used
   in a context that does not allow the unescaped character to appear.



A "." can be escaped without changing the semantics of the url. For that reason
I think we should change CoalesceDirs in a way that it also recognizes %2E and
%2e as . For example we can do a replace of %2E or %2e to "." at the beginning
of CoalesceDirs and then use the usual algorithm.

But please do not watch out for %2F .. doing so would change the semantics of
the url, usually %2F is used to hide a slash that is for example part of a
directory or filename. Looking for %2F would have really strange effects.

Comment 7

[Jesse Ruderman]

Searching bugzilla comments for %2e%2e turned up bug 41571 and bug 182176.

Comment 8

[Bradley Baetz (:bbaetz)]

Bug 110418, sorry.

Comment 9

[Andreas Otte]

Attachment: quick and dirty patch, replaces %2E or %2e in the path with . and then does the usual stuff

Comment 10

[Daniel Veditz [:dveditz]]

Should we not do the same thing in net_CoalesceDirsAbs?

Comment 11

[Andreas Otte]

That function does not exist anymore, see bug 207298 for details or update your
tree ;-)

Comment 12

[Andreas Otte]

But of course, if you want this fix in 1.4x or 1.5a you must do this patch on
net_coalesceDirsAbs and net_coalesceDirsRel instead.

Comment 13

[Andreas Otte]

Comment on attachment 128080 [details] [diff] [review]
quick and dirty patch, replaces %2E or %2e in the path with . and then does the usual stuff

since no one instantly objected to this quick fix, requesting reviews

Comment 14

[Darin Fisher]

Comment on attachment 128080 [details] [diff] [review]
quick and dirty patch, replaces %2E or %2e in the path with . and then does the usual stuff

this looks good to me.	thanks andreas!  sr=darin

Comment 15

[Andreas Otte]

Attachment: same patch as before but against 1.4

Comment 16

[Asaf Amit]

There is a potential i18n problem with this change. If the URL contained  
escaped multibyte-sequences, the straightforward-conversion of %2E to '.' 
might corrupt the URL.

A multibyte-sequence starts with a non-ASCII character, but the trailing bytes
can be 7-bit ASCII and therefore one of them might be 0x2E.

This can probably be prevented if the URL will be unescaped as whole. 

Comment 17

[Daniel Veditz [:dveditz]]

The conversion shouldn't affect the intepretation -- the server gets %2e and
converts it to '.', or we send it the '.' directly. Either way if it's part of a
multi-byte sequence it should be combinable with the previous character.  We
only strip out the dots if after conversion there are one or two in a row
bounded by slashes which is never going to be a multibyte sequence.

Comment 18

[Daniel Veditz [:dveditz]]

Comment on attachment 128080 [details] [diff] [review]
quick and dirty patch, replaces %2E or %2e in the path with . and then does the usual stuff

r=dveditz

Comment 19

[Daniel Veditz [:dveditz]]

Comment on attachment 128429 [details] [diff] [review]
same patch as before but against 1.4

r=dveditz

Comment 20

[Andreas Otte]

I agree with Dan here. If %2E would be "IsDBCSLeadByte" we would have a problem,
but as far as I know it isn't.

Comment 21

[Andreas Otte]

Comment on attachment 128429 [details] [diff] [review]
same patch as before but against 1.4

should this also be fixed for 1.0.x?

Comment 22

[Andreas Otte]

Thinking again about what Asaf Amit said, if the first byte is "IsDBCSLeadByte"
and the following chars are "%2E" ("%" being the other byte) we would destroy
that by replacing "%2E" with ".". Does this make sense?

Comment 23

[Andreas Otte]

Attachment: i18n safe trunk patch

Comment 24

[Andreas Otte]

Attachment: i18n safe 1.4 patch

Comment 25

[Darin Fisher]

hmm... when net_CoalesceDirs is called from BuildNormalizedSpec, the input
string will have any non-ASCII characters (or possibly non-UTF8 characters
depending on preferences) URL-escaped.  so, there cannot be an unescaped DBCS
lead byte passed to net_CoalesceDirs when invoked from BuildNormalizedSpec. 
likewise, since the input/output of nsIURI::resolve must be a UTF-8 string, the
input to net_CoalesceDirs will not contain an unescaped DBCS lead byte.  so, i
think the original patches are i18n safe.

Comment 26

[Andreas Otte]

In return this would mean that that code in coalesceDirs is old cruft that can
be removed when conveniant.

Comment 27

[Daniel Veditz [:dveditz]]

literal '%' is not a valid URL character. If you were really trying to enter a
path with "<2ndByte-%>2e" in it you should escape it as %252e.

Comment 28

[Andreas Otte]

fix checked in on trunk

Comment 29

[Mike Kaply [:mkaply]]

Comment on attachment 128429 [details] [diff] [review]
same patch as before but against 1.4

Please check the appropriate patch into 1.4.

Thanks

Comment 30

[Andreas Otte]

fix also checked in into the 1.4 branch

Comment 31

[Christopher Aillon (sabbatical, not receiving bugmail)]

Opening.