enhance validation in forms by checking hostname and path prefix for invalid characters and reserved prefixes

This commit is contained in:
Eduardo Silva
2026-03-16 20:48:44 -03:00
parent fb17394099
commit cf4674b933
2 changed files with 37 additions and 10 deletions

View File

@@ -62,6 +62,8 @@ class ApplicationForm(forms.ModelForm):
if not is_valid: if not is_valid:
self.add_error("upstream", _("Enter a valid upstream URL starting with http:// or https://")) self.add_error("upstream", _("Enter a valid upstream URL starting with http:// or https://"))
elif parsed.path not in ("", "/") or parsed.query or parsed.fragment:
self.add_error("upstream", _("Upstream must be a bare host address with no path, query or fragment. Use http://host or http://host:port"))
return cleaned_data return cleaned_data
@@ -74,6 +76,14 @@ class ApplicationHostForm(forms.ModelForm):
'hostname': _('Hostname'), 'hostname': _('Hostname'),
} }
def clean(self):
cleaned_data = super().clean()
hostname = (cleaned_data.get('hostname') or '').strip()
if hostname:
if any(c in hostname for c in ('{', '}', '\n', '\r', '\x00', ' ')):
self.add_error('hostname', _('Hostname contains invalid characters.'))
return cleaned_data
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
cancel_url = kwargs.pop('cancel_url', '#') cancel_url = kwargs.pop('cancel_url', '#')
super().__init__(*args, **kwargs) super().__init__(*args, **kwargs)
@@ -278,6 +288,9 @@ class ApplicationRouteForm(forms.ModelForm):
) )
) )
# Reserved path prefixes that map to internal system routes
_RESERVED_PREFIXES = ('/auth-gateway',)
def clean(self): def clean(self):
cleaned_data = super().clean() cleaned_data = super().clean()
path_prefix = (cleaned_data.get('path_prefix') or '').strip() path_prefix = (cleaned_data.get('path_prefix') or '').strip()
@@ -286,8 +299,13 @@ class ApplicationRouteForm(forms.ModelForm):
self.add_error('path_prefix', _('Path prefix must start with /.')) self.add_error('path_prefix', _('Path prefix must start with /.'))
elif ' ' in path_prefix: elif ' ' in path_prefix:
self.add_error('path_prefix', _('Path prefix cannot contain spaces.')) self.add_error('path_prefix', _('Path prefix cannot contain spaces.'))
elif any(c in path_prefix for c in ('{', '}', '\n', '\r')): elif any(c in path_prefix for c in ('{', '}', '\n', '\r', '\x00', '%')):
self.add_error('path_prefix', _('Path prefix contains invalid characters.')) self.add_error('path_prefix', _('Path prefix contains invalid characters.'))
elif any(
path_prefix == r or path_prefix.startswith(r + '/')
for r in self._RESERVED_PREFIXES
):
self.add_error('path_prefix', _('This path prefix is reserved by the system.'))
else: else:
cleaned_data['path_prefix'] = path_prefix cleaned_data['path_prefix'] = path_prefix
return cleaned_data return cleaned_data

View File

@@ -61,6 +61,11 @@ def split_upstream(upstream):
return base, path return base, path
def _safe_value(val: str) -> str:
"""Strip characters that would break or inject into a Caddyfile directive."""
return re.sub(r"[\r\n\x00]", "", val)
def build_caddyfile(apps, auth_policies, routes): def build_caddyfile(apps, auth_policies, routes):
policies = auth_policies.get("policies", {}) if auth_policies else {} policies = auth_policies.get("policies", {}) if auth_policies else {}
route_entries = routes.get("entries", {}) if routes else {} route_entries = routes.get("entries", {}) if routes else {}
@@ -82,12 +87,15 @@ def build_caddyfile(apps, auth_policies, routes):
lines.append(f"{indent}request_header -{header_name}") lines.append(f"{indent}request_header -{header_name}")
def emit_encoded_slash_block(): def emit_encoded_slash_block():
# Reject paths containing %2f or %2F (percent-encoded slash). # Reject paths containing encoded slashes in any depth of percent-encoding:
# Caddy's path matcher does not decode percent-encoding, so /%2fadmin # %2f = /
# would NOT match path /admin and would fall through to the default # %252f = %2f (double-encoded, decodes to / on second pass)
# (potentially bypass) handler, even though upstreams may decode it to /admin. # %25252f = triple-encoded, etc.
# Caddy's path matcher does not decode percent-encoding, so /%2fadmin would
# NOT match path /admin and would fall through to the default (potentially
# bypass) handler even though some upstreams decode it to /admin.
lines.append(" @encoded_slash {") lines.append(" @encoded_slash {")
lines.append(" path_regexp (?i)%2f") lines.append(" path_regexp (?i)%(?:25)*2f")
lines.append(" }") lines.append(" }")
lines.append(" handle @encoded_slash {") lines.append(" handle @encoded_slash {")
lines.append(" respond 400") lines.append(" respond 400")
@@ -146,7 +154,8 @@ def build_caddyfile(apps, auth_policies, routes):
base, upstream_path = split_upstream(upstream) base, upstream_path = split_upstream(upstream)
lines.append(f"{', '.join(hosts)} {{") safe_hosts = [_safe_value(h) for h in hosts]
lines.append(f"{', '.join(safe_hosts)} {{")
lines.append(" # Security: overwrite client-supplied forwarding headers with verified values") lines.append(" # Security: overwrite client-supplied forwarding headers with verified values")
lines.append(" request_header X-Forwarded-For {http.request.remote.host}") lines.append(" request_header X-Forwarded-For {http.request.remote.host}")
lines.append(" request_header -X-Forwarded-Host") lines.append(" request_header -X-Forwarded-Host")
@@ -156,9 +165,9 @@ def build_caddyfile(apps, auth_policies, routes):
emit_auth_portal() emit_auth_portal()
for static_route in static_routes: for static_route in static_routes:
path_prefix = static_route.get("path_prefix", "") path_prefix = _safe_value(static_route.get("path_prefix", ""))
root_dir = static_route.get("root", "") root_dir = _safe_value(static_route.get("root", ""))
cache_control = static_route.get("cache_control", "") cache_control = _safe_value(static_route.get("cache_control", ""))
lines.append(f" handle_path {path_prefix}/* {{") lines.append(f" handle_path {path_prefix}/* {{")
lines.append(f" root * {root_dir}") lines.append(f" root * {root_dir}")
lines.append(" file_server") lines.append(" file_server")