From: Ben Darnell Date: Fri, 6 Mar 2026 19:50:25 +0000 (-0500) Subject: web: Validate characters in all cookie attributes. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=459e1c3d3b;p=thirdparty%2Ftornado.git 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. --- diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py index 27df3f7a..a7886795 100644 --- a/tornado/test/web_test.py +++ b/tornado/test/web_test.py @@ -1,3 +1,5 @@ +import http + from tornado.concurrent import Future from tornado import gen from tornado.escape import ( @@ -292,11 +294,67 @@ class CookieTest(WebTestCase): self.set_cookie("unicode_args", "blah", domain="foo.com", path="/foo") class SetCookieSpecialCharHandler(RequestHandler): + # "Special" characters are allowed in cookie values, but trigger special quoting. def get(self): self.set_cookie("equals", "a=b") self.set_cookie("semicolon", "a;b") self.set_cookie("quote", 'a"b') + class SetCookieForbiddenCharHandler(RequestHandler): + def get(self): + # Control characters and semicolons raise errors in cookie names and attributes + # (but not values, which are tested in SetCookieSpecialCharHandler) + for char in list(map(chr, range(0x20))) + [chr(0x7F), ";"]: + try: + self.set_cookie("foo" + char, "bar") + self.write( + "Didn't get expected exception for char %r in name\n" % char + ) + except http.cookies.CookieError as e: + if "Invalid cookie attribute name" not in str(e): + self.write( + "unexpected exception for char %r in name: %s\n" + % (char, e) + ) + + try: + self.set_cookie("foo", "bar", domain="example" + char + ".com") + self.write( + "Didn't get expected exception for char %r in domain\n" + % char + ) + except http.cookies.CookieError as e: + if "Invalid cookie attribute domain" not in str(e): + self.write( + "unexpected exception for char %r in domain: %s\n" + % (char, e) + ) + + try: + self.set_cookie("foo", "bar", path="/" + char) + self.write( + "Didn't get expected exception for char %r in path\n" % char + ) + except http.cookies.CookieError as e: + if "Invalid cookie attribute path" not in str(e): + self.write( + "unexpected exception for char %r in path: %s\n" + % (char, e) + ) + + try: + self.set_cookie("foo", "bar", samesite="a" + char) + self.write( + "Didn't get expected exception for char %r in samesite\n" + % char + ) + except http.cookies.CookieError as e: + if "Invalid cookie attribute samesite" not in str(e): + self.write( + "unexpected exception for char %r in samesite: %s\n" + % (char, e) + ) + class SetCookieOverwriteHandler(RequestHandler): def get(self): self.set_cookie("a", "b", domain="example.com") @@ -331,6 +389,7 @@ class CookieTest(WebTestCase): ("/get", GetCookieHandler), ("/set_domain", SetCookieDomainHandler), ("/special_char", SetCookieSpecialCharHandler), + ("/forbidden_char", SetCookieForbiddenCharHandler), ("/set_overwrite", SetCookieOverwriteHandler), ("/set_max_age", SetCookieMaxAgeHandler), ("/set_expires_days", SetCookieExpiresDaysHandler), @@ -388,6 +447,12 @@ class CookieTest(WebTestCase): response = self.fetch("/get", headers={"Cookie": header}) self.assertEqual(response.body, utf8(expected)) + def test_set_cookie_forbidden_char(self): + response = self.fetch("/forbidden_char") + self.assertEqual(response.code, 200) + self.maxDiff = 10000 + self.assertMultiLineEqual(to_unicode(response.body), "") + def test_set_cookie_overwrite(self): response = self.fetch("/set_overwrite") headers = response.headers.get_list("Set-Cookie") diff --git a/tornado/web.py b/tornado/web.py index 2351afdb..ec7ec3f5 100644 --- a/tornado/web.py +++ b/tornado/web.py @@ -693,9 +693,30 @@ class RequestHandler: # The cookie library only accepts type str, in both python 2 and 3 name = escape.native_str(name) value = escape.native_str(value) - if re.search(r"[\x00-\x20]", name + value): - # Don't let us accidentally inject bad stuff + if re.search(r"[\x00-\x20]", value): + # Legacy check for control characters in cookie values. This check is no longer needed + # since the cookie library escapes these characters correctly now. It will be removed + # in the next feature release. raise ValueError(f"Invalid cookie {name!r}: {value!r}") + for attr_name, attr_value in [ + ("name", name), + ("domain", domain), + ("path", path), + ("samesite", samesite), + ]: + # Cookie attributes may not contain control characters or semicolons (except when + # escaped in the value). A check for control characters was added to the http.cookies + # library in a Feb 2026 security release; as of March it still does not check for + # semicolons. + # + # When a semicolon check is added to the standard library (and the release has had time + # for adoption), this check may be removed, but be mindful of the fact that this may + # change the timing of the exception (to the generation of the Set-Cookie header in + # flush()). We m + if attr_value is not None and re.search(r"[\x00-\x20\x3b\x7f]", attr_value): + raise http.cookies.CookieError( + f"Invalid cookie attribute {attr_name}={attr_value!r} for cookie {name!r}" + ) if not hasattr(self, "_new_cookie"): self._new_cookie = ( http.cookies.SimpleCookie()