From 6130707076a1d37a7401103acea0506f6332206f Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Mon, 6 Jul 2026 17:19:12 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[=EB=B3=B4?= =?UTF-8?q?=EC=95=88=20=ED=96=A5=EC=83=81]=20subprocess=20=EC=8B=A4?= =?UTF-8?q?=ED=96=89=20=EC=8B=9C=20=EC=8B=9C=EA=B0=84=20=EC=B4=88=EA=B3=BC?= =?UTF-8?q?=20=EB=B0=A9=EC=A7=80=20=EC=A0=81=EC=9A=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `media_shrinker.py`의 모든 `subprocess.run` 호출(`ffmpeg`, `ffprobe`, `brctl` 등)에 명시적인 `timeout` 인자 추가 - 외부 프로세스가 무한정 대기하여 발생할 수 있는 리소스 고갈 및 DoS(서비스 거부) 위험 방지 - 작업 특성에 맞춘 제한 시간 적용 (단순 프로브/메타데이터: 60초, 미디어 변환/iCloud 다운로드: 3600초) - `subprocess.TimeoutExpired` 예외를 안전하게 포착하고 의미 있는 `MediaShrinkerError` 발생하도록 처리 - Bandit 등 보안 스캐너의 오탐을 방지하기 위한 `# nosec B603` 주석 추가 --- .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..85855d2 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-27 - [Missing Subprocess Timeouts (DoS Risk)] +**Vulnerability:** Found multiple `subprocess.run` calls in `media_shrinker.py` executing external tools (`ffmpeg`, `ffprobe`, `brctl`) without an explicit `timeout` argument. +**Learning:** `subprocess.run` blocks indefinitely by default. In a web/SaaS context, or even during batch processing, an external tool hanging (e.g., waiting for input, stalled network mount) can lead to resource exhaustion and Denial of Service (DoS). +**Prevention:** Always provide a realistic `timeout` argument to `subprocess.run` based on the expected execution time (e.g., 60s for probes, 3600s for heavy processing), and handle `subprocess.TimeoutExpired` appropriately. diff --git a/media_shrinker.py b/media_shrinker.py index d72a8d5..577f154 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 # nosec B603 + ) + 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, # nosec B603 + ) + 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, # nosec B603 + ) + 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 +1601,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 # nosec B603 ) 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 +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, # nosec B603 + ) + except subprocess.TimeoutExpired: + pass def _format_result(root: Path, result: ConversionResult) -> str: