From d5ba4a1695fbf7c6a3e54313262639b198291533 Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Thu, 21 Nov 2024 14:48:05 -0500 Subject: [PATCH 1/3] httputil: Fix quadratic performance of cookie parsing Maliciously-crafted cookies can cause Tornado to spend an unreasonable amount of CPU time and block the event loop. This change replaces the quadratic algorithm with a more efficient one. The implementation is copied from the Python 3.13 standard library (the previous one was from Python 3.5). Fixes CVE-2024-52804 See CVE-2024-7592 for a similar vulnerability in cpython. Thanks to github.com/kexinoh for the report. --- tornado/httputil.py | 38 ++++++++--------------------- tornado/test/httputil_test.py | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 28 deletions(-) diff --git a/tornado/httputil.py b/tornado/httputil.py index 9ce992d82b..ebdc8059c1 100644 --- a/tornado/httputil.py +++ b/tornado/httputil.py @@ -1057,15 +1057,20 @@ def qs_to_qsl(qs: Dict[str, List[AnyStr]]) -> Iterable[Tuple[str, AnyStr]]: yield (k, v) -_OctalPatt = re.compile(r"\\[0-3][0-7][0-7]") -_QuotePatt = re.compile(r"[\\].") -_nulljoin = "".join +_unquote_sub = re.compile(r"\\(?:([0-3][0-7][0-7])|(.))").sub + + +def _unquote_replace(m: re.Match) -> str: + if m[1]: + return chr(int(m[1], 8)) + else: + return m[2] def _unquote_cookie(s: str) -> str: """Handle double quotes and escaping in cookie values. - This method is copied verbatim from the Python 3.5 standard + This method is copied verbatim from the Python 3.13 standard library (http.cookies._unquote) so we don't have to depend on non-public interfaces. """ @@ -1086,30 +1091,7 @@ def _unquote_cookie(s: str) -> str: # \012 --> \n # \" --> " # - i = 0 - n = len(s) - res = [] - while 0 <= i < n: - o_match = _OctalPatt.search(s, i) - q_match = _QuotePatt.search(s, i) - if not o_match and not q_match: # Neither matched - res.append(s[i:]) - break - # else: - j = k = -1 - if o_match: - j = o_match.start(0) - if q_match: - k = q_match.start(0) - if q_match and (not o_match or k < j): # QuotePatt matched - res.append(s[i:k]) - res.append(s[k + 1]) - i = k + 2 - else: # OctalPatt matched - res.append(s[i:j]) - res.append(chr(int(s[j + 1 : j + 4], 8))) - i = j + 4 - return _nulljoin(res) + return _unquote_sub(_unquote_replace, s) def parse_cookie(cookie: str) -> Dict[str, str]: diff --git a/tornado/test/httputil_test.py b/tornado/test/httputil_test.py index 6d618839e0..975900aa9c 100644 --- a/tornado/test/httputil_test.py +++ b/tornado/test/httputil_test.py @@ -560,3 +560,49 @@ def test_invalid_cookies(self): self.assertEqual( parse_cookie(" = b ; ; = ; c = ; "), {"": "b", "c": ""} ) + + def test_unquote(self): + # Copied from + # https://github.com/python/cpython/blob/dc7a2b6522ec7af41282bc34f405bee9b306d611/Lib/test/test_http_cookies.py#L62 + cases = [ + (r'a="b=\""', 'b="'), + (r'a="b=\\"', "b=\\"), + (r'a="b=\="', "b=="), + (r'a="b=\n"', "b=n"), + (r'a="b=\042"', 'b="'), + (r'a="b=\134"', "b=\\"), + (r'a="b=\377"', "b=\xff"), + (r'a="b=\400"', "b=400"), + (r'a="b=\42"', "b=42"), + (r'a="b=\\042"', "b=\\042"), + (r'a="b=\\134"', "b=\\134"), + (r'a="b=\\\""', 'b=\\"'), + (r'a="b=\\\042"', 'b=\\"'), + (r'a="b=\134\""', 'b=\\"'), + (r'a="b=\134\042"', 'b=\\"'), + ] + for encoded, decoded in cases: + with self.subTest(encoded): + c = parse_cookie(encoded) + self.assertEqual(c["a"], decoded) + + def test_unquote_large(self): + # Adapted from + # https://github.com/python/cpython/blob/dc7a2b6522ec7af41282bc34f405bee9b306d611/Lib/test/test_http_cookies.py#L87 + # Modified from that test because we handle semicolons differently from the stdlib. + # + # This is a performance regression test: prior to improvements in Tornado 6.4.2, this test + # would take over a minute with n= 100k. Now it runs in tens of milliseconds. + n = 100000 + for encoded in r"\\", r"\134": + with self.subTest(encoded): + start = time.time() + data = 'a="b=' + encoded * n + '"' + value = parse_cookie(data)["a"] + end = time.time() + self.assertEqual(value[:3], "b=\\") + self.assertEqual(value[-3:], "\\\\\\") + self.assertEqual(len(value), n + 2) + + # Very loose performance check to avoid false positives + self.assertLess(end - start, 1, "Test took too long") From bc7df6bafdec61155e7bf385081feb205463857d Mon Sep 17 00:00:00 2001 From: Colin Watson Date: Sun, 18 Aug 2024 18:58:11 +0100 Subject: [PATCH 2/3] Fix tests with Twisted 24.7.0 `twisted.internet.defer.returnValue` was needed on Python 2, but on Python 3 a simple `return` statement works fine. Twisted 24.7.0 deprecated the former, causing `tornado.test.twisted_test.ConvertDeferredTest.test_success` to fail. --- tornado/test/twisted_test.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tornado/test/twisted_test.py b/tornado/test/twisted_test.py index 7f983a73e7..36a541a7a8 100644 --- a/tornado/test/twisted_test.py +++ b/tornado/test/twisted_test.py @@ -18,10 +18,7 @@ from tornado.testing import AsyncTestCase, gen_test try: - from twisted.internet.defer import ( # type: ignore - inlineCallbacks, - returnValue, - ) + from twisted.internet.defer import inlineCallbacks # type: ignore have_twisted = True except ImportError: @@ -43,7 +40,7 @@ def fn(): # inlineCallbacks doesn't work with regular functions; # must have a yield even if it's unreachable. yield - returnValue(42) + return 42 res = yield fn() self.assertEqual(res, 42) From a5ecfab15e52202a46d34638aad93cddca86d87b Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Thu, 21 Nov 2024 16:26:44 -0500 Subject: [PATCH 3/3] Bump version to 6.4.2 --- docs/releases.rst | 1 + docs/releases/v6.4.2.rst | 12 ++++++++++++ tornado/__init__.py | 4 ++-- 3 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 docs/releases/v6.4.2.rst diff --git a/docs/releases.rst b/docs/releases.rst index 8a0fad4c2f..5c7a106d87 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -4,6 +4,7 @@ Release notes .. toctree:: :maxdepth: 2 + releases/v6.4.2 releases/v6.4.1 releases/v6.4.0 releases/v6.3.3 diff --git a/docs/releases/v6.4.2.rst b/docs/releases/v6.4.2.rst new file mode 100644 index 0000000000..0dc567d171 --- /dev/null +++ b/docs/releases/v6.4.2.rst @@ -0,0 +1,12 @@ +What's new in Tornado 6.4.2 +=========================== + +Nov 21, 2024 +------------ + +Security Improvements +~~~~~~~~~~~~~~~~~~~~~ + +- Parsing of the cookie header is now much more efficient. The older algorithm sometimes had + quadratic performance which allowed for a denial-of-service attack in which the server would spend + excessive CPU time parsing cookies and block the event loop. This change fixes CVE-2024-7592. \ No newline at end of file diff --git a/tornado/__init__.py b/tornado/__init__.py index f542de3558..91e4cdec1c 100644 --- a/tornado/__init__.py +++ b/tornado/__init__.py @@ -22,8 +22,8 @@ # is zero for an official release, positive for a development branch, # or negative for a release candidate or beta (after the base version # number has been incremented) -version = "6.4.1" -version_info = (6, 4, 0, 1) +version = "6.4.2" +version_info = (6, 4, 2, 0) import importlib import typing