From 53d5a891659dc4e9379d791e01bafddb87b35595 Mon Sep 17 00:00:00 2001 From: Peter Bieringer Date: Tue, 21 Apr 2026 18:54:28 +0200 Subject: [PATCH] use new format checker for path/user --- radicale/app/__init__.py | 9 +++++- radicale/app/base.py | 60 ++++++++++++++++++++++++++++++++++++ radicale/app/move.py | 3 ++ radicale/sharing/__init__.py | 16 ++++------ 4 files changed, 77 insertions(+), 11 deletions(-) diff --git a/radicale/app/__init__.py b/radicale/app/__init__.py index 71cd8f1c..0a58e102 100644 --- a/radicale/app/__init__.py +++ b/radicale/app/__init__.py @@ -466,6 +466,9 @@ class Application(ApplicationPartDelete, ApplicationPartHead, logger.warning("Called by reverse proxy, cannot remove base prefix %r from path: %r as not matching (may cause authentication issues using internal WebUI)", base_prefix, path) else: logger.debug("Called by reverse proxy, cannot remove base prefix %r from path: %r as not matching", base_prefix, path) + if not self._check_path_format(path): + logger.error("request contains invalid path: %r (not compliant to %r)", path, self._validate_path_value) + return response(*httputils.BAD_REQUEST) # Get function corresponding to method function = getattr(self, "do_%s" % request_method, None) @@ -496,7 +499,11 @@ class Application(ApplicationPartDelete, ApplicationPartHead, self.configuration, environ, base64.b64decode( authorization.encode("ascii"))).split(":", 1) - (user, info) = self._auth.login(login, password, context) or ("", "") if login else ("", "") + if login and not self._check_user_format(login): + info = "not compliant to %r" % self._validate_user_value + user = "" + else: + (user, info) = self._auth.login(login, password, context) or ("", "") if login else ("", "") if self.configuration.get("auth", "type") == "ldap": try: logger.debug("Groups received from LDAP: %r", ",".join(self._auth._ldap_groups)) diff --git a/radicale/app/base.py b/radicale/app/base.py index 7d89758a..dd6a68bd 100644 --- a/radicale/app/base.py +++ b/radicale/app/base.py @@ -17,7 +17,9 @@ import io import logging +import re import sys +import unicodedata import xml.etree.ElementTree as ET from typing import Optional, Union @@ -30,6 +32,18 @@ from radicale.rights import intersect import defusedxml.ElementTree as DefusedET # isort:skip sys.modules["xml.etree"].ElementTree = ET # type:ignore[attr-defined] +USER_PATTERN_STRICT: str = "a-zA-Z0-9@\\.\\-_" +PATH_PATTERN_STRICT: str = USER_PATTERN_STRICT + "\\/" # / as separator + +USER_PATTERN_STRICT_RE: str = "^[" + USER_PATTERN_STRICT + "]+$" +PATH_PATTERN_STRICT_RE: str = "^[" + PATH_PATTERN_STRICT + "]+$" + +USER_BLACKLIST_MINIMAL: list = [":", "'", '"', '*', '?'] +PATH_BLACKLIST_MINIMAL: list = USER_BLACKLIST_MINIMAL + +USER_WHITELIST_UNICODE: list = ["-", ".", "@"] # from USER_PATTERN_STRICT +PATH_WHITELIST_UNICODE: list = ["-", ".", "@", "/"] # from PATH_PATTERN_STRICT + class ApplicationBase: @@ -103,6 +117,52 @@ class ApplicationBase: content = self._xml_response(xmlutils.webdav_error(human_tag)) return status, headers, content, None + def _check_format(self, + string: str, + blacklist_minimal: list[str], + whitelist_unicode: list[str], + validation_type: str, + ) -> bool: + check_minimal = (validation_type == "minimal") + check_unicode = (validation_type == "unicodeletter") + logger.trace("_check_format investigate %r (validation_type=%r check_minimal=%s check_unicode=%s)", string, validation_type, check_minimal, check_unicode) + for c in string: + if c <= chr(31) or (c >= chr(127) and c <= chr(159)): + # ASCII: control char + return False + if unicodedata.category(c)[0] == "C": + # Unicode: control + return False + if check_minimal: + if c in blacklist_minimal: + logger.trace("_check_format found %r", c) + return False + elif check_unicode: + if c not in whitelist_unicode: + if unicodedata.category(c)[0] != "L": + return False + return True + + def _check_user_format(self, user: str) -> bool: + if self._validate_user_value == "strict": + return (re.search(USER_PATTERN_STRICT_RE, user) is not None) + else: + return self._check_format(user, + USER_BLACKLIST_MINIMAL, + USER_WHITELIST_UNICODE, + self._validate_user_value, + ) + + def _check_path_format(self, path: str) -> bool: + if self._validate_path_value == "strict": + return (re.search(PATH_PATTERN_STRICT_RE, path) is not None) + else: + return self._check_format(path, + PATH_BLACKLIST_MINIMAL, + PATH_WHITELIST_UNICODE, + self._validate_path_value, + ) + class Access: """Helper class to check access rights of an item""" diff --git a/radicale/app/move.py b/radicale/app/move.py index 9af6e378..7d0376ee 100644 --- a/radicale/app/move.py +++ b/radicale/app/move.py @@ -82,6 +82,9 @@ class ApplicationPartMove(ApplicationBase): if not access.check("w"): return httputils.NOT_ALLOWED to_path = pathutils.sanitize_path(to_url.path) + if not self._check_path_format(to_path): + logger.warning("request contains invalid path: %r (not compliant to %r)", to_path, self._validate_path_value) + return httputils.BAD_REQUEST if not (to_path + "/").startswith(base_prefix + "/"): logger.warning("Destination %r from MOVE request on %r doesn't " "start with base prefix", to_path, path) diff --git a/radicale/sharing/__init__.py b/radicale/sharing/__init__.py index 01f7ff83..5e7bc434 100644 --- a/radicale/sharing/__init__.py +++ b/radicale/sharing/__init__.py @@ -122,10 +122,6 @@ API_TYPES_V1: dict[str, type] = { TOKEN_PATTERN_V1: str = "v1/[a-zA-Z0-9_\\-]{44}" -PATH_PATTERN: str = "([a-zA-Z0-9/.\\-@]+)" # TODO: extend or find better source - -USER_PATTERN: str = "([a-zA-Z0-9@.]+)" # TODO: extend or find better source - OVERLAY_PROPERTIES_WHITELIST: Sequence[str] = ("C:calendar-description", "ICAL:calendar-color", "CR:addressbook-description", "INF:addressbook-color", "D:displayname") CONVERSIONS_WHITELIST: Sequence[str] = ("bday", "none") @@ -727,20 +723,20 @@ class BaseSharing(ApplicationBase): logger.warning(api_info + ": unsupported " + key) return httputils.bad_request("Invalid value for PathOrToken") else: - if not re.search('^' + PATH_PATTERN + '$', request_data[key]): - logger.warning(api_info + ": unsupported " + key) + if not self._check_path_format(request_data[key]): + logger.warning("%s: invalid %r: %r (not compliant to %r)", api_info, key, request_data[key], self._validate_path_value) return httputils.bad_request("Invalid value for PathOrToken") if not request_data[key].endswith("/"): return httputils.bad_request("PathOrToken not ending with /") elif key == "PathMapped": - if not re.search('^' + PATH_PATTERN + '$', request_data[key]): - logger.warning(api_info + ": unsupported " + key) + if not self._check_path_format(request_data[key]): + logger.warning("%s: invalid %r: %r (not compliant to %r)", api_info, key, request_data[key], self._validate_path_value) return httputils.bad_request("Invalid value for PathMapped") elif not request_data[key].endswith("/"): return httputils.bad_request("PathMapped not ending with /") elif key == "User": - if not re.search('^' + USER_PATTERN + '$', request_data[key]): - logger.warning(api_info + ": unsupported " + key) + if not self._check_user_format(request_data[key]): + logger.warning("%s: invalid %r: %r (not compliant to %r)", api_info, key, request_data[key], self._validate_user_value) return httputils.bad_request("Invalid value for User") # check for optional parameters