diff --git a/.jules/sentinel.md b/.jules/sentinel.md index a0d3824..cdd46d8 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-07-04 - 서브프로세스 타임아웃 부재로 인한 DoS 취약점 +**Vulnerability:** `subprocess.run` 호출 시 `timeout` 인자가 누락되어 외부 바이너리(ffmpeg, ffprobe 등)가 무한 대기 상태에 빠질 경우 시스템 리소스 고갈(DoS)이 발생할 수 있었습니다. +**Learning:** Python의 `subprocess.run`은 기본적으로 타임아웃을 적용하지 않으므로, 신뢰할 수 없는 외부 명령이나 I/O 대기가 발생할 수 있는 경우 프로세스가 영구적으로 멈출 수 있습니다. +**Prevention:** `subprocess.run`을 사용할 때는 항상 적절한 `timeout`(예: 프로브는 60초, 변환은 3600초)을 설정하고 `subprocess.TimeoutExpired` 예외를 처리하여 애플리케이션이 안전하게 복구될 수 있도록 해야 합니다. diff --git a/media_shrinker.py b/media_shrinker.py index d72a8d5..225f62a 100644 --- a/media_shrinker.py +++ b/media_shrinker.py @@ -465,9 +465,13 @@ 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 +524,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, - 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 +655,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, - 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,10 +1604,12 @@ 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 FileNotFoundError as exc: raise MediaShrinkerError(f"ffmpeg not found: {ffmpeg_path}") from exc + except subprocess.TimeoutExpired as exc: + raise MediaShrinkerError(f"ffmpeg timed out for {source}") from exc if completed.returncode != 0: raise MediaShrinkerError( @@ -1658,13 +1674,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: diff --git a/tests/test_media_shrinker.py b/tests/test_media_shrinker.py index 0ab2397..03b8100 100644 --- a/tests/test_media_shrinker.py +++ b/tests/test_media_shrinker.py @@ -1705,7 +1705,6 @@ def test_attribute_copy_helpers_ignore_platform_errors(self) -> None: class FastPathTests(unittest.TestCase): def test_copy_extended_attributes_dummy(self) -> None: - import os from media_shrinker import _copy_extended_attributes # We need to hit lines 703-704 @@ -1720,7 +1719,6 @@ def test_copy_extended_attributes_dummy(self) -> None: def test_copy_macos_creation_time_dummy(self) -> None: from media_shrinker import _copy_macos_creation_time - import stat # Hit 1632 class MockStat: @@ -1747,7 +1745,6 @@ def test_format_result_dummy(self) -> None: self.assertIn("foo.txt", s) def test_copy_extended_attributes_dummy_success(self) -> None: - import os from media_shrinker import _copy_extended_attributes # Hit 703-704 @@ -1763,7 +1760,6 @@ def test_copy_extended_attributes_dummy_success(self) -> None: def test_copy_macos_creation_time_dummy_none(self) -> None: from media_shrinker import _copy_macos_creation_time - import stat # Hit 1632 class MockStat: @@ -1775,7 +1771,6 @@ class MockStat: _copy_macos_creation_time(MockStat(), dest, "/bin/echo") def test_copy_extended_attributes_dummy_set_fail(self) -> None: - import os from media_shrinker import _copy_extended_attributes # Hit 703-704 @@ -1792,7 +1787,6 @@ def test_copy_extended_attributes_dummy_set_fail(self) -> None: def test_copy_macos_creation_time_dummy_not_found(self) -> None: from media_shrinker import _copy_macos_creation_time - import stat # Hit 1632 class MockStat: @@ -1807,7 +1801,6 @@ class MockStat: def test_copy_macos_creation_time_dummy_success(self) -> None: from media_shrinker import _copy_macos_creation_time - import stat # Hit 1632 class MockStat: @@ -1838,7 +1831,6 @@ class MockStat: preserve_file_attributes(src, dest) def test_copy_extended_attributes_dummy_success_branch(self) -> None: - import os from media_shrinker import _copy_extended_attributes # Hit 703-704 @@ -1856,7 +1848,6 @@ def test_copy_extended_attributes_dummy_success_branch(self) -> None: def test_copy_extended_attributes_dummy_listxattr_missing(self) -> None: import builtins - import os from media_shrinker import _copy_extended_attributes # Hit early return inside _copy_extended_attributes when OS doesn't support it @@ -1877,7 +1868,6 @@ def fake_hasattr(obj, name): def test_preserve_file_attributes_chmod_error(self) -> None: from media_shrinker import preserve_file_attributes - import stat # Hit 703-704 class MockStat: @@ -1897,7 +1887,6 @@ class MockStat: def test_preserve_file_attributes_with_setfile(self) -> None: from media_shrinker import preserve_file_attributes - import stat # Hit 703-704 class MockStat: