T09 · Insecure Skill Coding Practices
Warning
- Location
- download_video.py:31
- Finding
- Untrusted URL Is Interpreted as yt-dlp Command-Line Options<![CDATA[ ## Vulnerability Details **File Location**: `download_video.py`, lines 31–59 **Vulnerability Type**: Command-line argument injection **Risk Level**: Medium ### Vulnerable Code ```python def inspect_video(url: str) -> dict: """Fetch metadata without downloading.""" result = subprocess.run( ["yt-dlp", "--dump-single-json", "--no-playlist", url], check=True, text=True, capture_output=True, ) return json.loads(result.stdout) def build_command(args: argparse.Namespace) -> list[str]: """Build the yt-dlp command from parsed arguments.""" output_dir = Path(args.output).expanduser() output_template = str(output_dir / "%(title)s [%(id)s].%(ext)s") cmd = ["yt-dlp", "--no-playlist", "--no-progress"] if args.audio_only: cmd.extend(["-x", "--audio-format", "mp3", "--audio-quality", "0"]) else: cmd.extend(["-f", format_selector(args.quality), "--merge-output-format", args.format]) if args.restrict_filenames: cmd.append("--restrict-filenames") cmd.extend(["-o", output_template, args.url]) return cmd ``` ### Technical Analysis The purported URL is appended directly to two `yt-dlp` command lines without an end-of-options delimiter such as `--`. A value beginning with `-` can therefore be parsed by `yt-dlp` as an option rather than as a media URL. Python's `subprocess.run` is correctly invoked with an argument list and without `shell=True`, so ordinary shell metacharacters do not produce shell injection. However, using a list does not prevent injection into the invoked program's own command-line parser. The wrapper's `argparse` configuration normally rejects an option-like positional value. Nevertheless, a caller can explicitly terminate the wrapper's options and pass such a value as the positional argument: ```bash python3 download_video.py -- '--batch-file=/tmp/attacker-list.txt' ``` The resulting metadata command is effectively: ```bash yt-dlp --d ...[truncated 2668 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Place an explicit end-of-options delimiter before every untrusted URL passed to yt-dlp: ```python result = subprocess.run( ["yt-dlp", "--dump-single-json", "--no-playlist", "--", url], check=True, text=True, capture_output=True, ) ``` Apply the same protection to the download command: ```python cmd.extend(["-o", output_template, "--", args.url]) ``` 2. Validate the URL before invoking yt-dlp. Permit only explicitly supported network schemes and require a hostname: ```python from urllib.parse import urlparse def validate_url(value: str) -> str: parsed = urlparse(value) if parsed.scheme not in {"https", "http"} or not parsed.hostname: raise ValueError("A valid HTTP or HTTPS video URL is required.") return value ``` 3. Explicitly reject values beginning with `-`, even after URL parsing, as defense in depth. 4. Consider adding `--ignore-config` to both yt-dlp invocations if ambient user or system yt-dlp configurations are not required. This prevents unrelated configuration files from silently changing the wrapper's reviewed behavior: ```python ["yt-dlp", "--ignore-config", "--dump-single-json", "--no-playlist", "--", url] ``` 5. Add regression tests covering: - `--batch-file=...` - `--config-locations=...` - `--exec=...` - malformed URLs - unsupported schemes - valid HTTP and HTTPS URLs 6. Continue using argument-list subprocess execution with `shell=False`; do not replace it with a shell command string. ]]>