Skip to content

Commit

Permalink
Merge pull request #3307 from bdarnell/branch6.3
Browse files Browse the repository at this point in the history
Version 6.3.3
  • Loading branch information
bdarnell authored Aug 11, 2023
2 parents e3aa6c5 + 6a9e6fb commit e4d6984
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 21 deletions.
5 changes: 3 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ jobs:
tox_env: py311-full
- python: '3.11.0'
tox_env: py311-full
- python: '3.12.0-alpha - 3.12'
tox_env: py312-full
# py312 testing is disabled in branch6.3; full support is coming in tornado 6.4
#- python: '3.12.0-alpha - 3.12'
# tox_env: py312-full
- python: 'pypy-3.8'
# Pypy is a lot slower due to jit warmup costs, so don't run the
# "full" test config there.
Expand Down
1 change: 1 addition & 0 deletions docs/releases.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Release notes
.. toctree::
:maxdepth: 2

releases/v6.3.3
releases/v6.3.2
releases/v6.3.1
releases/v6.3.0
Expand Down
12 changes: 12 additions & 0 deletions docs/releases/v6.3.3.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
What's new in Tornado 6.3.3
===========================

Aug 11, 2023
------------

Security improvements
~~~~~~~~~~~~~~~~~~~~~

- The ``Content-Length`` header and ``chunked`` ``Transfer-Encoding`` sizes are now parsed
more strictly (according to the relevant RFCs) to avoid potential request-smuggling
vulnerabilities when deployed behind certain proxies.
4 changes: 2 additions & 2 deletions tornado/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.3.2"
version_info = (6, 3, 2, 0)
version = "6.3.3"
version_info = (6, 3, 3, 0)

