T09 · Insecure Skill Coding Practices
Error
- Location
- sonos_core.py:73
- Finding
- Shell Command Injection Through the media_dir Parameter## Vulnerability Details **File Location**: `sonos_core.py:73-81` **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```python if platform.system() == "Windows": # Start server and save PID to file os.system(f'start /b python -m http.server {HTTP_PORT} --directory "{media_dir}"') # On Windows, we can't easily get the PID of start /b, so just track by port # The stop_http_server will use port-based killing as fallback else: # Use nohup to ensure it persists and save PID os.system(f'nohup python3 -m http.server {HTTP_PORT} --directory "{media_dir}" > /tmp/sonos_http.log 2>&1 &') ``` ### Technical Analysis The `media_dir` argument is interpolated directly into commands executed through `os.system()`. Although it is surrounded by double quotes, the value is not escaped for the relevant command shell. A crafted value containing a closing quote and shell control operators can terminate the intended argument and append arbitrary commands. This issue affects both the Windows and Unix-like execution paths. The function is reachable through the public `announce(..., media_dir=...)` API. Consequently, any party capable of influencing that argument may be able to execute commands with the privileges of the process running the Skill. ### Attack Path 1. An attacker gains control over, or influences, the `media_dir` value supplied to `announce()`. 2. `announce()` passes the value to `start_http_server(media_dir)`. 3. `start_http_server()` embeds the untrusted value into a shell command. 4. A crafted quote closes the `--directory` argument, while shell metacharacters append another command. 5. `os.system()` invokes the platform shell and executes the injected command. 6. The injected command runs under the account and permissions of the Agent or Skill host. ### Impact Assessment Successful exploitation permits arbitrary local command execution. The att ...[truncated 260 chars]
- Remediation
- ## Remediation Suggestions - Replace `os.system()` with `subprocess.Popen()` using an argument array and without `shell=True`. - Resolve `media_dir` with `pathlib.Path.resolve()` and require it to be an existing directory. - Restrict serving to an explicitly approved media root and reject paths that escape that root. - Track the returned `Popen` object or PID directly instead of discovering the process through shell commands. - Open log files through Python and pass their handles to `stdout` and `stderr`. Example hardened approach: ```python from pathlib import Path import subprocess directory = Path(media_dir).expanduser().resolve() if not directory.is_dir(): raise ValueError("Invalid media directory") process = subprocess.Popen( [ "python3", "-m", "http.server", str(HTTP_PORT), "--bind", HTTP_HOST, "--directory", str(directory), ], stdout=log_file, stderr=subprocess.STDOUT, start_new_session=True, ) ```