Skip to content

Commit 021c416

Browse files
committed
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 * GHSA-v6wp-4m6f-gcjg (cherry picked from commit 76c1fa1315faf48d44b061a1433d0d0c3e4dc12f)
1 parent 4ed7c25 commit 021c416

File tree

3 files changed

+43
-0
lines changed

3 files changed

+43
-0
lines changed

CHANGES/5497.bugfix

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
**(SECURITY BUG)** Started preventing open redirects in the
2+
``aiohttp.web.normalize_path_middleware`` middleware. For
3+
more details, see
4+
https://github.com/aio-libs/aiohttp/security/advisories/GHSA-v6wp-4m6f-gcjg.
5+
6+
Thanks to `Beast Glatisant <https://github.com/g147>`__ for
7+
finding the first instance of this issue and `Jelmer Vernooij
8+
<https://jelmer.uk/>`__ for reporting and tracking it down
9+
in aiohttp.

aiohttp/web_middlewares.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ async def impl(request: Request, handler: _Handler) -> StreamResponse:
102102
paths_to_check.append(merged_slashes[:-1])
103103

104104
for path in paths_to_check:
105+
path = re.sub("^//+", "/", path) # SECURITY: GHSA-v6wp-4m6f-gcjg
105106
resolves, request = await _check_request_resolves(request, path)
106107
if resolves:
107108
raise redirect_class(request.raw_path + query)

tests/test_web_middleware.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import re
2+
from typing import Any
23

34
import pytest
45
from yarl import URL
@@ -352,6 +353,38 @@ async def test_cannot_remove_and_add_slash(self) -> None:
352353
with pytest.raises(AssertionError):
353354
web.normalize_path_middleware(append_slash=True, remove_slash=True)
354355

356+
@pytest.mark.parametrize(
357+
["append_slash", "remove_slash"],
358+
[
359+
(True, False),
360+
(False, True),
361+
(False, False),
362+
],
363+
)
364+
async def test_open_redirects(
365+
self, append_slash: bool, remove_slash: bool, aiohttp_client: Any
366+
) -> None:
367+
async def handle(request: web.Request) -> web.StreamResponse:
368+
pytest.fail(
369+
msg="Security advisory 'GHSA-v6wp-4m6f-gcjg' test handler "
370+
"matched unexpectedly",
371+
pytrace=False,
372+
)
373+
374+
app = web.Application(
375+
middlewares=[
376+
web.normalize_path_middleware(
377+
append_slash=append_slash, remove_slash=remove_slash
378+
)
379+
]
380+
)
381+
app.add_routes([web.get("/", handle), web.get("/google.com", handle)])
382+
client = await aiohttp_client(app, server_kwargs={"skip_url_asserts": True})
383+
resp = await client.get("//google.com", allow_redirects=False)
384+
assert resp.status == 308
385+
assert resp.headers["Location"] == "/google.com"
386+
assert resp.url.query == URL("//google.com").query
387+
355388

356389
async def test_old_style_middleware(loop, aiohttp_client) -> None:
357390
async def handler(request):

0 commit comments

Comments
 (0)