import importlib
import typing
Expand Down
27 changes: 24 additions & 3 deletions tornado/http1connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ def write_headers(
):
self._expected_content_remaining = 0
elif "Content-Length" in headers:
self._expected_content_remaining = int(headers["Content-Length"])
self._expected_content_remaining = parse_int(headers["Content-Length"])
else:
self._expected_content_remaining = None
# TODO: headers are supposed to be of type str, but we still have some
Expand Down Expand Up @@ -618,7 +618,7 @@ def _read_body(
headers["Content-Length"] = pieces[0]

try:
content_length = int(headers["Content-Length"]) # type: Optional[int]
content_length: Optional[int] = parse_int(headers["Content-Length"])
except ValueError:
# Handles non-integer Content-Length value.
raise httputil.HTTPInputError(
Expand Down Expand Up @@ -668,7 +668,10 @@ async def _read_chunked_body(self, delegate: httputil.HTTPMessageDelegate) -> No
total_size = 0
while True:
chunk_len_str = await self.stream.read_until(b"\r\n", max_bytes=64)
chunk_len = int(chunk_len_str.strip(), 16)
try:
chunk_len = parse_hex_int(native_str(chunk_len_str[:-2]))
except ValueError:
raise httputil.HTTPInputError("invalid chunk size")
if chunk_len == 0:
crlf = await self.stream.read_bytes(2)
if crlf != b"\r\n":
Expand Down Expand Up @@ -842,3 +845,21 @@ async def _server_request_loop(
await asyncio.sleep(0)
finally:
delegate.on_close(self)


DIGITS = re.compile(r"[0-9]+")
HEXDIGITS = re.compile(r"[0-9a-fA-F]+")


def parse_int(s: str) -> int:
"""Parse a non-negative integer from a string."""
if DIGITS.fullmatch(s) is None:
raise ValueError("not an integer: %r" % s)
return int(s)


def parse_hex_int(s: str) -> int:
"""Parse a non-negative hexadecimal integer from a string."""
if HEXDIGITS.fullmatch(s) is None:
raise ValueError("not a hexadecimal integer: %r" % s)
return int(s, 16)
106 changes: 92 additions & 14 deletions tornado/test/httpserver_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
)
from tornado.iostream import IOStream
from tornado.locks import Event
from tornado.log import gen_log
from tornado.log import gen_log, app_log
from tornado.netutil import ssl_options_to_context
from tornado.simple_httpclient import SimpleAsyncHTTPClient
from tornado.testing import (
Expand All @@ -41,6 +41,7 @@
import ssl
import sys
import tempfile
import textwrap
import unittest
import urllib.parse
from io import BytesIO
Expand Down Expand Up @@ -118,7 +119,7 @@ class SSLTestMixin(object):
def get_ssl_options(self):
return dict(
ssl_version=self.get_ssl_version(),
**AsyncHTTPSTestCase.default_ssl_options()
**AsyncHTTPSTestCase.default_ssl_options(),
)

def get_ssl_version(self):
Expand Down Expand Up @@ -558,23 +559,60 @@ def test_chunked_request_uppercase(self):
)
self.assertEqual(json_decode(response), {"foo": ["bar"]})

@gen_test
def test_invalid_content_length(self):
with ExpectLog(
gen_log, ".*Only integer Content-Length is allowed", level=logging.INFO
):
self.stream.write(
b"""\
def test_chunked_request_body_invalid_size(self):
# Only hex digits are allowed in chunk sizes. Python's int() function
# also accepts underscores, so make sure we reject them here.
self.stream.write(
b"""\
POST /echo HTTP/1.1
Content-Length: foo
Transfer-Encoding: chunked
bar
1_a
1234567890abcdef1234567890
0
""".replace(
b"\n", b"\r\n"
)
b"\n", b"\r\n"
)
)
with ExpectLog(gen_log, ".*invalid chunk size", level=logging.INFO):
start_line, headers, response = self.io_loop.run_sync(
lambda: read_stream_body(self.stream)
)
yield self.stream.read_until_close()
self.assertEqual(400, start_line.code)

@gen_test
def test_invalid_content_length(self):
# HTTP only allows decimal digits in content-length. Make sure we don't
# accept anything else, with special attention to things accepted by the
# python int() function (leading plus signs and internal underscores).
test_cases = [
("alphabetic", "foo"),
("leading plus", "+10"),
("internal underscore", "1_0"),
]
for name, value in test_cases:
with self.subTest(name=name), closing(IOStream(socket.socket())) as stream:
with ExpectLog(
gen_log,
".*Only integer Content-Length is allowed",
level=logging.INFO,
):
yield stream.connect(("127.0.0.1", self.get_http_port()))
stream.write(
utf8(
textwrap.dedent(
f"""\
POST /echo HTTP/1.1
Content-Length: {value}
Connection: close
1234567890
"""
).replace("\n", "\r\n")
)
)
yield stream.read_until_close()


class XHeaderTest(HandlerBaseTestCase):
Expand Down Expand Up @@ -1123,6 +1161,46 @@ def body_producer(write):
)


class InvalidOutputContentLengthTest(AsyncHTTPTestCase):
class MessageDelegate(HTTPMessageDelegate):
def __init__(self, connection):
self.connection = connection

def headers_received(self, start_line, headers):
content_lengths = {
"normal": "10",
"alphabetic": "foo",
"leading plus": "+10",
"underscore": "1_0",
}
self.connection.write_headers(
ResponseStartLine("HTTP/1.1", 200, "OK"),
HTTPHeaders({"Content-Length": content_lengths[headers["x-test"]]}),
)
self.connection.write(b"1234567890")
self.connection.finish()

def get_app(self):
class App(HTTPServerConnectionDelegate):
def start_request(self, server_conn, request_conn):
return InvalidOutputContentLengthTest.MessageDelegate(request_conn)

return App()

def test_invalid_output_content_length(self):
with self.subTest("normal"):
response = self.fetch("/", method="GET", headers={"x-test": "normal"})
response.rethrow()
self.assertEqual(response.body, b"1234567890")
for test in ["alphabetic", "leading plus", "underscore"]:
with self.subTest(test):
# This log matching could be tighter but I think I'm already
# over-testing here.
with ExpectLog(app_log, "Uncaught exception"):
with self.assertRaises(HTTPError):
self.fetch("/", method="GET", headers={"x-test": test})


class MaxHeaderSizeTest(AsyncHTTPTestCase):
def get_app(self):
return Application([("/", HelloWorldRequestHandler)])
Expand Down

0 comments on commit e4d6984

Please sign in to comment.