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.

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.

Comment on attachment 520373 [details] [diff] [review] patch r=me Might be worth creating a mochitest with document.cookie too.

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).

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

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.

Comment on attachment 525202 [details] [diff] [review] Patch v2 Approved for 1.9.2.18, 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 ;-)

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 1.9.2.18.