From e23b6b9a6c7109457aeb56f191012269791ae5e8 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:58:21 -0500 Subject: [PATCH] check the nginx-normalized path in the media auth dot-segment fallback The dot-segment fallback only checked the raw first segment, so a role restricted to one camera could request `/x/../clips/back_door-1.jpg` and nginx served back_door's clip. The fallback now resolves the path the way nginx does before checking for a media root, and the URI split ends at a literal `#` since nginx ignores everything after it. --- frigate/api/auth.py | 24 +++--------------------- frigate/api/media_auth.py | 30 +++++++++++++++++++++++++----- frigate/test/test_media_auth.py | 12 ++++++++++++ 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/frigate/api/auth.py b/frigate/api/auth.py index 2446589b15..1a842377f2 100644 --- a/frigate/api/auth.py +++ b/frigate/api/auth.py @@ -11,7 +11,7 @@ import secrets import time from datetime import datetime from pathlib import Path -from urllib.parse import parse_qs, unquote +from urllib.parse import parse_qs import requests from fastapi import APIRouter, Depends, HTTPException, Request, Response @@ -31,6 +31,7 @@ from frigate.api.media_auth import ( check_camera_access, deny_response_for_media_uri, is_role_restricted, + normalize_request_path, split_request_uri, ) from frigate.config import AuthConfig, ProxyConfig @@ -1187,32 +1188,13 @@ GO2RTC_STREAM_PROXY_PATHS = frozenset( ) -def _normalize_request_path(path: str) -> str: - """Normalize a raw request path the way nginx does before matching locations. - - nginx percent-decodes the path, merges repeated slashes and resolves dot - segments, while `X-Original-URL` carries the raw `$request_uri`. - """ - segments: list[str] = [] - for segment in unquote(path).split("/"): - if segment in ("", "."): - continue - if segment == "..": - if segments: - segments.pop() - continue - segments.append(segment) - - return "/" + "/".join(segments) - - def _is_go2rtc_stream_proxy_request(path: str, request: Request) -> bool: # nginx marks requests routed through the go2rtc stream locations, and the # normalized path catches them if that marker is ever missing if request.headers.get("x-go2rtc-stream-proxy") == "1": return True - normalized = _normalize_request_path(path) + normalized = normalize_request_path(path) return any(normalized.startswith(p) for p in GO2RTC_STREAM_PROXY_PATHS) diff --git a/frigate/api/media_auth.py b/frigate/api/media_auth.py index a94e720196..802077188a 100644 --- a/frigate/api/media_auth.py +++ b/frigate/api/media_auth.py @@ -48,12 +48,32 @@ def split_request_uri(request_uri: str) -> tuple[str, str]: """Split a raw request URI into its path and query string. urlparse is not used since it reads a path starting with `//` as a host. + nginx ends the URI at a literal `#`, so anything after it is dropped. """ uri = _URI_SCHEME_AND_AUTHORITY.sub("", request_uri, count=1) - path, _, query = uri.partition("?") + path, _, query = uri.partition("#")[0].partition("?") return path, query +def normalize_request_path(path: str) -> str: + """Normalize a raw request path the way nginx does before matching locations. + + nginx percent-decodes the path, merges repeated slashes and resolves dot + segments, while `X-Original-URL` carries the raw `$request_uri`. + """ + segments: list[str] = [] + for segment in unquote(path).split("/"): + if segment in ("", "."): + continue + if segment == "..": + if segments: + segments.pop() + continue + segments.append(segment) + + return "/" + "/".join(segments) + + def extract_path(original_url: str | None) -> str | None: """Return the decoded path component of nginx's `X-Original-URL` header. @@ -265,11 +285,11 @@ def deny_response_for_media_uri( path = extract_path(original_url) # `extract_path` returns None for URIs containing `.` or `..` segments. - # For media-root URIs that's a traversal attempt — deny outright. For - # non-media URIs, pass through (nginx / the backend handle them). + # Deny them when nginx would resolve them under a media root, and pass + # the rest through to nginx and the backend. if path is None: - decoded = unquote(split_request_uri(original_url)[0]) - first = decoded.lstrip("/").split("/", 1)[0] if decoded else "" + normalized = normalize_request_path(split_request_uri(original_url)[0]) + first = normalized.lstrip("/").split("/", 1)[0] if first in ("clips", "recordings", "exports"): return 403 return None diff --git a/frigate/test/test_media_auth.py b/frigate/test/test_media_auth.py index ea0e6164ac..f8d6289ed7 100644 --- a/frigate/test/test_media_auth.py +++ b/frigate/test/test_media_auth.py @@ -406,6 +406,18 @@ class TestDenyResponseForMediaUri(unittest.TestCase): self._deny("//clips/front_door/../back_door-1.jpg", "admin"), 403 ) + def test_traversal_from_non_media_prefix_denied(self): + # nginx resolves these into a media root before serving + for url in ( + "/x/../clips/back_door-1.jpg", + "/x/%2E%2E/clips/back_door-1.jpg", + "/./clips/front_door/../back_door-1.jpg", + "/api/../recordings/2026-05-11/14/back_door/00.00.mp4", + "/clips/back_door-1.jpg#/../../x", + ): + self.assertEqual(self._deny(url, "limited_user"), 403, url) + self.assertIsNone(self._deny("/x/../api/events", "limited_user")) + if __name__ == "__main__": unittest.main()