Merge pull request #3704 from bdarnell/security-6.5.8

Security 6.5.8

Author: bdarnell

docs/releases.rst

@@ -4,6 +4,7 @@ Release notes
 .. toctree::
    :maxdepth: 2
 
+   releases/v6.5.8
    releases/v6.5.7
    releases/v6.5.6
    releases/v6.5.5

docs/releases/v6.5.8.rst

@@ -0,0 +1,26 @@
+What's new in Tornado 6.5.8
+===========================
+
+Aug 6, 2026
+-----------
+
+Security fixes
+~~~~~~~~~~~~~~
+
+- Form-encoded ``POST`` bodies are now subject to a limit of 1000 arguments by default. This
+  prevents a CPU and memory denial of service attack. This limit can be overridden via the
+  `.set_parse_body_config` function. Thanks to `Arpit Jain <https://github.com/arpitjain099>`_ 
+  for reporting this issue.
+- Multipart parsing now rejects requests with an excessive number of parts earlier in the parsing
+  process, limiting memory consumption. Thanks to `afldl <https://github.com/afldl>`_ for
+  reporting this issue.
+- The deprecated mixed-case arguments to `.RequestHandler.set_cookie` now enforce the same
+  restrictions on invalid characters that were introduced in Tornado 6.5.5 for the standard
+  lowercase arguments. Thanks to `sec-reex <https://github.com/sec-reex>`_ and
+  `Arpit Jain <https://github.com/arpitjain099>`_ for reporting this issue.
+
+Deprecations
+~~~~~~~~~~~~
+
+- The `.OpenIdMixin` class is deprecated and will be removed in Tornado 6.7. OpenID 2.0 is no
+  longer widely supported by identity providers.

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.5.7"
-version_info = (6, 5, 7, 0)
+version = "6.5.8"
+version_info = (6, 5, 8, 0)
 
 import importlib
 import typing

tornado/auth.py

@@ -98,8 +98,19 @@ class OpenIdMixin:
     Class attributes:
 
     * ``_OPENID_ENDPOINT``: the identity provider's URI.
+
+    .. deprecated:: 6.6
+        OpenID 2.0 is no longer widely supported by identity providers.
+        This class will be removed in Tornado 6.7.
     """
 
+    def __init__(self) -> None:
+        warnings.warn(
+            "OpenIdMixin is deprecated and will be removed in Tornado 6.7",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+
     def authenticate_redirect(
         self,
         callback_uri: Optional[str] = None,

tornado/escape.py

@@ -171,21 +171,33 @@ def url_unescape(
 
 
 def parse_qs_bytes(
-    qs: Union[str, bytes], keep_blank_values: bool = False, strict_parsing: bool = False
+    qs: Union[str, bytes],
+    keep_blank_values: bool = False,
+    strict_parsing: bool = False,
+    *,
+    max_num_fields: Optional[int] = None,
 ) -> Dict[str, List[bytes]]:
     """Parses a query string like urlparse.parse_qs,
     but takes bytes and returns the values as byte strings.
 
     Keys still become type str (interpreted as latin1 in python3!)
     because it's too painful to keep them as byte strings in
     python3 and in practice they're nearly always ascii anyway.
+
+    .. versionadded:: 6.5.8
+       The ``max_num_fields`` argument. ValueError is raised if this limit is exceeded.
     """
     # This is gross, but python3 doesn't give us another way.
     # Latin1 is the universal donor of character encodings.
     if isinstance(qs, bytes):
         qs = qs.decode("latin1")
     result = urllib.parse.parse_qs(
-        qs, keep_blank_values, strict_parsing, encoding="latin1", errors="strict"
+        qs,
+        keep_blank_values,
+        strict_parsing,
+        encoding="latin1",
+        errors="strict",
+        max_num_fields=max_num_fields,
     )
     encoded = {}
     for k, v in result.items():

tornado/httputil.py

@@ -37,7 +37,6 @@
 from tornado.escape import native_str, parse_qs_bytes, utf8, to_unicode
 from tornado.util import ObjectDict, unicode_type
 
-
 # responses is unused in this file, but we re-export it to other files.
 # Reference it so pyflakes doesn't complain.
 responses
@@ -948,6 +947,23 @@ class ParseMultipartConfig:
     """
 
 
+@dataclasses.dataclass
+class ParseUrlEncodedConfig:
+    """This class configures the parsing of ``application/x-www-form-urlencoded`` request bodies.
+
+    Its primary purpose is to place limits on the size and complexity of request messages
+    to avoid potential denial-of-service attacks.
+
+    .. versionadded:: 6.5.8
+    """
+
+    max_arguments: int = 1000
+    """The maximum number of arguments accepted in a urlencoded request.
+
+    Each ``<input>`` element in an HTML form corresponds to at least one argument.
+    """
+
+
 @dataclasses.dataclass
 class ParseBodyConfig:
     """This class configures the parsing of request bodies.
@@ -958,6 +974,9 @@ class ParseBodyConfig:
     multipart: ParseMultipartConfig = dataclasses.field(
         default_factory=ParseMultipartConfig
     )
+    urlencoded: ParseUrlEncodedConfig = dataclasses.field(
+        default_factory=ParseUrlEncodedConfig
+    )
     """Configuration for ``multipart/form-data`` request bodies."""
 
 
@@ -1016,7 +1035,11 @@ def parse_body_arguments(
             )
         try:
             # real charset decoding will happen in RequestHandler.decode_argument()
-            uri_arguments = parse_qs_bytes(body, keep_blank_values=True)
+            uri_arguments = parse_qs_bytes(
+                body,
+                keep_blank_values=True,
+                max_num_fields=config.urlencoded.max_arguments,
+            )
         except Exception as e:
             raise HTTPInputError("Invalid x-www-form-urlencoded body: %s" % e) from e
         for name, values in uri_arguments.items():
@@ -1078,7 +1101,9 @@ def parse_multipart_form_data(
     final_boundary_index = data.rfind(b"--" + boundary + b"--")
     if final_boundary_index == -1:
         raise HTTPInputError("Invalid multipart/form-data: no final boundary found")
-    parts = data[:final_boundary_index].split(b"--" + boundary + b"\r\n")
+    parts = data[:final_boundary_index].split(
+        b"--" + boundary + b"\r\n", config.max_parts + 1
+    )
     if len(parts) > config.max_parts:
         raise HTTPInputError("multipart/form-data has too many parts")
     for part in parts:

tornado/test/auth_test.py

@@ -18,7 +18,8 @@
 from tornado.httpclient import HTTPClientError
 from tornado.httputil import url_concat
 from tornado.log import app_log
-from tornado.testing import AsyncHTTPTestCase, ExpectLog
+from tornado.testing import AsyncHTTPTestCase, ExpectLog, setup_with_context_manager
+from tornado.test.util import ignore_deprecation
 from tornado.web import RequestHandler, Application, HTTPError
 
 try:
@@ -279,12 +280,46 @@ def get(self):
         self.write(dict(screen_name="foo", name="Foo"))
 
 
+class OpenIDAuthTest(AsyncHTTPTestCase):
+    def setUp(self):
+        setup_with_context_manager(self, ignore_deprecation())
+        return super().setUp()
+
+    def get_app(self):
+        return Application(
+            [
+                ("/openid/client/login", OpenIdClientLoginHandler, dict(test=self)),
+                ("/openid/server/authenticate", OpenIdServerAuthenticateHandler),
+            ],
+            http_client=self.http_client,
+        )
+
+    def test_openid_redirect(self):
+        with ignore_deprecation():
+            response = self.fetch("/openid/client/login", follow_redirects=False)
+            self.assertEqual(response.code, 302)
+            self.assertIn("/openid/server/authenticate?", response.headers["Location"])
+
+    def test_openid_get_user(self):
+        for i in range(2):
+            with self.subTest(i=i):
+                with ignore_deprecation():
+                    response = self.fetch(
+                        "/openid/client/login?openid.mode=blah"
+                        "&openid.ns.ax=http://openid.net/srv/ax/1.0"
+                        "&openid.ax.type.email=http://axschema.org/contact/email"
+                        "&openid.ax.value.email=foo@example.com"
+                    )
+                response.rethrow()
+                parsed = json_decode(response.body)
+                self.assertEqual(parsed["email"], "foo@example.com")
+
+
 class AuthTest(AsyncHTTPTestCase):
     def get_app(self):
         return Application(
             [
                 # test endpoints
-                ("/openid/client/login", OpenIdClientLoginHandler, dict(test=self)),
                 (
                     "/oauth10/client/login",
                     OAuth1ClientLoginHandler,
@@ -329,7 +364,6 @@ def get_app(self):
                     dict(test=self),
                 ),
                 # simulated servers
-                ("/openid/server/authenticate", OpenIdServerAuthenticateHandler),
                 ("/oauth1/server/request_token", OAuth1ServerRequestTokenHandler),
                 ("/oauth1/server/access_token", OAuth1ServerAccessTokenHandler),
                 ("/facebook/server/access_token", FacebookServerAccessTokenHandler),
@@ -348,24 +382,6 @@ def get_app(self):
             facebook_secret="test_facebook_secret",
         )
 
-    def test_openid_redirect(self):
-        response = self.fetch("/openid/client/login", follow_redirects=False)
-        self.assertEqual(response.code, 302)
-        self.assertIn("/openid/server/authenticate?", response.headers["Location"])
-
-    def test_openid_get_user(self):
-        for i in range(2):
-            with self.subTest(i=i):
-                response = self.fetch(
-                    "/openid/client/login?openid.mode=blah"
-                    "&openid.ns.ax=http://openid.net/srv/ax/1.0"
-                    "&openid.ax.type.email=http://axschema.org/contact/email"
-                    "&openid.ax.value.email=foo@example.com"
-                )
-                response.rethrow()
-                parsed = json_decode(response.body)
-                self.assertEqual(parsed["email"], "foo@example.com")
-
     def test_oauth10_redirect(self):
         response = self.fetch("/oauth10/client/login", follow_redirects=False)
         self.assertEqual(response.code, 302)

tornado/test/httpclient_test.py

@@ -795,7 +795,12 @@ def test_strip_headers_on_redirect(self):
                     "/redirect?url=%s&status=302" % self.get_url2("/echo_headers")
                 )
                 if url_creds:
-                    url = url.replace("http://", "http://%s@" % url_creds)
+                    # Only add credentials to the outer URL being fetched, not to the
+                    # "url" query parameter (the redirect target), which also starts
+                    # with "http://". Otherwise the redirect's Location header would
+                    # carry its own explicit credentials for the new origin, which
+                    # libcurl legitimately honors instead of stripping.
+                    url = url.replace("http://", "http://%s@" % url_creds, 1)
                 response = self.fetch(**dict(path=url) | kwargs)
                 response.rethrow()
                 echoed_headers = json_decode(response.body)
@@ -809,17 +814,27 @@ def test_strip_headers_on_redirect(self):
                     "/redirect?url=%s&status=302" % self.get_url("/echo_headers")
                 )
                 if url_creds:
-                    url = url.replace("http://", "http://%s@" % url_creds)
+                    url = url.replace("http://", "http://%s@" % url_creds, 1)
                 response = self.fetch(**dict(path=url) | kwargs)
                 response.rethrow()
                 echoed_headers = json_decode(response.body)
                 # Confirm that non-auth headers are getting through
                 self.assertIn("User-Agent", echoed_headers)
-                # Auth headers are not stripped when the redirect is same-origin.
-                # Each of our tests uses one of these headers, but not both.
-                self.assertTrue(
-                    "Authorization" in echoed_headers or "Cookie" in echoed_headers
-                )
+                if name == "credentials in URL":
+                    # Some libcurl versions (known regression as of 8.20/8.21,
+                    # still present as of curl's git master) drop credentials
+                    # embedded in the URL across a same-origin redirect whose
+                    # Location header is an absolute URL, even though they
+                    # should be preserved. This isn't a security concern
+                    # (nothing is leaked to another origin), so just don't
+                    # assert on it either way here.
+                    pass
+                else:
+                    # Auth headers are not stripped when the redirect is same-origin.
+                    # Each of our tests uses one of these headers, but not both.
+                    self.assertTrue(
+                        "Authorization" in echoed_headers or "Cookie" in echoed_headers
+                    )
 
 
 class RequestProxyTest(unittest.TestCase):

tornado/test/httputil_test.py

@@ -1,4 +1,5 @@
 from tornado.httputil import (
+    parse_body_arguments,
     url_concat,
     parse_multipart_form_data,
     HTTPHeaders,
@@ -95,6 +96,23 @@ def test_parsing(self):
         self.assertIn(("b", "2"), qsl)
 
 
+class UrlEncodedDataTest(unittest.TestCase):
+    def test_urlencoded_data(self):
+        data = b"a=1&b=2&a=3"
+        args, files = form_data_args()
+        parse_body_arguments("application/x-www-form-urlencoded", data, args, files)
+        self.assertEqual(args["a"], [b"1", b"3"])
+        self.assertEqual(args["b"], [b"2"])
+        self.assertEqual(files, {})
+
+    def test_max_arguments(self):
+        data = b"".join(b"a=1&" for _ in range(1001))
+        args, files = form_data_args()
+        with self.assertRaises(HTTPInputError) as cm:
+            parse_body_arguments("application/x-www-form-urlencoded", data, args, files)
+        self.assertIn("Max number of fields exceeded", str(cm.exception))
+
+
 class MultipartFormDataTest(unittest.TestCase):
     def test_file_upload(self):
         data = b"""\

tornado/test/web_test.py

@@ -329,6 +329,18 @@ def get(self):
                                 "unexpected exception for char %r in domain: %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)