Closed Bug 1191423 (CVE-2015-7208) Opened 9 years ago Closed 9 years ago

allowing vertical tab in cookies leads to cookie injection on some servers


(Core :: Networking: Cookies, defect)

42 Branch
Not set



Tracking Status
firefox41 --- wontfix
firefox42 + wontfix
firefox43 + fixed
firefox44 --- fixed
firefox-esr38 --- wontfix


(Reporter: todayisnew, Assigned: u408661)



(Keywords: csectype-other, sec-moderate, Whiteboard: [adv-main43+])


(7 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.125 Safari/537.36

Steps to reproduce:


Firefox allows storing vertical tabs (ASCII 11) in cookies being set.
When later replayed by the browser, they can be interrupted by parser as a new line
This allows setting arbitrary cookies for user.

Example case:


User visits, it asks of course do you like pizza?

It parses this user submited data and stores it in a cookie named likespizza.

This data is cleaned for xss, or injection but allows through a vertical tab since there is no harm IE, and Chrome do not allow saving vertical tabs in cookies.

This cookie is roughly stored in the jar as:



Malicious user sends link which injects value to set likespizza to create user cookie;;{verticaltab}currentsessionpassword:12345678;

3) Impact:

When victum vists firefox sends the current stored cookie for

When we visit the site we should have the vertical tab just be parsed as a regular character

GET / http/1.0
Cookie: likespizza:true;{vertialtab}currentsession:12345678

But with the middle parser Nginx / lighttp / apache / django etc might parses the vertical tab as a /n or /r so we get a,

GET / http/1.0
Cookie: likespizza:true;
Cookie: currentsessionpassword:12345678;


Many sites facebook google etc, parse http only cookies from url or user submitted content, its an injection point for this type 
of exploit, the middle parser tried to be helpful but can't be trusted to properly parse the vertical tab.


Firefox should filter non standard characters from cookie data, if other browsers do not support them then it is unlikely a developer will
use these characters in cookie data, also if wanted to be used we can base64 encode the data for the cookie to keep it safe and return the characters vertical tab etc inside.
Component: Untriaged → Networking: Cookies
Product: Firefox → Core
Flags: sec-bounty?
Nick--can you take this?  We don't have anyone handy who knows the cookie code well, but we need this fixed.
Flags: needinfo?(hurley)
> But with the middle parser Nginx / lighttp / apache / django etc might 
> parse the vertical tab as a /n or /r

Do we have evidence that any of these servers are parsing a vertical tab as a carriage return?  I.e. how real vs theoretical is this threat right now?

> IE, and Chrome do not allow saving vertical tabs in cookies.

If true that's probably reason enough for us to ban them too.
Does anyone know offhand if IE & Chrome strip vtab from cookies, or just reject cookies containing vtab entirely? (I can figure that out myself, but I always like to ask in case anyone reading knows already.)
Flags: needinfo?(hurley)
So, I just tested... and chrome canary 47.0.2494.0 will happily save (and send!) a cookie with a vtab in it.

So this bug is looking possibly spurious.  Do you or anyone else in security know if there really are server-side vulnerabilities with parsing vertical tabs (ASCII 11) as carriage returns?
Flags: needinfo?(dveditz)
Original reporter (Today is still new :)

Do you have reproducible cases where this is happening that I can reach with a URI?  The doyoulikepizza site seems to be a blank website.
Flags: needinfo?(todayisnew)
good day :) sorry for slow response lucky to have my first new born child here :) happy little girl.  The domain ilikepizza was an example not a real domain :)

I'll look into better case examples, I had tested against chrome and Internet explorer builds about 2 months back, I was looking to have my server send a %0a%0d to force the cookie onto a new line.  When I found Firefox was the only browser that would save out ASCII 11 (vertical tab) 

I will go through all characters ASCII 0-255 to save out to cookies for chrome / safari / Internet explorer / and Firefox current builds post back with results.  Hope the day treats you all well :)

