From d6a3aa7d788451badc89ccf2a53e9cb1d1d037b3 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 5 Jul 2026 18:13:13 +0000 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITIC?= =?UTF-8?q?AL]=20=EC=99=B8=EB=B6=80=20=ED=94=84=EB=A1=9C=EC=84=B8=EC=8A=A4?= =?UTF-8?q?=20=EC=8B=A4=ED=96=89=20=EC=8B=9C=20=EB=AC=B4=ED=95=9C=20?= =?UTF-8?q?=EB=8C=80=EA=B8=B0=20(DoS)=20=EC=B7=A8=EC=95=BD=EC=A0=90=20?= =?UTF-8?q?=ED=95=B4=EA=B2=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 외부 명령어(ffmpeg, ffprobe, brctl 등)를 실행하는 `subprocess.run` 호출에 명시적인 `timeout` 인자가 누락되어 있어, 해당 프로세스가 무한 대기할 경우 애플리케이션의 리소스가 고갈되는 취약점(Uncontrolled Resource Consumption)을 해결하였습니다. 각 명령어의 특성에 맞게 적절한 타임아웃을 추가하고, 발생하는 TimeoutExpired 예외를 안전하게 처리하도록 개선하였습니다. --- .jules/sentinel.md | 4 +++ media_shrinker.py | 77 ++++++++++++++++++++++++++++------------------ 2 files changed, 51 insertions(+), 30 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index a0d3824..a1041c7 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -46,3 +46,7 @@ **Vulnerability:** Argument Injection via relative paths starting with a hyphen in command-line utilities (CWE-88). **Learning:** Even when `ffmpeg` inputs are protected by `-i`, command-line utilities (like `ffprobe` and `ffmpeg` filters) can interpret user input (like a file path) starting with a hyphen (e.g., `-version.wav`) as options if passed as a relative path. **Prevention:** File paths must be converted to absolute paths using `.resolve()` before they are passed to `subprocess.run`. This prefixes the path with a root, drive, or UNC prefix rather than a leading hyphen, thereby averting the possibility of argument injection. +## 2024-05-24 - Missing Timeout on External Binary Execution (DoS Risk) +**Vulnerability:** Found multiple `subprocess.run` calls without explicit timeouts (e.g. `ffmpeg`, `ffprobe`, `brctl`, `SetFile`). +**Learning:** This exposes the application to Uncontrolled Resource Consumption (DoS) if these external binaries hang indefinitely. +**Prevention:** Always configure an explicit `timeout` parameter and handle `subprocess.TimeoutExpired` exceptions when executing external binaries with `subprocess.run`. diff --git a/media_shrinker.py b/media_shrinker.py index d72a8d5..98b9dd4 100644 --- a/media_shrinker.py +++ b/media_shrinker.py @@ -465,9 +465,12 @@ def probe_media( "-i", str(Path(source_path).resolve()), ] - completed = subprocess.run( - command, check=False, capture_output=True, text=True, shell=False - ) + try: + completed = subprocess.run( + command, check=False, capture_output=True, text=True, shell=False, timeout=60 + ) + except subprocess.TimeoutExpired as 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()}" @@ -520,18 +523,22 @@ 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, - shell=False, - ) + 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, + shell=False, + timeout=3600, + ) + except subprocess.TimeoutExpired as 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()}" @@ -646,13 +653,17 @@ 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, - shell=False, - ) + try: + completed = subprocess.run( + build_icloud_download_command(source_path, brctl_path=brctl_path), + check=False, + capture_output=True, + text=True, + shell=False, + timeout=3600, + ) + except subprocess.TimeoutExpired as 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()}" @@ -1590,8 +1601,10 @@ def _execute_plan( ) try: completed = subprocess.run( - command, check=False, capture_output=True, text=True, shell=False + command, check=False, capture_output=True, text=True, shell=False, 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 @@ -1658,13 +1671,17 @@ 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, - shell=False, - ) + try: + subprocess.run( + [setfile_path, "-d", creation_date, str(dest.resolve())], + check=False, + capture_output=True, + text=True, + shell=False, + timeout=60, + ) + except subprocess.TimeoutExpired: + pass def _format_result(root: Path, result: ConversionResult) -> str: From 1ed027272099d7bd9473a7e73b628fb7f316c875 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 5 Jul 2026 18:38:41 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITIC?= =?UTF-8?q?AL]=20Fix=20Uncontrolled=20Resource=20Consumption=20DoS\n\n-=20?= =?UTF-8?q?Add=20explicit=20timeout=20for=20`subprocess.run`=20to=20preven?= =?UTF-8?q?t=20DoS.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/test_media_shrinker.py | 53 ++++++++++++++++++++++++++++++++++++ tests/test_security.py | 47 ++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/tests/test_media_shrinker.py b/tests/test_media_shrinker.py index 0ab2397..ebe2bd2 100644 --- a/tests/test_media_shrinker.py +++ b/tests/test_media_shrinker.py @@ -1914,3 +1914,56 @@ class MockStat: with patch("media_shrinker._copy_macos_creation_time"): with patch("media_shrinker._get_setfile_path", return_value="/bin/echo"): preserve_file_attributes(src, dest) + @patch("media_shrinker.subprocess.run") + def test_probe_media_handles_timeout(self, mock_run: MagicMock) -> None: + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffprobe", timeout=60) + from media_shrinker import probe_media, MediaShrinkerError + with self.assertRaisesRegex(MediaShrinkerError, "ffprobe timed out"): + probe_media(Path("source.wav")) + + @patch("media_shrinker.subprocess.run") + def test_detect_silence_handles_timeout(self, mock_run: MagicMock) -> None: + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) + from media_shrinker import detect_silence_intervals, MediaShrinkerError + with self.assertRaisesRegex(MediaShrinkerError, "silencedetect timed out"): + detect_silence_intervals(Path("source.wav")) + + @patch("media_shrinker.subprocess.run") + def test_download_icloud_handles_timeout(self, mock_run: MagicMock) -> None: + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="brctl", timeout=3600) + from media_shrinker import download_from_icloud, MediaShrinkerError + with patch("shutil.which", return_value="brctl"): + with self.assertRaisesRegex(MediaShrinkerError, "iCloud download timed out"): + download_from_icloud(Path("source.wav")) + + @patch("media_shrinker.subprocess.run") + def test_execute_plan_handles_timeout(self, mock_run: MagicMock) -> None: + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) + from media_shrinker import _execute_plan, ConversionPlan, MediaShrinkerError + plan = ConversionPlan( + strategy="test", + input_path=Path("source.wav"), + output_path=Path("out.opus"), + ffmpeg_args=["-i", "source.wav", "out.opus"] + ) + with self.assertRaisesRegex(MediaShrinkerError, "ffmpeg timed out"): + _execute_plan(plan, Path("source.wav"), Path("final.opus"), ffmpeg_path="ffmpeg", overwrite=True) + + @patch("media_shrinker.subprocess.run") + def test_copy_macos_creation_time_handles_timeout(self, mock_run: MagicMock) -> None: + import subprocess + mock_run.side_effect = subprocess.TimeoutExpired(cmd="SetFile", timeout=60) + mock_stat = MagicMock() + mock_stat.st_birthtime = 1234567890.0 + from media_shrinker import _copy_macos_creation_time + # Should not raise exception + _copy_macos_creation_time(mock_stat, Path("dest.txt"), "SetFile") + + def test_build_silencedetect_command_invalid_noise(self) -> None: + from media_shrinker import build_silencedetect_command, MediaShrinkerError + with self.assertRaisesRegex(MediaShrinkerError, "Invalid silence_noise value"): + build_silencedetect_command(Path("source.wav"), silence_noise="invalid") diff --git a/tests/test_security.py b/tests/test_security.py index 322f1bb..310a07b 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -35,3 +35,50 @@ def test_probe_media_uses_explicit_input_flag_for_dash_prefixed_path( if __name__ == "__main__": unittest.main() + @patch("media_shrinker.subprocess.run") + def test_probe_media_handles_timeout(self, mock_run: MagicMock) -> None: + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffprobe", timeout=60) + with self.assertRaisesRegex(MediaShrinkerError, "ffprobe timed out"): + probe_media(Path("source.wav")) + + @patch("media_shrinker.subprocess.run") + def test_detect_silence_handles_timeout(self, mock_run: MagicMock) -> None: + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) + with self.assertRaisesRegex(MediaShrinkerError, "silencedetect timed out"): + detect_silence_intervals(Path("source.wav")) + + @patch("media_shrinker.subprocess.run") + def test_download_icloud_handles_timeout(self, mock_run: MagicMock) -> None: + mock_run.side_effect = subprocess.TimeoutExpired(cmd="brctl", timeout=3600) + with patch("shutil.which", return_value="brctl"): + with self.assertRaisesRegex(MediaShrinkerError, "iCloud download timed out"): + download_from_icloud(Path("source.wav")) + + @patch("media_shrinker.subprocess.run") + def test_execute_plan_handles_timeout(self, mock_run: MagicMock) -> None: + mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) + plan = ConversionPlan( + strategy="test", + input_path=Path("source.wav"), + output_path=Path("out.opus"), + ffmpeg_args=["-i", "source.wav", "out.opus"] + ) + with self.assertRaisesRegex(MediaShrinkerError, "ffmpeg timed out"): + _execute_plan(plan, Path("source.wav"), Path("final.opus"), ffmpeg_path="ffmpeg", overwrite=True) + + @patch("media_shrinker.subprocess.run") + def test_copy_macos_creation_time_handles_timeout(self, mock_run: MagicMock) -> None: + mock_run.side_effect = subprocess.TimeoutExpired(cmd="SetFile", timeout=60) + mock_stat = MagicMock() + mock_stat.st_birthtime = 1234567890.0 + # Should not raise exception + _copy_macos_creation_time(mock_stat, Path("dest.txt"), "SetFile") + + @patch("media_shrinker.subprocess.run") + def test_probe_media_invalid_json(self, mock_run: MagicMock) -> None: + mock_completed = MagicMock() + mock_completed.returncode = 0 + mock_completed.stdout = "invalid json" + mock_run.return_value = mock_completed + with self.assertRaisesRegex(MediaShrinkerError, "ffprobe returned invalid JSON"): + probe_media(Path("source.wav")) From 4952141132e4db71b7dae8fe8e744705b744be55 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Mon, 6 Jul 2026 01:28:58 +0000 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITIC?= =?UTF-8?q?AL]=20Fix=20Uncontrolled=20Resource=20Consumption=20DoS\n\n-=20?= =?UTF-8?q?Add=20explicit=20timeout=20for=20`subprocess.run`=20to=20preven?= =?UTF-8?q?t=20DoS.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/test_security.py | 40 +--------------------------------------- 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/tests/test_security.py b/tests/test_security.py index 310a07b..b251e29 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -35,50 +35,12 @@ def test_probe_media_uses_explicit_input_flag_for_dash_prefixed_path( if __name__ == "__main__": unittest.main() - @patch("media_shrinker.subprocess.run") - def test_probe_media_handles_timeout(self, mock_run: MagicMock) -> None: - mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffprobe", timeout=60) - with self.assertRaisesRegex(MediaShrinkerError, "ffprobe timed out"): - probe_media(Path("source.wav")) - - @patch("media_shrinker.subprocess.run") - def test_detect_silence_handles_timeout(self, mock_run: MagicMock) -> None: - mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) - with self.assertRaisesRegex(MediaShrinkerError, "silencedetect timed out"): - detect_silence_intervals(Path("source.wav")) - - @patch("media_shrinker.subprocess.run") - def test_download_icloud_handles_timeout(self, mock_run: MagicMock) -> None: - mock_run.side_effect = subprocess.TimeoutExpired(cmd="brctl", timeout=3600) - with patch("shutil.which", return_value="brctl"): - with self.assertRaisesRegex(MediaShrinkerError, "iCloud download timed out"): - download_from_icloud(Path("source.wav")) - - @patch("media_shrinker.subprocess.run") - def test_execute_plan_handles_timeout(self, mock_run: MagicMock) -> None: - mock_run.side_effect = subprocess.TimeoutExpired(cmd="ffmpeg", timeout=3600) - plan = ConversionPlan( - strategy="test", - input_path=Path("source.wav"), - output_path=Path("out.opus"), - ffmpeg_args=["-i", "source.wav", "out.opus"] - ) - with self.assertRaisesRegex(MediaShrinkerError, "ffmpeg timed out"): - _execute_plan(plan, Path("source.wav"), Path("final.opus"), ffmpeg_path="ffmpeg", overwrite=True) - - @patch("media_shrinker.subprocess.run") - def test_copy_macos_creation_time_handles_timeout(self, mock_run: MagicMock) -> None: - mock_run.side_effect = subprocess.TimeoutExpired(cmd="SetFile", timeout=60) - mock_stat = MagicMock() - mock_stat.st_birthtime = 1234567890.0 - # Should not raise exception - _copy_macos_creation_time(mock_stat, Path("dest.txt"), "SetFile") - @patch("media_shrinker.subprocess.run") def test_probe_media_invalid_json(self, mock_run: MagicMock) -> None: mock_completed = MagicMock() mock_completed.returncode = 0 mock_completed.stdout = "invalid json" mock_run.return_value = mock_completed + from media_shrinker import probe_media, MediaShrinkerError with self.assertRaisesRegex(MediaShrinkerError, "ffprobe returned invalid JSON"): probe_media(Path("source.wav"))