Closed Bug 373228 (CVE-2007-1362) Opened 18 years ago Closed 17 years ago

Cookie injection problems


(Core :: Networking: Cookies, defect)

1.8 Branch
Not set





(Reporter: dveditz, Assigned: dveditz)


(Keywords: fixed1.8.0.12, fixed1.8.1.4, Whiteboard: [sg:dos])


(5 files)

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.
Group: security
Attachment #257857 - Attachment mime type: text/plain → text/plain;charset=ISO-8859-1

#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.
Alias: CVE-2007-1362
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.
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.9?
Flags: blocking1.8.1.4?
Flags: blocking1.8.0.12?
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
Flags: blocking1.9? → blocking1.9+
Assignee: nobody → dveditz
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.
Attachment #263342 - Flags: review?
Comment on attachment 263342 [details] [diff] [review]
enforce path length and tablessness

r=dwitte, yay for sanity checks. thanks dveditz!
Attachment #263342 - Flags: review? → review+
fix checked into trunk, 1.8 and 1.8.0 branches
Closed: 17 years ago
Resolution: --- → FIXED
Whiteboard: [sg:low dos]
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".
Attachment #263500 - Flags: review?(dwitte)
Attachment #263500 - Flags: approval1.8.1.4?
Attachment #263500 - Flags: approval1.8.0.12?
Attachment #263500 - Flags: review?(dwitte) → review+
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
Attachment #263500 - Flags: approval1.8.1.4?
Attachment #263500 - Flags: approval1.8.1.4+
Attachment #263500 - Flags: approval1.8.0.12?
Attachment #263500 - Flags: approval1.8.0.12+
attachment 263500 [details] [diff] [review] checked into trunk, 1.8 and 1.8.0 branches.
unit test for the new path checks; i've checked this into trunk only.
Group: security
is tab "\t" in cookie.value safe?

it is accepted
Tabs are explicitly allowed in cookie content
Whiteboard: [sg:low dos] → [sg:dos]
You need to log in before you can comment on or make changes to this bug.