From a9ce82c00a756d415130dbc1c85b78c584cba3b2 Mon Sep 17 00:00:00 2001 From: Geoff Brown Date: Mon, 7 Feb 2022 14:12:55 -0700 Subject: [PATCH 1/4] Harden trailing slashes redirect --- pollbot/middlewares.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pollbot/middlewares.py b/pollbot/middlewares.py index 249cb80..485857b 100644 --- a/pollbot/middlewares.py +++ b/pollbot/middlewares.py @@ -60,8 +60,12 @@ async def handle_any(request, response): async def handle_404(request, response): if 'json' not in response.headers['Content-Type']: + # This traling slash redirect has caused security issues. + # If it continues to be problematic, consider: + # - only redirect "/v1/.../"? + # - remove the redirect entirely; use duplicate routes instead, in app.py if request.path.endswith('/'): - return web.HTTPFound('/' + request.path.strip('/')) + return web.HTTPFound('/' + request.path.strip('/ \r\n')) return web.json_response({ "status": 404, "message": "Page '{}' not found".format(request.path) From c4f96130003284122b1774220e3cce29a02ff6b3 Mon Sep 17 00:00:00 2001 From: Geoff Brown Date: Mon, 7 Feb 2022 14:34:42 -0700 Subject: [PATCH 2/4] Use string.whitespace --- pollbot/middlewares.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pollbot/middlewares.py b/pollbot/middlewares.py index 485857b..5816b0b 100644 --- a/pollbot/middlewares.py +++ b/pollbot/middlewares.py @@ -1,6 +1,7 @@ import logging from aiohttp import web import os +import string logger = logging.getLogger(__package__) @@ -65,7 +66,7 @@ async def handle_404(request, response): # - only redirect "/v1/.../"? # - remove the redirect entirely; use duplicate routes instead, in app.py if request.path.endswith('/'): - return web.HTTPFound('/' + request.path.strip('/ \r\n')) + return web.HTTPFound('/' + request.path.strip('/'+string.whitespace)) return web.json_response({ "status": 404, "message": "Page '{}' not found".format(request.path) From 7749e7de856064a7a652431cb8b016e14aec9982 Mon Sep 17 00:00:00 2001 From: Geoff Brown Date: Mon, 7 Feb 2022 15:33:56 -0700 Subject: [PATCH 3/4] Expand unit tests --- tests/test_views.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_views.py b/tests/test_views.py index 434b369..ed8f67d 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -65,6 +65,11 @@ async def test_redirects_strip_leading_slashes(cli): cli.server.skip_url_asserts = True resp = await check_response(cli, "//page/", status=302, allow_redirects=False) assert resp.headers['Location'] == "/page" + # also strip leading and trailing whitespace + resp = await check_response(cli, "/%0a/www.evil.com/", status=302, allow_redirects=False) + assert resp.headers['Location'] == "/www.evil.com" + resp = await check_response(cli, "/%0a /www.evil.com %0a%0b/", status=302, allow_redirects=False) + assert resp.headers['Location'] == "/www.evil.com" async def check_yaml_resource(cli, url, filename, **kwargs): From 0d6651666534d313d143604ac914ec8157570963 Mon Sep 17 00:00:00 2001 From: Geoff Brown Date: Mon, 7 Feb 2022 15:43:58 -0700 Subject: [PATCH 4/4] Fix lint error --- tests/test_views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_views.py b/tests/test_views.py index ed8f67d..6f28e52 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -68,7 +68,8 @@ async def test_redirects_strip_leading_slashes(cli): # also strip leading and trailing whitespace resp = await check_response(cli, "/%0a/www.evil.com/", status=302, allow_redirects=False) assert resp.headers['Location'] == "/www.evil.com" - resp = await check_response(cli, "/%0a /www.evil.com %0a%0b/", status=302, allow_redirects=False) + resp = await check_response(cli, "/%0a /www.evil.com %0a%0b/", status=302, + allow_redirects=False) assert resp.headers['Location'] == "/www.evil.com"