T09 · Insecure Skill Coding Practices
- Location
scripts/download_music.py:25- Finding
Unrestricted Category Path Allows Writes Outside the Music Directory
- Content
View full analysis
Vulnerability Details
File Location:
scripts/download_music.py, lines 25–28
Vulnerability Type: Path traversal and unrestricted filesystem write destination
Risk Level: MediumVulnerable Code
python def ensure_dir(category): """Ensure the category directory exists.""" category_path = os.path.join(MUSIC_DIR, category) os.makedirs(category_path, exist_ok=True) return category_pathThe resulting path is subsequently used as the output directory:
python output_dir = ensure_dir(category) output_template = os.path.join( output_dir, "%(title)s-%(uploader)s-%(extractor)s.%(ext)s" )Technical Analysis
The
categoryvalue comes directly from the second command-line argument and is passed toos.path.join()without validation or canonicalization.A relative value containing parent-directory components, such as
../../tmp/music, can escapeMUSIC_DIR. An absolute category path is more direct: on supported platforms,os.path.join(MUSIC_DIR, absolute_path)resolves to the supplied absolute path rather than a child ofMUSIC_DIR.os.makedirs()then creates the selected directory, and the path is incorporated into theyt-dlpoutput template. Although the final filename is generated byyt-dlp, the user controls the directory in which the downloaded media is written.The issue is not shell command injection because
subprocess.run()receives an argument list. It is instead an insufficient path-boundary validation vulnerability.Attack Path
- An attacker or untrusted caller supplies a category containing traversal components or an absolute path:
bash python3 scripts/download_music.py "https://example.com/media" "../../tmp/attacker-output" ensure_dir()joins the untrusted category withMUSIC_DIRwithout resolving and checking the resulting path.os.makedirs()creates the destination outside the intended music library.- The destination is embedded in the `yt-d ...[truncated 855 chars]
- An attacker or untrusted caller supplies a category containing traversal components or an absolute path:
- Remediation
View remediation
Remediation Suggestions
Treat categories as logical names rather than arbitrary paths:
- Reject absolute category paths.
- Resolve both the configured music root and proposed destination to canonical paths.
- Verify that the destination remains strictly within
MUSIC_DIR. - Optionally restrict category names to an allowlist of safe characters and prohibit
.and..path components. - Consider refusing symbolic-link components where an attacker can modify the music directory concurrently.
Example hardening:
python from pathlib import Path import re def ensure_dir(category): if not category or Path(category).is_absolute(): raise ValueError("Category must be a non-empty relative name") if any(part in (".", "..") for part in Path(category).parts): raise ValueError("Category contains prohibited path components") if not re.fullmatch(r"[\w .-]+", category): raise ValueError("Category contains unsupported characters") music_root = Path(MUSIC_DIR).expanduser().resolve() category_path = (music_root / category).resolve() try: category_path.relative_to(music_root) except ValueError: raise ValueError("Category must remain inside the music directory") category_path.mkdir(parents=True, exist_ok=True) return str(category_path)For stronger protection against symbolic-link races, perform filesystem operations through directory file descriptors or revalidate the resolved destination immediately before invoking the downloader.