373228 - (CVE-2007-1362) Cookie injection problems

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[Daniel Veditz [:dveditz]]

Attachment: original advisory (French)

Nicolas DEROUET sent an advisory about multiple similar problems with cookies that he says "could be exploited by malicious users to bypass the same origin policy, cause a denial of service and conduct other attacks by writing the path of 'document.cookie' with tabulations or/and a large size."

I think "bypass same origin" is wrong, but there's definite problems such as cookie injection (definitely mucking with the path, will have to see about the host). His examples all involve setting document.cookie through the DOM, but I think it'd end up in the same code if sent as an HTTP header.

Might want to generate sub-bugs for the individual problems if they are different enough.

Comment 1

[Daniel Veditz [:dveditz]]

highlights:

#1 introduction
cookies are stored in a tab-delimited file with 7 fields. We're interested in the PATH of the cookie.

#2 Injection
If you put a tab character in the path then you break the file format and can hack the interpretation of fields after that point. (variations)

#3 No size limit on path
Unlike cookie max size or max number we don't limit path size, and since the cookie file is read into memory we can easily cause a persistent DoS or Degradation of Service by eating most of someone's memory.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Attachment: English translation of original advisory

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

It seems to me that the name and the value of the cookie have, in theory, the same problems. They get written out to the cookie file without any escaping.

A solution might be to either quote those values or url-escape them. The problem is to make it work with older cookies.txt files. In the ideal case, in both directions: reading old files and old applications reading the new file. I'm not sure how we can do that.

Another solution might be to only escape tabs. That might be the least intrusive, but is not as generic. There might still be other, similar, problems.

Comment 4

[Daniel Veditz [:dveditz]]

Attachment: enforce path length and tablessness

Limits paths to 1K, and rejects cookies with either a name or path containing a tab. Left domain unchecked because tab is an invalid DNS char (we won't load it so you can't set cookies from it); tabs are allowed as part of the cookie value.

Comment 5

[[e-mail]]

Comment on attachment 263342 [details] [diff] [review]
enforce path length and tablessness

r=dwitte, yay for sanity checks. thanks dveditz!

Comment 6

[Daniel Veditz [:dveditz]]

fix checked into trunk, 1.8 and 1.8.0 branches

Comment 7

[Daniel Veditz [:dveditz]]

dwitte points out that a malicious server wouldn't have to implement URI paths as directory paths so there could in fact be extremely long paths and embedded tabs even when the path is implicit in the URL. The checks I added should have been on both paths, not just in the "else".

Comment 8

[Daniel Veditz [:dveditz]]

Attachment: check path for tabs regardless of where we got it

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 263500 [details] [diff] [review]
check path for tabs regardless of where we got it

approved for 1.8.0.12 and 1.8.1.4, a=dveditz for release-drivers

Comment 10

[Daniel Veditz [:dveditz]]

attachment 263500 [details] [diff] [review] checked into trunk, 1.8 and 1.8.0 branches.

Comment 11

[[e-mail]]

Attachment: unit test for path checks

unit test for the new path checks; i've checked this into trunk only.

Comment 12

[georgi - hopefully not receiving bugspam]

is tab "\t" in cookie.value safe?

it is accepted

Comment 13

[Daniel Veditz [:dveditz]]

Tabs are explicitly allowed in cookie content