mirror of
https://github.com/blakeblackshear/frigate.git
synced 2026-10-15 11:12:49 +03:00
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.
This commit is contained in:
+3
-21
@@ -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)
|
||||
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user