Last Comment Bug 213012 - relative URLs can violate cookie path restrictions
: relative URLs can violate cookie path restrictions
Status: RESOLVED FIXED
: fixed1.4.1
Product: Core
Classification: Components
Component: Networking: Cookies (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Darin Fisher
:
: Patrick McManus [:mcmanus]
Mentors:
Depends on:
Blocks: 224532
  Show dependency treegraph
 
Reported: 2003-07-17 14:57 PDT by Daniel Veditz [:dveditz]
Modified: 2003-11-24 07:30 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
quick and dirty patch, replaces %2E or %2e in the path with . and then does the usual stuff (1.24 KB, patch)
2003-07-19 12:04 PDT, Andreas Otte
dveditz: review+
darin.moz: superreview+
Details | Diff | Splinter Review
same patch as before but against 1.4 (2.10 KB, patch)
2003-07-24 08:40 PDT, Andreas Otte
dveditz: review+
darin.moz: superreview+
mozilla: approval1.4.1+
Details | Diff | Splinter Review
i18n safe trunk patch (1.51 KB, patch)
2003-07-24 12:59 PDT, Andreas Otte
no flags Details | Diff | Splinter Review
i18n safe 1.4 patch (2.64 KB, patch)
2003-07-24 13:00 PDT, Andreas Otte
no flags Details | Diff | Splinter Review

Description Daniel Veditz [:dveditz] 2003-07-17 14:57:42 PDT
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 dwitte@gmail.com 2003-07-18 02:00:16 PDT
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) 2003-07-18 02:10:03 PDT
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 2003-07-18 09:21:46 PDT
dveditz and i thought about making net_CoalesceDirsAbs (in nsURLHelper.cpp)
treat %2E equivalently to a simple unescaped '.'
Comment 4 Daniel Veditz [:dveditz] 2003-07-18 12:46:19 PDT
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] 2003-07-18 12:56:14 PDT
bbaetz: bug 114018 doesn't seem relevant, did you mean another?
Comment 6 Andreas Otte 2003-07-18 15:10:31 PDT
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 2003-07-18 17:50:08 PDT
Searching bugzilla comments for %2e%2e turned up bug 41571 and bug 182176.
Comment 8 Bradley Baetz (:bbaetz) 2003-07-18 19:59:21 PDT
Bug 110418, sorry.
Comment 9 Andreas Otte 2003-07-19 12:04:51 PDT
Created attachment 128080 [details] [diff] [review]
quick and dirty patch, replaces %2E or %2e in the path with . and then does the usual stuff
Comment 10 Daniel Veditz [:dveditz] 2003-07-20 12:23:44 PDT
Should we not do the same thing in net_CoalesceDirsAbs?
Comment 11 Andreas Otte 2003-07-20 13:16:51 PDT
That function does not exist anymore, see bug 207298 for details or update your
tree ;-)
Comment 12 Andreas Otte 2003-07-20 13:26:25 PDT
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 2003-07-23 09:11:33 PDT
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 2003-07-23 09:19:27 PDT
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 2003-07-24 08:40:19 PDT
Created attachment 128429 [details] [diff] [review]
same patch as before but against 1.4
Comment 16 Asaf Amit 2003-07-24 09:09:03 PDT
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] 2003-07-24 10:05:47 PDT
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] 2003-07-24 10:06:22 PDT
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] 2003-07-24 10:06:35 PDT
Comment on attachment 128429 [details] [diff] [review]
same patch as before but against 1.4

r=dveditz
Comment 20 Andreas Otte 2003-07-24 10:13:24 PDT
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 2003-07-24 10:21:30 PDT
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 2003-07-24 11:27:19 PDT
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 2003-07-24 12:59:25 PDT
Created attachment 128451 [details] [diff] [review]
i18n safe trunk patch
Comment 24 Andreas Otte 2003-07-24 13:00:14 PDT
Created attachment 128453 [details] [diff] [review]
i18n safe 1.4 patch
Comment 25 Darin Fisher 2003-07-24 14:33:07 PDT
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 2003-07-24 14:39:13 PDT
In return this would mean that that code in coalesceDirs is old cruft that can
be removed when conveniant.
Comment 27 Daniel Veditz [:dveditz] 2003-07-24 15:01:46 PDT
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 2003-07-25 11:38:14 PDT
fix checked in on trunk
Comment 29 Mike Kaply [:mkaply] 2003-07-28 13:44:27 PDT
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 2003-07-28 14:21:17 PDT
fix also checked in into the 1.4 branch
Comment 31 Christopher Aillon (sabbatical, not receiving bugmail) 2003-11-24 07:30:02 PST
Opening.

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