Flags: needinfo?(todayisnew)
Control characters aren't valid in cookies, we should neither accept nor emit such cookies.
Flags: needinfo?(dveditz)
While I agree with the RFC (that control characters in cookies are rather silly), so far every browser I've tested (Firefox, Safari, and Chrome - I'm working on getting a windows VM up on this machine to test IE) will happily accept and send cookies with control characters in them (or at least, vtabs - I'll follow up with tests of other control characters).

I'll also see if httparchive keeps cookie-related stats (though I doubt they do) to see if use of RFC-disallowed characters is prevalent on the internet, as making this change with (so far) such wide support for brokenness could end up breaking things for users who regularly visit non-compliant sites.
Just checked IE 11 on WinXP (the only windows/IE combo I have currently available to me), and it also happily accepts and sends vtabs in cookies.
Good day thanks for the patience :)

Hmm After doing further tests, it seems that Firefox is in the company with with Internet Explorer 9, that is still allowing control characters to be set  / resent as cookies.

Browsers That will not store Control characters  00-20 (Hex) as Cookies, and resend the request as cookies from browser to server:

- Google Chrome Current Build Desktop
- Safari Current Build Desktop
- Internet Explorer 10+ Desktop
- Opera 31.0 Desktop

Browsers That will store Control characters  00-20 (Hex)
- Internet Explorer 9
- Firefox current build

Control Characters:

Control Characters: Hex 00-20
NUL	null
SOH	start of heading
STX	start of text
ETX	end of text
EOT	end of transmission
ENQ	enquiry
ACK	acknowledge
BEL	bell
BS	backspace
HT	horizontal tab
VT	vertical tab
NP	new page (or FF, form feed)
SO	shift out
SI	shift in
DLE	data link escape
DC1	device control 1
DC2	device control 2
DC3	device control 3
DC4	device control 4
NAK	negative acknowledge
SYN	synchronous idle
ETB	end of transmission block
CAN	cancel
EM	end of medium
SUB	substitute
ESC	escape
FS	file separator
GS	group separator
RS	record separator
US	unit separator
SP	space

Example Php to test:

Source if want to make own:


header('Set-Cookie: firefoxtest=    !\"#$%&\'()*+,-./0123456789:<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~€‚ƒ„…†‡ˆ‰Š‹ŒŽ‘’“”•–—˜™š›œžŸ ¡¢£¤¥¦§¨©ª«¬­®¯°±²³´µ¶·¸¹º»¼½¾¿ÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖ×ØÙÚÛÜÝÞßàáâãäåæçèéêëìíîïðñòóôõö÷øùúûüýþÿ');


Results from fiddler dump:

User-Agent: Mozilla/5.0 (Android; Mobile; rv:29.0) Gecko/29.0 Firefox/29.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-GB,en;q=0.5
Accept-Encoding: gzip, deflate
Cookie: firefoxtest=    
 !\"#$%&'()*+,-./0123456789:<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~€‚ƒ„…†‡ˆ‰Š‹ŒŽ‘’“”•–—˜™š›œžŸ ¡¢£¤¥¦§¨©ª«¬­®¯°±²³´µ¶·¸¹º»¼½¾¿ÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖ×ØÙÚÛÜÝÞßàáâãäåæçèéêëìíîïðñòóôõö÷øùúûüýþÿ
Connection: keep-alive

HTTP/1.1 200 OK
Date: Fri, 28 Aug 2015 18:12:47 GMT
Server: Apache
X-Powered-By: PHP/5.5.28
Set-Cookie: firefoxtest=    
 !\"#$%&'()*+,-./0123456789:<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~€‚ƒ„…†‡ˆ‰Š‹ŒŽ‘’“”•–—˜™š›œžŸ ¡¢£¤¥¦§¨©ª«¬­®¯°±²³´µ¶·¸¹º»¼½¾¿ÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖ×ØÙÚÛÜÝÞßàáâãäåæçèéêëìíîïðñòóôõö÷øùúûüýþÿ
Content-Length: 0
Keep-Alive: timeout=2, max=200
Connection: Keep-Alive
Content-Type: text/html
Attached image FireFoxScreenShot.jpg
Cookie Characters Set
Attached image Fiddler logs of request
The FireFoxScreenShot.jpg shows a better view of the the spacing, the browser here renders out the new lines being set / resent in the cookie.  

In my experience and testing, Chrome, Safari, Opera do not allow control characters to be set or accepted, it will store an encoded vertical tab as a cookie as "%0B" in chrome.

In Firefox it will store a "♂" which is a vertical tab as its actual ASCII character.

