T09 · Insecure Skill Coding Practices
Warning
- Location
- seedance_byteplus.py:232
- Finding
- Shell Command Injection Through Unsanitized Download Path<![CDATA[ ## Vulnerability Details **File Location**: `seedance_byteplus.py`, lines 232–242 **Vulnerability Type**: OS command injection **Risk Level**: Medium ### Vulnerable Code ```python download_path = Path(download_dir).expanduser() download_path.mkdir(parents=True, exist_ok=True) filename = f"seedance_{task_id}_{int(time.time())}.mp4" filepath = download_path / filename print(f"\nDownloading video to {filepath}...") try: urllib.request.urlretrieve(video_url, str(filepath)) print(f"Saved to: {filepath}") # Open on macOS if sys.platform == "darwin": os.system(f'open "{filepath}"') ``` ### Technical Analysis The generated file path is interpolated directly into a command passed to `os.system()`. This function invokes a system shell, causing shell metacharacters and command substitutions in `filepath` to be interpreted rather than treated as literal path characters. The path contains two insufficiently trusted values: - `download_dir`, supplied through the `--download` command-line option. - `task_id`, obtained from the remote API response and incorporated into the filename. Wrapping the path in double quotes does not prevent shell command substitution. For example, a download directory containing `$(malicious-command)` remains executable inside double quotes on a POSIX shell. Opening a file does not require shell interpretation. Consequently, invoking a shell exceeds the minimum privileges and execution capabilities needed for the declared functionality. ### Attack Path 1. An attacker influences an Agent or user to invoke the CLI on macOS with a crafted `--download` path, such as a path containing shell command substitution. 2. Alternatively, compromise or manipulation of the API response could provide a maliciously formed task ID. 3. The Skill creates the video-generation task and waits until its status is `succeeded`. 4. The generated video is downloaded to a path derived from the crafted value. 5. The path is interpol ...[truncated 1149 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Eliminate shell invocation and pass arguments directly to the operating-system utility: ```python import subprocess if sys.platform == "darwin": try: subprocess.run( ["/usr/bin/open", str(filepath)], check=False, shell=False, ) except OSError as e: print(f"Unable to open downloaded video: {e}", file=sys.stderr) ``` Additional hardening measures: 1. Validate `task_id` against the exact format documented by BytePlus before using it in a filename. A conservative fallback is to allow only letters, digits, underscores, and hyphens. 2. Generate the local filename independently of remote values, such as with `uuid.uuid4()`, while retaining the task ID only as metadata. 3. Treat `--download` as a filesystem path only and never interpolate it into shell commands. 4. Resolve the destination path and verify that it remains within an intended download directory if the surrounding Agent imposes workspace confinement. 5. Add regression tests using paths containing spaces, quotes, dollar signs, backticks, semicolons, and command-substitution syntax to verify that no shell interpretation occurs. 6. Avoid replacing `os.system()` with `subprocess.run(..., shell=True)`, because that would preserve the command-injection risk. ]]>