Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
77 changes: 47 additions & 30 deletions media_shrinker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()}"
Expand Down Expand Up @@ -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()}"
Expand Down Expand Up @@ -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()}"
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand Down
Loading