Hope all is well on all of your sides of the screen.  It just seems if most other browsers are filtering, this content for years, would make Firefox more secure to not allow it.
Argh, and of course there was an issue with my test case (accounting for the difference in results) - I was using a framework that "helpfully" turned control characters into strings containing the octal escape sequence for that character.

I wrote yet another test that does not translate control characters into escape sequence strings, and sends one Set-Cookie for each of the ascii characters (other than ;, \0, \r, or \n - see below for why I skipped those). This was more productive. Here's the behavior I'm seeing

- As the reporter suggests, firefox will accept any old character (with the exception of ;, \0, \r, and \n) in a cookie. This was apparently a conscious decision made when the cookie parser was written, as the comments explicitly say we deviate from 6265 and only disallow the 4 characters listed above.

- Chrome appears to be mostly in-line with 6265, though I was able to get it to accept (and send) cookies containing 0x09 (tab), 0x20 (space), 0x22 (dquote), 0x2C (comma), 0x5C (backslash), and 0x7F (del).

- Safari appears to freak out at the first invalid cookie and not accept any cookies sent at all (at least in my case). It does, strangely, store the first invalid cookie name with an empty value and send that on later requests (so it stored the cookie named "ascii0x01" with an empty value and sent that on subsequent requests).

- I did not test IE, as this seems like enough information to go on for now. I will test later.

Looking at our cookie code, it seems like the easiest route to go is to behave like chrome. We could be more strict than them and fully follow 6265, but I don't see any particular reason not to (none of the extra characters they allow are particularly harmful, with the possible exception of dquote).
Nicholas its all good, the helper tools betray us on occasion :)

Thanks for the heads up, you found some mistakes I made with chrome sneaking extra tab space comma dquote backslash and del! :)
Attached patch patch (obsolete) — Splinter Review
This disallows the same set of characters that chrome disallows in cookies.
The test doesn't test *all* disallowed values, as there's some disconnect
between xpcshell and gecko that results in some of the disallowed values not
making it through to SetCookieInternal, and therefore some empty cookies
being created, so the test just exists as a sanity check.

I'm not sure how we'll want to go about landing this - the implications of
this bug don't seem too horrid to me, as they require server-side
cooperation to exploit at 2 levels (1 - allowing setting cookies directly
from user input, and 2 - improper parsing of sent cookie values), but I'll
hold of on any landing or try pushing until I get guidance.
Attachment #8655107 - Flags: review?(jduell.mcbugs)
alot of sites Facebook Google set cookies based off the current browser URL, user can append any value to the URL after a ?<attackpayload> or a &randomvalue=<attackpayload>

Tesla for example will let you set arbitrary cookie values here:

So if a middle parser apache Nginx Django etc does not know how to handel these cookies a csrf to the site with the users auth already in place to modify and create the attack cookie, then another request to put the malicious cookie into play.

Happy it got patched :) If no bug bounty I understand :)  you seem a lot more down to earth as a group of devs then my experience with bugs with chrome and google ill do some better research and get some better bugs :)

Is the new Firefox port to Android in scope for bug bounties, I can see the iOS version is still in testing?

Have a good one and keep up the good work :)

Attachment #8655107 - Flags: review?(jduell.mcbugs) → review+
Group: core-security → network-core-security
Dan, how do we want to go about landing this one?
Flags: needinfo?(dveditz)
Assignee: nobody → hurley
Clearly this is confirmed.

I believe this was mis-rated "sec-high" based on a misinterpretation of the bug summary -- that it allowed modifying the cookies of any arbitrary site from elsewhere. Rather you can inject cookies on some sites that meet certain conditions which is a more limited attack. Could still, in theory, allow for something like a session-fixation attack on particular sites. Lowering to sec-moderate.

Nick: in general the answer to comment 20 for sec bugs is you request sec-approval? on the patch and answer the questions in the template about risk and affected versions. Since I've downrated it to moderate you can just land. It'd be nice to get this on aurora, too. Not urgent to fix on beta.
Ever confirmed: true
Flags: needinfo?(dveditz)
(In reply to todayisnew from comment #19)
> Happy it got patched :) If no bug bounty I understand :) [...]
> Is the new Firefox port to Android in scope for bug bounties, I can see the
> iOS version is still in testing?

bounty question are off-topic for bugs and best addressed in mail to our security@ mailing list, but this bug is nominated for a bounty and the bounty folks will consider it whenever their next meeting is.

Firefox for Android is definitely in scope for bounties.

Firefox for iOS has not had any formal security review so it's not in scope yet. Since we're forced to use the iOS webkit view to do the actual browsing on that platform we need to figure out which security aspects we can actually be responsible for.
Comment on attachment 8655107 [details] [diff] [review]

[Triage Comment]
approved for Aurora landing
Attachment #8655107 - Flags: approval-mozilla-aurora+
Will land on aurora once inbound seems happy with the push
And backed out in for causing

Apparently some of our tests relied on our broken cookie parsing :)
Attached patch patch v2Splinter Review
Here's a new version that, by my testing, appears to not cause the giant mess of cookie-related test failures that the previous one caused (and should also fix the ASAN complaint about a buffer overrun - forgot to manually null-terminate my list of characters that we disallow).

The big difference here is that the blacklist now only applies to cookies set via the Set-Cookie header. "Why is that?" I hear you asking. Here's the fun part. Set-Cookie is defined by RFC 6265 as able to carry a subset of US-ASCII characters (as we've already noted). The problem is document.cookie, which is defined by the HTML5 spec as UTF-8. Who needs anything to match?

Jason, this seems right-ish to me, and like I said, many more tests are passing with this version than with my previous version, but one never knows.
Attachment #8655107 - Attachment is obsolete: true
Attachment #8657343 - Flags: review?(jduell.mcbugs)
Summary: allow modification of any sites cookies → allowing vertical tab in cookies leads to cookie injection on some servers
Hmm, in

it says that setting document.cookie should "act as it would when receiving a set-cookie-string for the document's address via a "non-HTTP" API, consisting of the new value encoded as UTF-8".  My read of that is that the same set of illegal characters should be enforced for document.cookie setting as well as HTTP header cookie setting. I.e. we should only allow the "cookie-octet" chars from RFC 6265:

  %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
                       ; US-ASCII characters excluding CTLs,
                       ; whitespace DQUOTE, comma, semicolon,
                       ; and backslash

(AFAICT the only thing the "non-HTTP"-ness affects when getting/setting a cookie is that you can't see httponly cookies.)

bz: you +r'd the patches ages ago that added the tests here (dom/html/test/test_non-ascii-cookie.html) that are barfing because they want to use values other than these.  Do you repent and see the error of your ways (i.e. we should change the tests), or is there reason to support the use of other chars, only when using document.cookie?

Nick: the only test that's failing is test_non-ascii-cookie.html, right?
Flags: needinfo?(bzbarsky)
Note: see bug 784367 and maybe bug 826159 for the prior work here.
(In reply to Jason Duell [:jduell] (needinfo? me) from comment #28)
> Nick: the only test that's failing is test_non-ascii-cookie.html, right?

No, there were a ton of tests that failed with the old version of the patch, all of which are fixed (at least locally) by only applying this to Set-Cookie instead of document.cookie.
> or is there reason to support the use of other chars, only when using document.cookie

What do other browsers do?  That's the only real criterion for document.cookie.

In spec terms, as usual the RFC doesn't define a processing model and in particular doesn't say what should happen when the header value does not match the cookie-string production.  the HTML spec thus has no way to link to where it enters the processing model from the RFC (because the RFC doesn't define one), and hence it's not clear whether the cookie-string restrictions would in fact apply to the DOM API.  Yay, standards.

Once we figure out what other browsers do here, we should file an HTML spec bug to define the behavior here better.
Flags: needinfo?(bzbarsky)
OK, here's the state of other browsers (specifically, I tested using the cookie set via document.cookie from test_non-ascii-cookie.html):

Chrome (canary 47.0.2511.0): accepts the cookie both via document.cookie and Set-Cookie
Safari (8.0.8 (10600.8.9)): does not accept the cookie from either source (though it does create an empty cookie with the same name)
Edge (20.10240.16384.0): accepts the cookie via document.cookie, does NOT accept the cookie via Set-Cookie. NOTE: this is the same behavior that my proposed patch will introduce for Firefox.
IE 11: same as Edge (not really surprising)

Given the... inconsistent state of this across browsers (plus the fact that my proposed behavior aligns with at least one of the other major browsers), I feel relatively safe with this change. Of course, it's the internet, so there's always the possibility of me breaking everything for everyone :)
Comment on attachment 8657343 [details] [diff] [review]
patch v2

Review of attachment 8657343 [details] [diff] [review]:

I agree--this seems good enough for now, at least.  Ship it! :)
Attachment #8657343 - Flags: review?(jduell.mcbugs) → review+
Closed: 9 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Thanks all for letting me join in and see the adventure along the way :)  

