From 436c8c67e268dc40cfaf26aad79ce3c2ddbb1486 Mon Sep 17 00:00:00 2001 From: Pasit Sangprachathanarak Date: Wed, 22 Apr 2026 12:27:51 +0800 Subject: [PATCH 1/4] Prevent open redirect in login 'next' handling normalize the login "next" parameter in admin and contest login handlers. --- cms/server/admin/handlers/main.py | 10 ++++++++-- cms/server/contest/handlers/main.py | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/cms/server/admin/handlers/main.py b/cms/server/admin/handlers/main.py index e3ce0760aa..c245812b4a 100644 --- a/cms/server/admin/handlers/main.py +++ b/cms/server/admin/handlers/main.py @@ -27,6 +27,7 @@ import json import logging +from urllib.parse import urlsplit from cms import ServiceCoord, get_service_shards, get_service_address from cms.db import Admin, Contest, Question @@ -48,8 +49,13 @@ def post(self): next_page: str = self.get_argument("next", None) if next_page is not None: error_args["next"] = next_page - if next_page != "/": - next_page = self.url(*next_page.strip("/").split("/")) + split = urlsplit(next_page) + if split.scheme or split.netloc or not split.path.startswith("/"): + next_page = self.url() + elif split.path != "/": + next_page = self.url(*split.path.strip("/").split("/")) + if split.query: + next_page += "?" + split.query else: next_page = self.url() else: diff --git a/cms/server/contest/handlers/main.py b/cms/server/contest/handlers/main.py index 93402e2b0c..a4f3070b98 100644 --- a/cms/server/contest/handlers/main.py +++ b/cms/server/contest/handlers/main.py @@ -33,6 +33,7 @@ import logging import os.path import re +from urllib.parse import urlsplit import collections @@ -217,8 +218,13 @@ def post(self): next_page: str | None = self.get_argument("next", None) if next_page is not None: error_args["next"] = next_page - if next_page != "/": - next_page = self.url(*next_page.strip("/").split("/")) + split = urlsplit(next_page) + if split.scheme or split.netloc or not split.path.startswith("/"): + next_page = self.contest_url() + elif split.path != "/": + next_page = self.url(*split.path.strip("/").split("/")) + if split.query: + next_page += "?" + split.query else: next_page = self.url() else: From 2c83bef2e66115f101839e796bfc4f3b3b823c3b Mon Sep 17 00:00:00 2001 From: Pasit Sangprachathanarak Date: Wed, 22 Apr 2026 14:34:17 +0800 Subject: [PATCH 2/4] Update cms/server/contest/handlers/main.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- cms/server/contest/handlers/main.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cms/server/contest/handlers/main.py b/cms/server/contest/handlers/main.py index a4f3070b98..d101727fb8 100644 --- a/cms/server/contest/handlers/main.py +++ b/cms/server/contest/handlers/main.py @@ -222,9 +222,13 @@ def post(self): if split.scheme or split.netloc or not split.path.startswith("/"): next_page = self.contest_url() elif split.path != "/": - next_page = self.url(*split.path.strip("/").split("/")) - if split.query: - next_page += "?" + split.query + path_segments = split.path.strip("/").split("/") + if any(segment in ("", ".", "..") for segment in path_segments): + next_page = self.contest_url() + else: + next_page = self.url(*path_segments) + if split.query: + next_page += "?" + split.query else: next_page = self.url() else: From 46800cdf4768ab52a4a2501cf7b35adf4ed863d5 Mon Sep 17 00:00:00 2001 From: Pasit Sangprachathanarak Date: Wed, 22 Apr 2026 14:37:04 +0800 Subject: [PATCH 3/4] Validate and sanitize 'next' URL in login handlers Normalize and validate the parsed next-page path in admin and contest login handlers. Handle empty paths by treating them as "/", reject URLs with a scheme or netloc, and refuse path segments that are empty or contain "." or ".." to avoid unsafe redirects or path traversal. Also ensure the query string is preserved when constructing fallback URLs. These changes harden next-parameter handling and fix edge cases when urlsplit.path is empty. --- cms/server/admin/handlers/main.py | 17 ++++++++++++----- cms/server/contest/handlers/main.py | 9 ++++++--- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/cms/server/admin/handlers/main.py b/cms/server/admin/handlers/main.py index c245812b4a..ba85fee6d1 100644 --- a/cms/server/admin/handlers/main.py +++ b/cms/server/admin/handlers/main.py @@ -50,14 +50,21 @@ def post(self): if next_page is not None: error_args["next"] = next_page split = urlsplit(next_page) - if split.scheme or split.netloc or not split.path.startswith("/"): + path = split.path or "/" + if split.scheme or split.netloc or not path.startswith("/"): next_page = self.url() - elif split.path != "/": - next_page = self.url(*split.path.strip("/").split("/")) - if split.query: - next_page += "?" + split.query + elif path != "/": + path_segments = path.strip("/").split("/") + if any(segment in ("", ".", "..") for segment in path_segments): + next_page = self.url() + else: + next_page = self.url(*path_segments) + if split.query: + next_page += "?" + split.query else: next_page = self.url() + if split.query: + next_page += "?" + split.query else: next_page = self.url() error_page = self.url("login", **error_args) diff --git a/cms/server/contest/handlers/main.py b/cms/server/contest/handlers/main.py index d101727fb8..fb0111101e 100644 --- a/cms/server/contest/handlers/main.py +++ b/cms/server/contest/handlers/main.py @@ -219,10 +219,11 @@ def post(self): if next_page is not None: error_args["next"] = next_page split = urlsplit(next_page) - if split.scheme or split.netloc or not split.path.startswith("/"): + path = split.path or "/" + if split.scheme or split.netloc or not path.startswith("/"): next_page = self.contest_url() - elif split.path != "/": - path_segments = split.path.strip("/").split("/") + elif path != "/": + path_segments = path.strip("/").split("/") if any(segment in ("", ".", "..") for segment in path_segments): next_page = self.contest_url() else: @@ -231,6 +232,8 @@ def post(self): next_page += "?" + split.query else: next_page = self.url() + if split.query: + next_page += "?" + split.query else: next_page = self.contest_url() error_page = self.contest_url(**error_args) From 80bf1fe122780dd5852797de5b5d933e63fb6780 Mon Sep 17 00:00:00 2001 From: Pasit Sangprachathanarak Date: Thu, 23 Apr 2026 21:05:33 +0800 Subject: [PATCH 4/4] Move function to util.py --- cms/server/admin/handlers/main.py | 21 ++---------------- cms/server/contest/handlers/main.py | 21 ++---------------- cms/server/util.py | 34 ++++++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/cms/server/admin/handlers/main.py b/cms/server/admin/handlers/main.py index ba85fee6d1..4557e28a0b 100644 --- a/cms/server/admin/handlers/main.py +++ b/cms/server/admin/handlers/main.py @@ -27,11 +27,11 @@ import json import logging -from urllib.parse import urlsplit from cms import ServiceCoord, get_service_shards, get_service_address from cms.db import Admin, Contest, Question from cms.server.jinja2_toolbox import markdown_filter +from cms.server.util import normalize_login_next_page from cmscommon.crypto import validate_password from cmscommon.datetime import make_datetime, make_timestamp from .base import BaseHandler, SimpleHandler, require_permission @@ -49,24 +49,7 @@ def post(self): next_page: str = self.get_argument("next", None) if next_page is not None: error_args["next"] = next_page - split = urlsplit(next_page) - path = split.path or "/" - if split.scheme or split.netloc or not path.startswith("/"): - next_page = self.url() - elif path != "/": - path_segments = path.strip("/").split("/") - if any(segment in ("", ".", "..") for segment in path_segments): - next_page = self.url() - else: - next_page = self.url(*path_segments) - if split.query: - next_page += "?" + split.query - else: - next_page = self.url() - if split.query: - next_page += "?" + split.query - else: - next_page = self.url() + next_page = normalize_login_next_page(next_page, self.url, self.url()) error_page = self.url("login", **error_args) username: str = self.get_argument("username", "") diff --git a/cms/server/contest/handlers/main.py b/cms/server/contest/handlers/main.py index fb0111101e..2b3e60f160 100644 --- a/cms/server/contest/handlers/main.py +++ b/cms/server/contest/handlers/main.py @@ -33,7 +33,6 @@ import logging import os.path import re -from urllib.parse import urlsplit import collections @@ -53,6 +52,7 @@ from cms.grading.languagemanager import get_language from cms.grading.steps import COMPILATION_MESSAGES, EVALUATION_MESSAGES from cms.server import multi_contest +from cms.server.util import normalize_login_next_page from cms.server.contest.authentication import validate_login from cms.server.contest.communication import get_communications from cmscommon.crypto import hash_password, validate_password @@ -218,24 +218,7 @@ def post(self): next_page: str | None = self.get_argument("next", None) if next_page is not None: error_args["next"] = next_page - split = urlsplit(next_page) - path = split.path or "/" - if split.scheme or split.netloc or not path.startswith("/"): - next_page = self.contest_url() - elif path != "/": - path_segments = path.strip("/").split("/") - if any(segment in ("", ".", "..") for segment in path_segments): - next_page = self.contest_url() - else: - next_page = self.url(*path_segments) - if split.query: - next_page += "?" + split.query - else: - next_page = self.url() - if split.query: - next_page += "?" + split.query - else: - next_page = self.contest_url() + next_page = normalize_login_next_page(next_page, self.url, self.contest_url()) error_page = self.contest_url(**error_args) username: str = self.get_argument("username", "") diff --git a/cms/server/util.py b/cms/server/util.py index 3251a1fc24..902e521352 100644 --- a/cms/server/util.py +++ b/cms/server/util.py @@ -28,7 +28,7 @@ import logging from functools import wraps -from urllib.parse import quote, urlencode +from urllib.parse import quote, urlencode, urlsplit import collections try: @@ -168,6 +168,38 @@ def __getitem__(self, component: object) -> typing.Self: return self.__class__(self.__call__(component)) +def normalize_login_next_page(next_page: str | None, url: Url, default_url: str) -> str: + """Normalize a login redirection target. + + Accept only local absolute-path targets (plus optional query), and + rebase them through the provided URL builder. Query-only values are + treated as "/" and preserved. + + next_page: raw value of the "next" parameter. + url: URL builder for local paths. + default_url: fallback when next_page is missing or invalid. + + return: normalized redirect target. + """ + if next_page is None: + return default_url + + split = urlsplit(next_page) + path = split.path or "/" + if split.scheme or split.netloc or not path.startswith("/"): + return default_url + + if path != "/": + normalized = url(*path.strip("/").split("/")) + else: + normalized = url() + + if split.query: + normalized += "?" + split.query + + return normalized + + class CommonRequestHandler(RequestHandler): """Encapsulates shared RequestHandler functionality.