From b0252ca356668cab88af73991a247993aa426a0a Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 30 Jun 2026 17:38:44 +0000 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDIUM?= =?UTF-8?q?]=20DoS=20=EB=B0=A9=EC=A7=80=20-=20subprocess=20timeout=20?= =?UTF-8?q?=EC=B6=94=EA=B0=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 외부 프로세스(ffprobe, ffmpeg, brctl, SetFile 등)를 실행하는 `subprocess.run` 호출에 명시적인 `timeout`을 추가하였습니다. - 타임아웃 발생 시 무한정 대기(Hang)로 인한 리소스 고갈(CWE-400, DoS) 취약점을 방지하도록 `subprocess.TimeoutExpired` 예외 처리를 추가하였습니다. --- .jules/sentinel.md | 5 ++++ media_shrinker.py | 75 ++++++++++++++++++++++++++++++---------------- 2 files changed, 55 insertions(+), 25 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 467e052..fc8a923 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -36,3 +36,8 @@ **Vulnerability:** Argument Injection via relative paths starting with a hyphen in command-line utilities. **Learning:** Even when `ffmpeg` inputs are protected by `-i`, the output paths, as well as arguments to other utilities like `brctl` and `SetFile`, can be maliciously crafted to start with `-` and be interpreted as options if relative paths are used. **Prevention:** Resolve file paths before passing them to `subprocess.run` when a tool does not support an explicit input flag or `--` delimiter. Absolute paths use a root, drive, or UNC prefix rather than a leading hyphen, so they cannot be parsed as command-line options. + +## 2026-06-30 - [Sentinel: Prevent Uncontrolled Resource Consumption (DoS) via subprocess timeout] +**Vulnerability:** Uncontrolled Resource Consumption (DoS) via `subprocess.run` calls without explicit timeouts (CWE-400). +**Learning:** Using `subprocess.run` to execute external binaries like `ffmpeg`, `ffprobe`, or `brctl` without a `timeout` can hang the application indefinitely if the binary deadlocks or encounters malformed media. +**Prevention:** Always configure an explicit `timeout` parameter tailored to the specific binary (e.g., 60s for fast metadata reads, 3600s+ for slow conversions) and handle `subprocess.TimeoutExpired` exceptions when using `subprocess.run` to execute external binaries, ensuring the application does not hang indefinitely. diff --git a/media_shrinker.py b/media_shrinker.py index 7ee2dfb..f613420 100644 --- a/media_shrinker.py +++ b/media_shrinker.py @@ -465,7 +465,14 @@ def probe_media( "-i", str(source_path), ] - completed = subprocess.run(command, check=False, capture_output=True, text=True) + try: + completed = subprocess.run( + command, check=False, capture_output=True, text=True, timeout=60 + ) + except subprocess.TimeoutExpired as exc: + raise MediaShrinkerError( + f"ffprobe timed out after 60s for {source_path}" + ) from exc if completed.returncode != 0: raise MediaShrinkerError( f"ffprobe failed for {source_path}: {completed.stderr.strip()}" @@ -518,17 +525,23 @@ def detect_silence_intervals( ) -> list[SilenceInterval]: """Run ffmpeg silencedetect and return paired silence intervals.""" - completed = subprocess.run( - build_silencedetect_command( - source_path, - ffmpeg_path=ffmpeg_path, - silence_noise=silence_noise, - silence_min_duration_seconds=silence_min_duration_seconds, - ), - check=False, - capture_output=True, - text=True, - ) + try: + completed = subprocess.run( + build_silencedetect_command( + source_path, + ffmpeg_path=ffmpeg_path, + silence_noise=silence_noise, + silence_min_duration_seconds=silence_min_duration_seconds, + ), + check=False, + capture_output=True, + text=True, + timeout=3600, + ) + except subprocess.TimeoutExpired as exc: + raise MediaShrinkerError( + f"silencedetect timed out after 3600s for {source_path}" + ) from exc if completed.returncode != 0: raise MediaShrinkerError( f"silencedetect failed for {source_path}: {completed.stderr.strip()}" @@ -643,12 +656,18 @@ def download_from_icloud(source_path: Path, *, brctl_path: str = "brctl") -> Non raise MediaShrinkerError( f"iCloud download requested but '{brctl_path}' was not found" ) - completed = subprocess.run( - build_icloud_download_command(source_path, brctl_path=brctl_path), - check=False, - capture_output=True, - text=True, - ) + try: + completed = subprocess.run( + build_icloud_download_command(source_path, brctl_path=brctl_path), + check=False, + capture_output=True, + text=True, + timeout=3600, + ) + except subprocess.TimeoutExpired as exc: + raise MediaShrinkerError( + f"iCloud download timed out after 3600s for {source_path}" + ) from exc if completed.returncode != 0: raise MediaShrinkerError( f"iCloud download failed for {source_path}: {completed.stderr.strip()}" @@ -1586,10 +1605,12 @@ def _execute_plan( ) try: completed = subprocess.run( - command, check=False, capture_output=True, text=True + command, check=False, capture_output=True, text=True, timeout=14400 ) except FileNotFoundError as exc: raise MediaShrinkerError(f"ffmpeg not found: {ffmpeg_path}") from exc + except subprocess.TimeoutExpired as exc: + raise MediaShrinkerError(f"ffmpeg timed out after 14400s for {source}") from exc if completed.returncode != 0: raise MediaShrinkerError( @@ -1654,12 +1675,16 @@ def _copy_macos_creation_time( creation_date = datetime.fromtimestamp(float(birthtime)).strftime( "%m/%d/%Y %H:%M:%S" ) - subprocess.run( - [setfile_path, "-d", creation_date, str(dest.resolve())], - check=False, - capture_output=True, - text=True, - ) + try: + subprocess.run( + [setfile_path, "-d", creation_date, str(dest.resolve())], + check=False, + capture_output=True, + text=True, + timeout=60, + ) + except subprocess.TimeoutExpired: + pass def _format_result(root: Path, result: ConversionResult) -> str: From bdf1c4790d6d1c3a9b60d6e7dd3d1e093e5e9ab7 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 30 Jun 2026 18:11:05 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDIUM?= =?UTF-8?q?]=20DoS=20=EB=B0=A9=EC=A7=80=20-=20subprocess=20timeout=20?= =?UTF-8?q?=EC=B6=94=EA=B0=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 외부 프로세스(ffprobe, ffmpeg, brctl, SetFile 등)를 실행하는 `subprocess.run` 호출에 명시적인 `timeout`을 추가하였습니다. - 타임아웃 발생 시 무한정 대기(Hang)로 인한 리소스 고갈(CWE-400, DoS) 취약점을 방지하도록 `subprocess.TimeoutExpired` 예외 처리를 추가하였습니다. --- .jules/sentinel.md | 9 ++- media_shrinker.py | 28 ++++----- tests/test_media_shrinker.py | 111 +++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 23 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index fc8a923..bd74f37 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -36,8 +36,7 @@ **Vulnerability:** Argument Injection via relative paths starting with a hyphen in command-line utilities. **Learning:** Even when `ffmpeg` inputs are protected by `-i`, the output paths, as well as arguments to other utilities like `brctl` and `SetFile`, can be maliciously crafted to start with `-` and be interpreted as options if relative paths are used. **Prevention:** Resolve file paths before passing them to `subprocess.run` when a tool does not support an explicit input flag or `--` delimiter. Absolute paths use a root, drive, or UNC prefix rather than a leading hyphen, so they cannot be parsed as command-line options. - -## 2026-06-30 - [Sentinel: Prevent Uncontrolled Resource Consumption (DoS) via subprocess timeout] -**Vulnerability:** Uncontrolled Resource Consumption (DoS) via `subprocess.run` calls without explicit timeouts (CWE-400). -**Learning:** Using `subprocess.run` to execute external binaries like `ffmpeg`, `ffprobe`, or `brctl` without a `timeout` can hang the application indefinitely if the binary deadlocks or encounters malformed media. -**Prevention:** Always configure an explicit `timeout` parameter tailored to the specific binary (e.g., 60s for fast metadata reads, 3600s+ for slow conversions) and handle `subprocess.TimeoutExpired` exceptions when using `subprocess.run` to execute external binaries, ensuring the application does not hang indefinitely. +## 2024-06-25 - [Sentinel: Uncontrolled Resource Consumption in Subprocesses] +**Vulnerability:** Uncontrolled Resource Consumption (CWE-400) / DoS via hung external processes. +**Learning:** When using `subprocess.run` to execute external binaries (like `ffmpeg`, `ffprobe`, or `brctl`), the application can hang indefinitely if the external process stalls. This leads to resource exhaustion (e.g., worker threads/processes blocked) and potential Denial of Service. +**Prevention:** Always configure an explicit `timeout` parameter and handle `subprocess.TimeoutExpired` exceptions for all `subprocess.run` calls. Tailor the timeout duration to the specific task (e.g., 60s for `ffprobe`, 3600s for `ffmpeg` transcoding). diff --git a/media_shrinker.py b/media_shrinker.py index f613420..a9b5d8e 100644 --- a/media_shrinker.py +++ b/media_shrinker.py @@ -466,13 +466,9 @@ def probe_media( str(source_path), ] try: - completed = subprocess.run( - command, check=False, capture_output=True, text=True, timeout=60 - ) + completed = subprocess.run(command, check=False, capture_output=True, text=True, timeout=60) except subprocess.TimeoutExpired as exc: - raise MediaShrinkerError( - f"ffprobe timed out after 60s for {source_path}" - ) from exc + raise MediaShrinkerError(f"ffprobe timed out for {source_path}") from exc if completed.returncode != 0: raise MediaShrinkerError( f"ffprobe failed for {source_path}: {completed.stderr.strip()}" @@ -536,12 +532,10 @@ def detect_silence_intervals( check=False, capture_output=True, text=True, - timeout=3600, + timeout=600, ) except subprocess.TimeoutExpired as exc: - raise MediaShrinkerError( - f"silencedetect timed out after 3600s for {source_path}" - ) from exc + raise MediaShrinkerError(f"silencedetect timed out for {source_path}") from exc if completed.returncode != 0: raise MediaShrinkerError( f"silencedetect failed for {source_path}: {completed.stderr.strip()}" @@ -662,12 +656,10 @@ def download_from_icloud(source_path: Path, *, brctl_path: str = "brctl") -> Non check=False, capture_output=True, text=True, - timeout=3600, + timeout=600, ) except subprocess.TimeoutExpired as exc: - raise MediaShrinkerError( - f"iCloud download timed out after 3600s for {source_path}" - ) from exc + raise MediaShrinkerError(f"iCloud download timed out for {source_path}") from exc if completed.returncode != 0: raise MediaShrinkerError( f"iCloud download failed for {source_path}: {completed.stderr.strip()}" @@ -1605,12 +1597,12 @@ def _execute_plan( ) try: completed = subprocess.run( - command, check=False, capture_output=True, text=True, timeout=14400 + command, check=False, capture_output=True, text=True, timeout=3600 ) + except subprocess.TimeoutExpired as exc: + raise MediaShrinkerError(f"ffmpeg timed out for {source}") from exc except FileNotFoundError as exc: raise MediaShrinkerError(f"ffmpeg not found: {ffmpeg_path}") from exc - except subprocess.TimeoutExpired as exc: - raise MediaShrinkerError(f"ffmpeg timed out after 14400s for {source}") from exc if completed.returncode != 0: raise MediaShrinkerError( @@ -1681,7 +1673,7 @@ def _copy_macos_creation_time( check=False, capture_output=True, text=True, - timeout=60, + timeout=10, ) except subprocess.TimeoutExpired: pass diff --git a/tests/test_media_shrinker.py b/tests/test_media_shrinker.py index 7f5fc24..edd2282 100644 --- a/tests/test_media_shrinker.py +++ b/tests/test_media_shrinker.py @@ -1684,3 +1684,114 @@ def test_attribute_copy_helpers_ignore_platform_errors(self) -> None: if __name__ == "__main__": unittest.main() + +class TestSubprocessTimeouts(unittest.TestCase): + @patch("media_shrinker.subprocess.run") + def test_ffprobe_timeout(self, mock_run) -> None: + from media_shrinker import probe_media, MediaShrinkerError + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffprobe", timeout=60) + with self.assertRaisesRegex(MediaShrinkerError, "ffprobe timed out for"): + probe_media(Path("dummy.wav")) + + @patch("media_shrinker.subprocess.run") + def test_silencedetect_timeout(self, mock_run) -> None: + from media_shrinker import detect_silence_intervals, MediaShrinkerError + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) + with self.assertRaisesRegex(MediaShrinkerError, "silencedetect timed out for"): + detect_silence_intervals(Path("dummy.wav")) + + @patch("media_shrinker.subprocess.run") + def test_icloud_download_timeout(self, mock_run) -> None: + from media_shrinker import download_from_icloud, MediaShrinkerError + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="brctl", timeout=3600) + with self.assertRaisesRegex(MediaShrinkerError, "iCloud download timed out for"): + download_from_icloud(Path("dummy.wav"), brctl_path="ls") + + @patch("media_shrinker.subprocess.run") + def test_ffmpeg_timeout(self, mock_run) -> None: + from media_shrinker import _execute_plan, ConversionPlan, MediaShrinkerError + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) + plan = ConversionPlan( + strategy="flac-lossless", + input_path=Path("dummy.wav"), + output_path=Path("dummy.flac"), + ffmpeg_args=["-i", "dummy.wav", "-c:a", "flac", "dummy.flac"] + ) + with self.assertRaisesRegex(MediaShrinkerError, "ffmpeg timed out for dummy.wav"): + _execute_plan( + plan, ffmpeg_path="ffmpeg", overwrite=True + ) + + @patch("media_shrinker.subprocess.run") + def test_setfile_timeout(self, mock_run) -> None: + from media_shrinker import _copy_macos_creation_time + import subprocess + import os + from unittest.mock import Mock + mock_run.side_effect = subprocess.TimeoutExpired(cmd="SetFile", timeout=60) + + stat_mock = Mock(spec=os.stat_result) + stat_mock.st_birthtime = 1000.0 + + _copy_macos_creation_time(stat_mock, Path("dummy.wav"), "SetFile") + # Should catch and ignore the timeout, so no exception is raised + +class TestSubprocessTimeouts(unittest.TestCase): + @patch("media_shrinker.subprocess.run") + def test_ffprobe_timeout(self, mock_run) -> None: + from media_shrinker import probe_media, MediaShrinkerError + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffprobe", timeout=60) + with self.assertRaisesRegex(MediaShrinkerError, "ffprobe timed out for"): + probe_media(Path("dummy.wav")) + + @patch("media_shrinker.subprocess.run") + def test_silencedetect_timeout(self, mock_run) -> None: + from media_shrinker import detect_silence_intervals, MediaShrinkerError + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) + with self.assertRaisesRegex(MediaShrinkerError, "silencedetect timed out for"): + detect_silence_intervals(Path("dummy.wav")) + + @patch("media_shrinker.subprocess.run") + def test_icloud_download_timeout(self, mock_run) -> None: + from media_shrinker import download_from_icloud, MediaShrinkerError + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="brctl", timeout=3600) + with self.assertRaisesRegex(MediaShrinkerError, "iCloud download timed out for"): + download_from_icloud(Path("dummy.wav"), brctl_path="ls") + + @patch("media_shrinker.subprocess.run") + def test_ffmpeg_timeout(self, mock_run) -> None: + from media_shrinker import _execute_plan, ConversionPlan, MediaShrinkerError + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) + plan = ConversionPlan( + strategy="flac-lossless", + input_path=Path("dummy.wav"), + output_path=Path("dummy.flac"), + ffmpeg_args=["-i", "dummy.wav", "-c:a", "flac", "dummy.flac"] + ) + with self.assertRaisesRegex(MediaShrinkerError, "ffmpeg timed out for"): + _execute_plan( + plan, ffmpeg_path="ffmpeg", source=Path("dummy.wav"), final_output=Path("dummy.flac"), + protected_sources=set(), overwrite=True + ) + + @patch("media_shrinker.subprocess.run") + def test_setfile_timeout(self, mock_run) -> None: + from media_shrinker import _copy_macos_creation_time + import subprocess + import os + from unittest.mock import Mock + mock_run.side_effect = subprocess.TimeoutExpired(cmd="SetFile", timeout=60) + + stat_mock = Mock(spec=os.stat_result) + stat_mock.st_birthtime = 1000.0 + + _copy_macos_creation_time(stat_mock, Path("dummy.wav"), "SetFile") + # Should catch and ignore the timeout, so no exception is raised From a681258345779c6d47cbaa76daa5ea25b6950e1f Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 30 Jun 2026 18:25:03 +0000 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDIUM?= =?UTF-8?q?]=20DoS=20=EB=B0=A9=EC=A7=80=20-=20subprocess=20timeout=20?= =?UTF-8?q?=EC=B6=94=EA=B0=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 외부 프로세스(ffprobe, ffmpeg, brctl, SetFile 등)를 실행하는 `subprocess.run` 호출에 명시적인 `timeout`을 추가하였습니다. - 타임아웃 발생 시 무한정 대기(Hang)로 인한 리소스 고갈(CWE-400, DoS) 취약점을 방지하도록 `subprocess.TimeoutExpired` 예외 처리를 추가하였습니다. --- tests/test_media_shrinker.py | 55 ------------------------------------ 1 file changed, 55 deletions(-) diff --git a/tests/test_media_shrinker.py b/tests/test_media_shrinker.py index edd2282..1f3148b 100644 --- a/tests/test_media_shrinker.py +++ b/tests/test_media_shrinker.py @@ -1685,61 +1685,6 @@ def test_attribute_copy_helpers_ignore_platform_errors(self) -> None: if __name__ == "__main__": unittest.main() -class TestSubprocessTimeouts(unittest.TestCase): - @patch("media_shrinker.subprocess.run") - def test_ffprobe_timeout(self, mock_run) -> None: - from media_shrinker import probe_media, MediaShrinkerError - import subprocess - mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffprobe", timeout=60) - with self.assertRaisesRegex(MediaShrinkerError, "ffprobe timed out for"): - probe_media(Path("dummy.wav")) - - @patch("media_shrinker.subprocess.run") - def test_silencedetect_timeout(self, mock_run) -> None: - from media_shrinker import detect_silence_intervals, MediaShrinkerError - import subprocess - mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) - with self.assertRaisesRegex(MediaShrinkerError, "silencedetect timed out for"): - detect_silence_intervals(Path("dummy.wav")) - - @patch("media_shrinker.subprocess.run") - def test_icloud_download_timeout(self, mock_run) -> None: - from media_shrinker import download_from_icloud, MediaShrinkerError - import subprocess - mock_run.side_effect = subprocess.TimeoutExpired(cmd="brctl", timeout=3600) - with self.assertRaisesRegex(MediaShrinkerError, "iCloud download timed out for"): - download_from_icloud(Path("dummy.wav"), brctl_path="ls") - - @patch("media_shrinker.subprocess.run") - def test_ffmpeg_timeout(self, mock_run) -> None: - from media_shrinker import _execute_plan, ConversionPlan, MediaShrinkerError - import subprocess - mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) - plan = ConversionPlan( - strategy="flac-lossless", - input_path=Path("dummy.wav"), - output_path=Path("dummy.flac"), - ffmpeg_args=["-i", "dummy.wav", "-c:a", "flac", "dummy.flac"] - ) - with self.assertRaisesRegex(MediaShrinkerError, "ffmpeg timed out for dummy.wav"): - _execute_plan( - plan, ffmpeg_path="ffmpeg", overwrite=True - ) - - @patch("media_shrinker.subprocess.run") - def test_setfile_timeout(self, mock_run) -> None: - from media_shrinker import _copy_macos_creation_time - import subprocess - import os - from unittest.mock import Mock - mock_run.side_effect = subprocess.TimeoutExpired(cmd="SetFile", timeout=60) - - stat_mock = Mock(spec=os.stat_result) - stat_mock.st_birthtime = 1000.0 - - _copy_macos_creation_time(stat_mock, Path("dummy.wav"), "SetFile") - # Should catch and ignore the timeout, so no exception is raised - class TestSubprocessTimeouts(unittest.TestCase): @patch("media_shrinker.subprocess.run") def test_ffprobe_timeout(self, mock_run) -> None: From a9f2ed6016e6f81bc9f6d59c0f4825b770b31c49 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 30 Jun 2026 18:37:09 +0000 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDIUM?= =?UTF-8?q?]=20DoS=20=EB=B0=A9=EC=A7=80=20-=20subprocess=20timeout=20?= =?UTF-8?q?=EC=B6=94=EA=B0=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 외부 프로세스(ffprobe, ffmpeg, brctl, SetFile 등)를 실행하는 `subprocess.run` 호출에 명시적인 `timeout`을 추가하였습니다. - 타임아웃 발생 시 무한정 대기(Hang)로 인한 리소스 고갈(CWE-400, DoS) 취약점을 방지하도록 `subprocess.TimeoutExpired` 예외 처리를 추가하였습니다.