You do a lot of hard work and back and forth to make sure it gets fixed proper :)  Thought fining the bugs was hard, fixing them seems to be just as hard too ;)

Good luck with the adventures and I'll try to bring some more bugs (hopefully better examples and more clearly defined)

Hope the sun shines on you all, and have a great day!

Group: network-core-security → core-security-release
Nicholas, this need approval requests for beta and aurora i think ? :)
Flags: needinfo?(hurley)
Flags: sec-bounty? → sec-bounty+
Jason can you request uplift for this fix if Nicholas is out? I'd like to get it into at least aurora.
Flags: needinfo?(jduell.mcbugs)
Comment on attachment 8657343 [details] [diff] [review]
patch v2

Approval Request Comment
[Feature/regressing bug #]: forever
[User impact if declined]: hypothetical security risk (we didn't identify any concrete attacks that work)
[Describe test coverage new/current, TreeHerder]:  tests added
[Risks and why]: could conceivably block some cookies that aren't malware.  But unlikely
[String/UUID change made/needed]: none

Given we haven't seen any concrete attack vectors here I'm not pushing hard for beta here, and wouldn't be put off if we just let this ride the trains. But see comment 21 and comment 23. I'm happy to defer to dveditz.
Flags: needinfo?(jduell.mcbugs)
Flags: needinfo?(hurley)
Attachment #8657343 - Flags: approval-mozilla-beta?
Attachment #8657343 - Flags: approval-mozilla-aurora?
Comment on attachment 8657343 [details] [diff] [review]
patch v2

42 being a special release, I would like to avoid the uplift to 42. It seems that it might generate some noise on the channel.
Anyway, taking it in aurora.
Attachment #8657343 - Flags: approval-mozilla-beta?
Attachment #8657343 - Flags: approval-mozilla-beta-
Attachment #8657343 - Flags: approval-mozilla-aurora?
Attachment #8657343 - Flags: approval-mozilla-aurora+
Hi Eric, does this fix address the use cases you reported?
Flags: needinfo?(todayisnew)
I can check :) which build / version should I check against?
Flags: needinfo?(todayisnew)
Thanks!  A Firefox 43 beta should be good, given when this was checked in.
Alias: CVE-2015-7208
Hello :)

I hope life has treated everyone here well, and the summer has been find if you are in the northern hemisphere :)

1) I have tested and verified the new fix for cookies in build 43, it stores the cookies names as the url encoded versions:

See image: 01-fireFox43InvalidCookieValueFixed.png

2) I did another test and found though we have another potential issue, not sure if I should open a separate bug, or continue with it here.

If we do the same setting of cookies, but instead of the value, we set the cookie name to invalid characters, Firefox will happily set the cookie name values
for characters rfc says it should not, and other browsers do not do (chrome).

FireFox: 02-fireFox43InvalidCharsInCookieNames.png

Chrome: 03-googleValidCookies.png

4)I wrote a tool to test setting cookies for browser if it helps to see:<integer start position 0 to 255>&howManyCookiesToSet=<number of cookies to set from start point>&setType=<name or value>


Start as ascii character 0, set 100 cookies which we will set for the cookie name.

Wish you all well on your side of your screens :)

Yeah, the name portion is still a problem. I had a look at other browsers, and the status of compat there is much more consistent than with odd characters in the values. All browsers I tested (with the exception of Firefox) disallow any character 0x1e and below, as well as space (0x20). IE/Edge is the only browser other than Firefox that allows 0x1f in a name. Beyond that, it gets a bit messier, but not too crazy (safari is slightly stricter than everyone else). I think we're safe disallowing characters <= 0x20 to occur in the name. I'll clone this bug and attach a patch to fix it.
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.