Closed Bug 643051 Opened 12 years ago Closed 12 years ago
.cookie should only allow setting one cookie at a time
Currently, document.cookie allows setting multiple cookies in a single call. This is inconsistent with other browsers as well as our documentation: https://developer.mozilla.org/en/document.cookie "Note that you can only set/update a single cookie at a time using this method." This can lead to security problems in web applications: When an external script takes untrusted input (e.g. from a URL parameter), that input can contain line breaks, and thus set multiple cookies to the including web site. Therefore, we should disallow setting multiple cookies using document.cookie.
Assignee: nobody → cbiesinger
Status: NEW → ASSIGNED
Attachment #520373 - Flags: review?
Attachment #520373 - Flags: review? → review?(jduell.mcbugs)
As a security problem this really arises because of insufficient filtering on the web app side more than a problem in Firefox. But it's clearly a bug and we should fix it.
Whiteboard: [sg:low] → [sg:nse]
Comment on attachment 520373 [details] [diff] [review] patch r=me Might be worth creating a mochitest with document.cookie too.
Attachment #520373 - Flags: review?(jduell.mcbugs) → review+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [sg:nse] → [sg:nse] fixed-in-cedar
Comment on attachment 524733 [details] [diff] [review] final patch for checkin transfering r=bz I would like to land this on the ff4 and 3.6 branches too, if possible.
This failed mochitest-1 persistently, and the followups didn't fix it, so I backed out the original patch and both followups from cedar: http://hg.mozilla.org/projects/cedar/rev/4e6e2a9c48c6 http://hg.mozilla.org/projects/cedar/rev/865870bbe148 http://hg.mozilla.org/projects/cedar/rev/a998aa8043f1 http://hg.mozilla.org/projects/cedar/rev/9409b0c8864f http://hg.mozilla.org/projects/cedar/rev/c04359d5920f Reopening the bug (which shouldn't have been marked Resolved:Fixed yet anyway as it hadn't landed on m-c).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [sg:nse] fixed-in-cedar → [sg:nse]
Let's try this again. Currently running this version through a local "make mochitest-1", will push to cedar if that worked. This version clears the "a" cookie in the test that sets it.
Landed again, this time on m-c. Let's hope this will stick. Once I have test results, will request approval again. http://hg.mozilla.org/mozilla-central/rev/61b822ac2d41
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment on attachment 525202 [details] [diff] [review] Patch v2 This is now passing the tests on Tinderbox. Transferring r=bz. Since this affects webapp security I'd like to check this in to the branches too.
Attachment #525202 - Flags: approval220.127.116.11? → approval18.104.22.168?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Comment on attachment 525202 [details] [diff] [review] Patch v2 Approved for 22.214.171.124, a=dveditz clearing request for dead mozilla2.0 branch
Per security group discussion, requesting landing on mozilla-2.0.
Comment on attachment 525202 [details] [diff] [review] Patch v2 Approved for the mozilla2.0 repository, a=dveditz Cameron: if you request approval on these using flags it'd be easier to find than watching random IRC conversations ;-)
Attachment #525202 - Flags: approval2.0+
Ah, flag was semi-disabled so you couldn't. Fixed now, request away.
Yes -- thanks for reenabling it. I will properly remark the other bugs (it looks like some were done already).
Verified fixed based on the updated tests passing for 126.96.36.199.
You need to log in before you can comment on or make changes to this bug.