Prevent XSS in the cookie attributes

FagnerMartinsBrack · FagnerMartinsBrack · commit 6ac440da669a · 2018-02-11T12:00:34.000+11:00
Closes js-cookiegh-400.
diff --git a/README.md b/README.md
@@ -203,7 +203,9 @@ Cookies.remove('name', { path: '' });
 
 (From [Internet Explorer Cookie Internals (FAQ)](http://blogs.msdn.com/b/ieinternals/archive/2009/08/20/wininet-ie-cookie-internals-faq.aspx))
 
-This means one cannot set a path using `path: window.location.pathname` in case such pathname contains a filename like so: `/check.html` (or at least, such cookie cannot be read correctly).
+This means one cannot set a path using `window.location.pathname` in case such pathname contains a filename like so: `/check.html` (or at least, such cookie cannot be read correctly).
+
+In fact, you should never allow untrusted input to set the cookie attributes or you might be exposed to a [XSS attack](https://github.com/js-cookie/js-cookie/issues/396).
 
 ### domain
 
diff --git a/src/js.cookie.js b/src/js.cookie.js
@@ -87,7 +87,15 @@
 					if (attributes[attributeName] === true) {
 						continue;
 					}
-					stringifiedAttributes += '=' + attributes[attributeName];
+
+					// Considers RFC 6265 section 5.2:
+					// ...
+					// 3.  If the remaining unparsed-attributes contains a %x3B (";")
+					//     character:
+					// Consume the characters of the unparsed-attributes up to,
+					// not including, the first %x3B (";") character.
+					// ...
+					stringifiedAttributes += '=' + attributes[attributeName].split(';')[0];
 				}
 				return (document.cookie = key + '=' + value + stringifiedAttributes);
 			}
diff --git a/test/tests.js b/test/tests.js
@@ -294,6 +294,16 @@ QUnit.test('undefined attribute value', function (assert) {
 	}), 'c=v; path=/', 'should not write undefined unofficial attribute');
 });
 
+// github.com/js-cookie/js-cookie/issues/396
+QUnit.test('sanitization of attributes to prevent XSS from untrusted input', function (assert) {
+	assert.expect(1);
+	assert.strictEqual(Cookies.set('c', 'v', {
+		path: '/;domain=sub.domain.com',
+		domain: 'site.com;remove_this',
+		customAttribute: 'value;;remove_this'
+	}), 'c=v; path=/; domain=site.com; customAttribute=value', 'should not allow semicolon in a cookie attribute');
+});
+
 QUnit.module('remove', lifecycle);
 
 QUnit.test('deletion', function (assert) {

Original file line number	Diff line number	Diff line change
`@@ -87,7 +87,15 @@`
`87`	`87`	`if (attributes[attributeName] === true) {`
`88`	`88`	`continue;`
`89`	`89`	`}`
`90`		`- stringifiedAttributes += '=' + attributes[attributeName];`
	`90`	`+`
	`91`	`+ // Considers RFC 6265 section 5.2:`
	`92`	`+ // ...`
	`93`	`+ // 3. If the remaining unparsed-attributes contains a %x3B (";")`
	`94`	`+ // character:`
	`95`	`+ // Consume the characters of the unparsed-attributes up to,`
	`96`	`+ // not including, the first %x3B (";") character.`
	`97`	`+ // ...`
	`98`	`+ stringifiedAttributes += '=' + attributes[attributeName].split(';')[0];`
`91`	`99`	`}`
`92`	`100`	`return (document.cookie = key + '=' + value + stringifiedAttributes);`
`93`	`101`	`}`