Bug 373228 (CVE-2007-1362)

Cookie injection problems

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: dveditz, Assigned: dveditz)

Tracking

({fixed1.8.0.12, fixed1.8.1.4})

1.8 Branch
Points:
---
Bug Flags:
blocking1.9 +
blocking1.8.1.4 +
wanted1.8.1.x +
blocking1.8.0.12 +
wanted1.8.0.x +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:dos])

Attachments

(5 attachments)

Assignee

Description

12 years ago
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.
Assignee

Updated

12 years ago
Group: security
Assignee

Updated

12 years ago
Attachment #257857 - Attachment mime type: text/plain → text/plain;charset=ISO-8859-1
Assignee

Comment 1

12 years ago
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.
Assignee

Updated

12 years ago
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.
Assignee

Updated

12 years ago
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.9?
Flags: blocking1.8.1.4?
Flags: blocking1.8.0.12?
Assignee

Updated

12 years ago
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+
Assignee

Updated

12 years ago
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+

Updated

12 years ago
Flags: blocking1.9? → blocking1.9+
Assignee

Updated

12 years ago
Assignee: nobody → dveditz
Assignee

Comment 4

12 years ago
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 5

12 years ago
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+
Assignee

Comment 6

12 years ago
fix checked into trunk, 1.8 and 1.8.0 branches
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee

Updated

12 years ago
Whiteboard: [sg:low dos]
Assignee

Comment 7

12 years ago
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".
Assignee

Comment 8

12 years ago
Attachment #263500 - Flags: review?(dwitte)
Attachment #263500 - Flags: approval1.8.1.4?
Attachment #263500 - Flags: approval1.8.0.12?

Updated

12 years ago
Attachment #263500 - Flags: review?(dwitte) → review+
Assignee

Comment 9

12 years ago
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
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+
Assignee

Updated

12 years ago
Assignee

Comment 10

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

Comment 11

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

Updated

12 years ago
Group: security
is tab "\t" in cookie.value safe?

it is accepted
Assignee

Comment 13

12 years ago
Tabs are explicitly allowed in cookie content
Assignee

Updated

10 years ago
Whiteboard: [sg:low dos] → [sg:dos]
You need to log in before you can comment on or make changes to this bug.