Skip to content

Commit 24a2d96

Browse files
committed
web: Validate characters in all cookie attributes.
Our previous control character check was missing a check for U+007F, and also semicolons, which are only allowed in quoted parts of values. This commit checks all attributes and updates the set of disallowed characters.
1 parent 119a195 commit 24a2d96

2 files changed

Lines changed: 88 additions & 2 deletions

File tree

tornado/test/web_test.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import http
2+
13
from tornado.concurrent import Future
24
from tornado import gen
35
from tornado.escape import (
@@ -292,11 +294,67 @@ def get(self):
292294
self.set_cookie("unicode_args", "blah", domain="foo.com", path="/foo")
293295

294296
class SetCookieSpecialCharHandler(RequestHandler):
297+
# "Special" characters are allowed in cookie values, but trigger special quoting.
295298
def get(self):
296299
self.set_cookie("equals", "a=b")
297300
self.set_cookie("semicolon", "a;b")
298301
self.set_cookie("quote", 'a"b')
299302

303+
class SetCookieForbiddenCharHandler(RequestHandler):
304+
def get(self):
305+
# Control characters and semicolons raise errors in cookie names and attributes
306+
# (but not values, which are tested in SetCookieSpecialCharHandler)
307+
for char in list(map(chr, range(0x20))) + [chr(0x7F), ";"]:
308+
try:
309+
self.set_cookie("foo" + char, "bar")
310+
self.write(
311+
"Didn't get expected exception for char %r in name\n" % char
312+
)
313+
except http.cookies.CookieError as e:
314+
if "Invalid cookie attribute name" not in str(e):
315+
self.write(
316+
"unexpected exception for char %r in name: %s\n"
317+
% (char, e)
318+
)
319+
320+
try:
321+
self.set_cookie("foo", "bar", domain="example" + char + ".com")
322+
self.write(
323+
"Didn't get expected exception for char %r in domain\n"
324+
% char
325+
)
326+
except http.cookies.CookieError as e:
327+
if "Invalid cookie attribute domain" not in str(e):
328+
self.write(
329+
"unexpected exception for char %r in domain: %s\n"
330+
% (char, e)
331+
)
332+
333+
try:
334+
self.set_cookie("foo", "bar", path="/" + char)
335+
self.write(
336+
"Didn't get expected exception for char %r in path\n" % char
337+
)
338+
except http.cookies.CookieError as e:
339+
if "Invalid cookie attribute path" not in str(e):
340+
self.write(
341+
"unexpected exception for char %r in path: %s\n"
342+
% (char, e)
343+
)
344+
345+
try:
346+
self.set_cookie("foo", "bar", samesite="a" + char)
347+
self.write(
348+
"Didn't get expected exception for char %r in samesite\n"
349+
% char
350+
)
351+
except http.cookies.CookieError as e:
352+
if "Invalid cookie attribute samesite" not in str(e):
353+
self.write(
354+
"unexpected exception for char %r in samesite: %s\n"
355+
% (char, e)
356+
)
357+
300358
class SetCookieOverwriteHandler(RequestHandler):
301359
def get(self):
302360
self.set_cookie("a", "b", domain="example.com")
@@ -330,6 +388,7 @@ def get(self):
330388
("/get", GetCookieHandler),
331389
("/set_domain", SetCookieDomainHandler),
332390
("/special_char", SetCookieSpecialCharHandler),
391+
("/forbidden_char", SetCookieForbiddenCharHandler),
333392
("/set_overwrite", SetCookieOverwriteHandler),
334393
("/set_max_age", SetCookieMaxAgeHandler),
335394
("/set_expires_days", SetCookieExpiresDaysHandler),
@@ -387,6 +446,12 @@ def test_cookie_special_char(self):
387446
response = self.fetch("/get", headers={"Cookie": header})
388447
self.assertEqual(response.body, utf8(expected))
389448

449+
def test_set_cookie_forbidden_char(self):
450+
response = self.fetch("/forbidden_char")
451+
self.assertEqual(response.code, 200)
452+
self.maxDiff = 10000
453+
self.assertMultiLineEqual(to_unicode(response.body), "")
454+
390455
def test_set_cookie_overwrite(self):
391456
response = self.fetch("/set_overwrite")
392457
headers = response.headers.get_list("Set-Cookie")

tornado/web.py

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -693,9 +693,30 @@ def set_cookie(
693693
# The cookie library only accepts type str, in both python 2 and 3
694694
name = escape.native_str(name)
695695
value = escape.native_str(value)
696-
if re.search(r"[\x00-\x20]", name + value):
697-
# Don't let us accidentally inject bad stuff
696+
if re.search(r"[\x00-\x20]", value):
697+
# Legacy check for control characters in cookie values. This check is no longer needed
698+
# since the cookie library escapes these characters correctly now. It will be removed
699+
# in the next feature release.
698700
raise ValueError(f"Invalid cookie {name!r}: {value!r}")
701+
for attr_name, attr_value in [
702+
("name", name),
703+
("domain", domain),
704+
("path", path),
705+
("samesite", samesite),
706+
]:
707+
# Cookie attributes may not contain control characters or semicolons (except when
708+
# escaped in the value). A check for control characters was added to the http.cookies
709+
# library in a Feb 2026 security release; as of March it still does not check for
710+
# semicolons.
711+
#
712+
# When a semicolon check is added to the standard library (and the release has had time
713+
# for adoption), this check may be removed, but be mindful of the fact that this may
714+
# change the timing of the exception (to the generation of the Set-Cookie header in
715+
# flush()). We m
716+
if attr_value is not None and re.search(r"[\x00-\x20\x3b\x7f]", attr_value):
717+
raise http.cookies.CookieError(
718+
f"Invalid cookie attribute {attr_name}={attr_value!r} for cookie {name!r}"
719+
)
699720
if not hasattr(self, "_new_cookie"):
700721
self._new_cookie = (
701722
http.cookies.SimpleCookie()

0 commit comments

Comments
 (0)