From 021c416c18392a111225bc7326063dc4a99a5138 Mon Sep 17 00:00:00 2001
From: Sviatoslav Sydorenko <wk@sydorenko.org.ua>
Date: Thu, 25 Feb 2021 12:06:01 +0100
Subject: [PATCH] Merge branch 'ghsa-v6wp-4m6f-gcjg' into master

This patch fixes an open redirect vulnerability bug in
`aiohttp.web_middlewares.normalize_path_middleware` by
making sure that there's at most one slash at the
beginning of the `Location` header value.

Refs:
* https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html
* https://github.com/aio-libs/aiohttp/security/advisories/GHSA-v6wp-4m6f-gcjg

(cherry picked from commit 76c1fa1315faf48d44b061a1433d0d0c3e4dc12f)
---
 CHANGES/5497.bugfix          |  9 +++++++++
 aiohttp/web_middlewares.py   |  1 +
 tests/test_web_middleware.py | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+)
 create mode 100644 CHANGES/5497.bugfix

diff --git a/CHANGES/5497.bugfix b/CHANGES/5497.bugfix
new file mode 100644
index 00000000000..5cec6d75fe8
--- /dev/null
+++ b/CHANGES/5497.bugfix
@@ -0,0 +1,9 @@
+**(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.
diff --git a/aiohttp/web_middlewares.py b/aiohttp/web_middlewares.py
index 5efad4fa13b..8a8967e8131 100644
--- a/aiohttp/web_middlewares.py
+++ b/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)
diff --git a/tests/test_web_middleware.py b/tests/test_web_middleware.py
index 9b42ba3747e..1a6ea61cdd5 100644
--- a/tests/test_web_middleware.py
+++ b/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):