Last Comment Bug 373228 - (CVE-2007-1362) Cookie injection problems
: Cookie injection problems
: fixed1.8.0.12, fixed1.8.1.4
Product: Core
Classification: Components
Component: Networking: Cookies (show other bugs)
: 1.8 Branch
: All All
-- normal (vote)
: ---
Assigned To: Daniel Veditz [:dveditz]
: Patrick McManus [:mcmanus]
Depends on:
  Show dependency treegraph
Reported: 2007-03-08 13:26 PST by Daniel Veditz [:dveditz]
Modified: 2009-05-08 13:57 PDT (History)
13 users (show)
benjamin: blocking1.9+
dveditz: blocking1.8.1.4+
dveditz: wanted1.8.1.x+
dveditz: blocking1.8.0.12+
dveditz: wanted1.8.0.x+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

original advisory (French) (13.70 KB, text/plain;charset=ISO-8859-1)
2007-03-08 13:26 PST, Daniel Veditz [:dveditz]
no flags Details
English translation of original advisory (10.12 KB, text/plain)
2007-03-08 14:31 PST, :Gavin Sharp [email:]
no flags Details
enforce path length and tablessness (3.08 KB, patch)
2007-04-30 23:59 PDT, Daniel Veditz [:dveditz]
dwitte: review+
Details | Diff | Splinter Review
check path for tabs regardless of where we got it (1.89 KB, patch)
2007-05-02 12:19 PDT, Daniel Veditz [:dveditz]
dwitte: review+
dveditz: approval1.8.1.4+
dveditz: approval1.8.0.12+
Details | Diff | Splinter Review
unit test for path checks (7.92 KB, patch)
2007-05-07 04:38 PDT,
no flags Details | Diff | Splinter Review

Description User image Daniel Veditz [:dveditz] 2007-03-08 13:26:44 PST
Created attachment 257857 [details]
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 User image Daniel Veditz [:dveditz] 2007-03-08 14:00:54 PST

#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 User image :Gavin Sharp [email:] 2007-03-08 14:31:18 PST
Created attachment 257867 [details]
English translation of original advisory
Comment 3 User image Michiel van Leeuwen (email: mvl+moz@) 2007-03-09 09:36:42 PST
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 User image Daniel Veditz [:dveditz] 2007-04-30 23:59:29 PDT
Created attachment 263342 [details] [diff] [review]
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 User image 2007-05-01 00:16:02 PDT
Comment on attachment 263342 [details] [diff] [review]
enforce path length and tablessness

r=dwitte, yay for sanity checks. thanks dveditz!
Comment 6 User image Daniel Veditz [:dveditz] 2007-05-01 01:15:19 PDT
fix checked into trunk, 1.8 and 1.8.0 branches
Comment 7 User image Daniel Veditz [:dveditz] 2007-05-02 11:49:11 PDT
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 User image Daniel Veditz [:dveditz] 2007-05-02 12:19:34 PDT
Created attachment 263500 [details] [diff] [review]
check path for tabs regardless of where we got it
Comment 9 User image Daniel Veditz [:dveditz] 2007-05-04 18:58:10 PDT
Comment on attachment 263500 [details] [diff] [review]
check path for tabs regardless of where we got it

approved for and, a=dveditz for release-drivers
Comment 10 User image Daniel Veditz [:dveditz] 2007-05-07 04:15:13 PDT
attachment 263500 [details] [diff] [review] checked into trunk, 1.8 and 1.8.0 branches.
Comment 11 User image 2007-05-07 04:38:58 PDT
Created attachment 263989 [details] [diff] [review]
unit test for path checks

unit test for the new path checks; i've checked this into trunk only.
Comment 12 User image georgi - hopefully not receiving bugspam 2007-05-31 01:24:45 PDT
is tab "\t" in cookie.value safe?

it is accepted
Comment 13 User image Daniel Veditz [:dveditz] 2007-05-31 11:59:17 PDT
Tabs are explicitly allowed in cookie content

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