CHANGES.rst

@@ -14,6 +14,34 @@ Changelog
 
 .. towncrier release notes start
 
+3.7.3 (2021-02-25)
+==================
+
+Bugfixes
+--------
+
+- **(SECURITY BUG)** Started preventing open redirects in the
+  ``aiohttp.web.normalize_path_middleware`` middleware. For
+  more details, see
+  https://github.com/aio-libs/aiohttp/security/advisories/GHSA-v6wp-4m6f-gcjg.
+
+  Thanks to `Beast Glatisant <https://github.com/g147>`__ for
+  finding the first instance of this issue and `Jelmer Vernooij
+  <https://jelmer.uk/>`__ for reporting and tracking it down
+  in aiohttp.
+  `#5497 <https://github.com/aio-libs/aiohttp/issues/5497>`_
+- Fix interpretation difference of the pure-Python and the Cython-based
+  HTTP parsers construct a ``yarl.URL`` object for HTTP request-target.
+
+  Before this fix, the Python parser would turn the URI's absolute-path
+  for ``//some-path`` into ``/`` while the Cython code preserved it as
+  ``//some-path``. Now, both do the latter.
+  `#5498 <https://github.com/aio-libs/aiohttp/issues/5498>`_
+
+
+----
+
+
 3.7.3 (2020-11-18)
 ==================
 

CONTRIBUTORS.txt

@@ -109,6 +109,7 @@ Florian Scheffler
 Frederik Gladhorn
 Frederik Peter Aalund
 Gabriel Tremblay
+Gary Wilson Jr.
 Gennady Andreyev
 Georges Dubus
 Greg Holt

aiohttp/__init__.py

@@ -1,4 +1,4 @@
-__version__ = "3.7.3"
+__version__ = "3.7.4"
 
 from typing import Tuple
 

aiohttp/http_parser.py

@@ -498,6 +498,9 @@ def parse_message(self, lines: List[bytes]) -> Any:
                 "Status line is too long", str(self.max_line_size), str(len(path))
             )
 
+        path_part, _hash_separator, url_fragment = path.partition("#")
+        path_part, _question_mark_separator, qs_part = path_part.partition("?")
+
         # method
         if not METHRE.match(method):
             raise BadStatusLine(method)
@@ -538,7 +541,16 @@ def parse_message(self, lines: List[bytes]) -> Any:
             compression,
             upgrade,
             chunked,
-            URL(path),
+            # NOTE: `yarl.URL.build()` is used to mimic what the Cython-based
+            # NOTE: parser does, otherwise it results into the same
+            # NOTE: HTTP Request-Line input producing different
+            # NOTE: `yarl.URL()` objects
+            URL.build(
+                path=path_part,
+                query_string=qs_part,
+                fragment=url_fragment,
+                encoded=True,
+            ),
         )
 
 

aiohttp/web_middlewares.py

@@ -102,6 +102,7 @@ async def impl(request: Request, handler: _Handler) -> StreamResponse:
                 paths_to_check.append(merged_slashes[:-1])
 
             for path in paths_to_check:
+                path = re.sub("^//+", "/", path)  # SECURITY: GHSA-v6wp-4m6f-gcjg
                 resolves, request = await _check_request_resolves(request, path)
                 if resolves:
                     raise redirect_class(request.raw_path + query)

docs/client_reference.rst

@@ -129,7 +129,7 @@ The client session supports the context manager protocol for self closing.
       requests where you need to handle responses with status 400 or
       higher.
 
-   :param timeout: a :class:`ClientTimeout` settings structure, 5min
+   :param timeout: a :class:`ClientTimeout` settings structure, 300 seconds (5min)
         total timeout by default.
 
       .. versionadded:: 3.3
@@ -150,9 +150,6 @@ The client session supports the context manager protocol for self closing.
 
          Use ``timeout`` parameter instead.
 
-   :param timeout: a :class:`ClientTimeout` settings structure, 300 seconds (5min)
-        total timeout by default.
-
    :param bool connector_owner:
 
       Close connector instance on session closing.

requirements/base.txt

@@ -6,7 +6,7 @@ async-timeout==3.0.1
 attrs==20.3.0
 brotlipy==0.7.0
 cchardet==2.1.7
-chardet==3.0.4
+chardet==4.0.0
 gunicorn==20.0.4
 idna-ssl==1.1.0; python_version<"3.7"
 typing_extensions==3.7.4.3

requirements/doc.txt

@@ -1,5 +1,5 @@
 aiohttp-theme==0.1.6
-pygments==2.7.2
+pygments==2.7.3
 sphinx==3.3.1
 sphinxcontrib-asyncio==0.3.0
 sphinxcontrib-blockdiag==2.0.0

requirements/lint.txt

@@ -3,4 +3,4 @@ flake8==3.8.4
 flake8-pyi==20.10.0
 isort==5.6.4
 mypy==0.790; implementation_name=="cpython"
-pre-commit==2.8.2
+pre-commit==2.9.3

requirements/multidict.txt

@@ -1 +1 @@
-multidict==5.0.2
+multidict==5.1.0

tests/test_http_parser.py

@@ -528,6 +528,7 @@ def test_http_request_parser_two_slashes(parser) -> None:
 
     assert msg.method == "GET"
     assert msg.path == "//path"
+    assert msg.url.path == "//path"
     assert msg.version == (1, 1)
     assert not msg.should_close
     assert msg.compression is None

tests/test_web_middleware.py

@@ -1,4 +1,5 @@
 import re
+from typing import Any
 
 import pytest
 from yarl import URL
@@ -352,6 +353,38 @@ async def test_cannot_remove_and_add_slash(self) -> None:
         with pytest.raises(AssertionError):
             web.normalize_path_middleware(append_slash=True, remove_slash=True)
 
+    @pytest.mark.parametrize(
+        ["append_slash", "remove_slash"],
+        [
+            (True, False),
+            (False, True),
+            (False, False),
+        ],
+    )
+    async def test_open_redirects(
+        self, append_slash: bool, remove_slash: bool, aiohttp_client: Any
+    ) -> None:
+        async def handle(request: web.Request) -> web.StreamResponse:
+            pytest.fail(
+                msg="Security advisory 'GHSA-v6wp-4m6f-gcjg' test handler "
+                "matched unexpectedly",
+                pytrace=False,
+            )
+
+        app = web.Application(
+            middlewares=[
+                web.normalize_path_middleware(
+                    append_slash=append_slash, remove_slash=remove_slash
+                )
+            ]
+        )
+        app.add_routes([web.get("/", handle), web.get("/google.com", handle)])
+        client = await aiohttp_client(app, server_kwargs={"skip_url_asserts": True})
+        resp = await client.get("//google.com", allow_redirects=False)
+        assert resp.status == 308
+        assert resp.headers["Location"] == "/google.com"
+        assert resp.url.query == URL("//google.com").query
+
 
 async def test_old_style_middleware(loop, aiohttp_client) -> None:
     async def handler(request):