diff --git a/frigate/api/app.py b/frigate/api/app.py index 4fd7bb9b32..3360598f95 100644 --- a/frigate/api/app.py +++ b/frigate/api/app.py @@ -627,6 +627,7 @@ def _restricted_go2rtc_response( Go2rtcStreamIssue.invalid_placeholder, "Invalid variable placeholder in go2rtc stream", ), + (Go2rtcStreamIssue.invalid_name, "Invalid go2rtc stream name"), ): names = [name for name, found in issues.items() if found == issue] if names: diff --git a/frigate/config/config.py b/frigate/config/config.py index 1b2c2f029c..5ad2bfa53c 100644 --- a/frigate/config/config.py +++ b/frigate/config/config.py @@ -39,6 +39,7 @@ from frigate.util.image import create_mask from frigate.util.live_streams import ( default_transcode_source, is_transcode_stream_name, + is_valid_go2rtc_stream_name, transcode_stream_name, transcode_streams, ) @@ -314,6 +315,11 @@ def apply_live_transcode_streams( f"Camera {camera_config.name} has transcoded streams enabled, but its source {transcode.source} is not a go2rtc stream." ) + if not is_valid_go2rtc_stream_name(transcode.source): + raise ValueError( + f"Camera {camera_config.name} transcodes go2rtc stream {transcode.source}, whose name contains '#', whitespace or quotes." + ) + generated = transcode_streams( camera_config.name, transcode.source, diff --git a/frigate/test/http_api/test_http_config_go2rtc_streams.py b/frigate/test/http_api/test_http_config_go2rtc_streams.py index f2fb883a32..ed32981c47 100644 --- a/frigate/test/http_api/test_http_config_go2rtc_streams.py +++ b/frigate/test/http_api/test_http_config_go2rtc_streams.py @@ -151,6 +151,19 @@ class TestConfigGo2rtcStreamRestrictions(BaseTestHttp): assert resp.status_code == 200, resp.text assert _EXISTING_SRC in self._read_config() + def test_set_rejects_stream_name_with_ffmpeg_params(self): + before = self._read_config() + resp = self._set_streams( + {"x#raw=-f#raw=mpegts#raw=/tmp/x": "rtsp://10.0.0.2:554/video"} + ) + assert resp.status_code == 400 + assert "Invalid go2rtc stream name" in resp.json()["message"] + assert self._read_config() == before + + def test_set_allows_plain_stream_names(self): + resp = self._set_streams({"back-yard.main_2": "rtsp://10.0.0.2:554/video"}) + assert resp.status_code == 200, resp.text + def test_set_allows_injected_source_when_override_set(self): with patch("frigate.util.services._GO2RTC_ARBITRARY_EXEC_ENV", "true"): resp = self._set_streams({"new_stream": [_INJECTED_SRC]}) diff --git a/frigate/test/test_live_streams.py b/frigate/test/test_live_streams.py index b72fa5d322..ec976f69fb 100644 --- a/frigate/test/test_live_streams.py +++ b/frigate/test/test_live_streams.py @@ -12,6 +12,7 @@ from frigate.util.live_streams import ( default_transcode_source, generated_transcode_streams, is_transcode_stream_name, + is_valid_go2rtc_stream_name, measure_stream_bitrate, raw_transcode_streams, sync_transcode_streams, @@ -37,6 +38,13 @@ class TestTranscodeNaming(unittest.TestCase): "#raw=-b:v#raw=500k#raw=-maxrate#raw=500k#raw=-bufsize#raw=1000k", ) + def test_valid_go2rtc_stream_names(self): + for name in ("front", "front_sub", "back-yard.main_2", "Haustür"): + self.assertTrue(is_valid_go2rtc_stream_name(name), name) + + for name in ("x#raw=-f", "front door", "front\tdoor", "a'b", 'a"b', "a${X}"): + self.assertFalse(is_valid_go2rtc_stream_name(name), name) + def test_default_source_skips_transcoded_streams(self): streams = {"Low": "front_transcode_360p", "Main": "front_main"} self.assertEqual(default_transcode_source("front", streams), "front_main") @@ -121,6 +129,22 @@ class TestRawTranscodeStreams(unittest.TestCase): self.assertEqual(list(raw_transcode_streams(config)), ["front_transcode_360p"]) + def test_unsafe_source_name_is_skipped(self): + config = { + "cameras": { + "front": { + "live": { + "transcode": {"enabled": True, "source": "x#raw=-f#raw=null"} + } + } + } + } + + self.assertEqual(raw_transcode_streams(config), {}) + + config["cameras"]["front"]["live"]["transcode"]["source"] = "front_main" + self.assertEqual(len(raw_transcode_streams(config)), 3) + def test_disabled_or_missing_generates_nothing(self): self.assertEqual(raw_transcode_streams({}), {}) self.assertEqual( @@ -286,6 +310,17 @@ class TestLiveTranscodeConfig(unittest.TestCase): with self.assertRaises(ValidationError): self.streams({"transcode": {"enabled": True, "source": "missing"}}) + def test_source_name_with_ffmpeg_params_fails(self): + name = "x#raw=-f#raw=null" + with self.assertRaisesRegex(ValidationError, "contains '#'"): + self.streams( + {"transcode": {"enabled": True, "source": name}}, + go2rtc={ + "front": ["rtsp://10.0.0.1:554/main"], + name: ["rtsp://10.0.0.1:554/main"], + }, + ) + def test_duplicate_heights_fail(self): with self.assertRaises(ValidationError): self.streams( diff --git a/frigate/util/live_streams.py b/frigate/util/live_streams.py index 65e765500f..dce8fb3ea6 100644 --- a/frigate/util/live_streams.py +++ b/frigate/util/live_streams.py @@ -31,6 +31,16 @@ def is_transcode_stream_name(camera: str, name: str) -> bool: return re.fullmatch(rf"{re.escape(camera)}_transcode_\d+p", name) is not None +def is_valid_go2rtc_stream_name(name: str) -> bool: + """Return whether a stream name is safe to splice into a go2rtc source. + + go2rtc splits sources on `#` and ffmpeg commands on whitespace, and expands + `${VAR}` at load, so a name with any of these, or with quotes, could add its + own parameters. + """ + return re.search(r"[\s#$'\"]", name) is None + + def transcode_stream_source(source: str, height: int, bitrate: int) -> str: """Build the go2rtc ffmpeg source that transcodes a stream to H.264. @@ -101,7 +111,9 @@ def raw_transcode_streams(config: dict[str, Any]) -> dict[str, str]: camera, live_streams or {camera: camera} ) - if source is None: + # an unsafe name fails Frigate's validation; skipping it here keeps + # it out of go2rtc + if source is None or not is_valid_go2rtc_stream_name(str(source)): continue qualities = transcode.get("qualities") diff --git a/frigate/util/services.py b/frigate/util/services.py index c7e4cca7ed..5cbaacb3e0 100644 --- a/frigate/util/services.py +++ b/frigate/util/services.py @@ -31,6 +31,7 @@ from frigate.const import ( ) from frigate.util.builtin import clean_camera_user_pass, escape_special_characters from frigate.util.env import UnknownVariableError, substitute_frigate_vars +from frigate.util.live_streams import is_valid_go2rtc_stream_name logger = logging.getLogger(__name__) @@ -1177,6 +1178,7 @@ class Go2rtcStreamIssue(str, Enum): restricted = "restricted" unknown_variable = "unknown_variable" invalid_placeholder = "invalid_placeholder" + invalid_name = "invalid_name" def find_restricted_go2rtc_stream_changes( @@ -1188,12 +1190,17 @@ def find_restricted_go2rtc_stream_changes( in the saved config for the same stream is left alone, so streams the operator configured on disk keep working; any other source must resolve its {FRIGATE_*} placeholders and pass `is_restricted_go2rtc_api_source`. + New stream names must pass `is_valid_go2rtc_stream_name`. """ old_streams = old_streams if isinstance(old_streams, dict) else {} new_streams = new_streams if isinstance(new_streams, dict) else {} issues: dict[str, Go2rtcStreamIssue] = {} for name, stream in new_streams.items(): + if name not in old_streams and not is_valid_go2rtc_stream_name(str(name)): + issues[name] = Go2rtcStreamIssue.invalid_name + continue + existing = _go2rtc_stream_sources(old_streams.get(name)) for source in _go2rtc_stream_sources(stream):