From 822905be7d41fbd52790de58ba20f0d82ce69efc Mon Sep 17 00:00:00 2001 From: Kevin Morris Date: Mon, 24 May 2021 05:19:57 -0700 Subject: [PATCH] bugfix: relax `next` verification AUR renders its own 404 Not Found page when a bad route is encountered. Introducing the previous verification caused an error in this case when setting a language while viewing the Not Found page. So, instead of checking through routes, just make sure that the next parameter starts with a '/' character, which removes the possibility of any cross attacks. + Removed aurweb.asgi.routes; no longer needed. Signed-off-by: Kevin Morris --- aurweb/asgi.py | 9 --------- aurweb/routers/html.py | 8 +++----- test/test_routes.py | 2 +- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/aurweb/asgi.py b/aurweb/asgi.py index 1a61b1f4..861f6056 100644 --- a/aurweb/asgi.py +++ b/aurweb/asgi.py @@ -12,8 +12,6 @@ from aurweb.auth import BasicAuthBackend from aurweb.db import get_engine from aurweb.routers import accounts, auth, errors, html, sso -routes = set() - # Setup the FastAPI app. app = FastAPI(exception_handlers=errors.exceptions) @@ -47,13 +45,6 @@ async def app_startup(): # Initialize the database engine and ORM. get_engine() -# NOTE: Always keep this dictionary updated with all routes -# that the application contains. We use this to check for -# parameter value verification. -routes = {route.path for route in app.routes} -routes.update({route.path for route in sso.router.routes}) -routes.update({route.path for route in html.router.routes}) - @app.exception_handler(HTTPException) async def http_exception_handler(request, exc): diff --git a/aurweb/routers/html.py b/aurweb/routers/html.py index e947d213..8f89e05c 100644 --- a/aurweb/routers/html.py +++ b/aurweb/routers/html.py @@ -32,11 +32,9 @@ async def language(request: Request, parameters across the redirect. """ from aurweb.db import session - from aurweb.asgi import routes - if unquote(next) not in routes: - return HTMLResponse( - b"Invalid 'next' parameter.", - status_code=400) + + if next[0] != '/': + return HTMLResponse(b"Invalid 'next' parameter.", status_code=400) query_string = "?" + q if q else str() diff --git a/test/test_routes.py b/test/test_routes.py index d512a172..e4816231 100644 --- a/test/test_routes.py +++ b/test/test_routes.py @@ -61,7 +61,7 @@ def test_language_invalid_next(): """ Test an invalid next route at '/language'. """ post_data = { "set_lang": "de", - "next": "/BLAHBLAHFAKE" + "next": "https://evil.net" } with client as req: response = req.post("/language", data=post_data)