Bug 373228 (CVE-2007-1362)

Cookie injection problems

RESOLVED FIXED

Status

()

Core
Networking: Cookies
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: dveditz, Assigned: dveditz)

Tracking

({fixed1.8.0.12, fixed1.8.1.4})

1.8 Branch
fixed1.8.0.12, fixed1.8.1.4
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

10 years ago
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.
(Assignee)

Updated

10 years ago
Group: security
(Assignee)

Updated

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

Comment 1

10 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

10 years ago
Alias: CVE-2007-1362
Created attachment 257867 [details]
English translation of original advisory
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

10 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

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

Updated

10 years ago
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
(Assignee)

Updated

10 years ago
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)

Updated

10 years ago
Assignee: nobody → dveditz
(Assignee)

Comment 4

10 years ago
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.
Attachment #263342 - Flags: review?

Comment 5

10 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

10 years ago
fix checked into trunk, 1.8 and 1.8.0 branches
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Keywords: fixed1.8.0.12, fixed1.8.1.4
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Whiteboard: [sg:low dos]
(Assignee)

Comment 7

10 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

10 years ago
Created attachment 263500 [details] [diff] [review]
check path for tabs regardless of where we got it
Attachment #263500 - Flags: review?(dwitte)
Attachment #263500 - Flags: approval1.8.1.4?
Attachment #263500 - Flags: approval1.8.0.12?

Updated

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

Comment 9

10 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

10 years ago
Keywords: fixed1.8.0.12, fixed1.8.1.4
(Assignee)

Comment 10

10 years ago
attachment 263500 [details] [diff] [review] checked into trunk, 1.8 and 1.8.0 branches.
Keywords: fixed1.8.0.12, fixed1.8.1.4

Comment 11

10 years ago
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.
(Assignee)

Updated

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

it is accepted
(Assignee)

Comment 13

10 years ago
Tabs are explicitly allowed in cookie content
(Assignee)

Updated

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