T09 · Insecure Skill Coding Practices
Error
- Location
agent.py:62- Finding
<![CDATA[Arbitrary Command Execution Through Shell-Interpolated User Input]]>
- Content
View full analysis
str: r = subprocess.run(cmd, shell=True, capture_output=True, text=True, timeout=timeout) if r.returncode != 0: raise RuntimeError(f"命令失败: {cmd[:80]}\n{r.stderr[:300]}") return r.stdout.strip() ``` User-controlled URLs are interpolated into commands: ```python dump = json.loads(run(f'yt-dlp --dump-json --no-playlist "{url}"', timeout=30)) ``` ```python cmd = f'yt-dlp -f bestvideo+bestaudio/best --merge-output-format mp4 -o "{out_tmpl}" --no-playlist "{url}"' out = run(cmd, timeout=300) ``` ```python title = run(f'yt-dlp --get-title "{url}"', timeout=30).strip() ``` ```python author = run(f'yt-dlp --get-uploader "{url}"', timeout=30).strip() ``` User-controlled paths and filenames are also interpolated: ```python md5 = run(f'certutil -hashfile "{file_path}" MD5 ^| find /v "MD5" ^| find /v "^$"').replace(" ", "") print(f" [上传] {file_path.name} ({size // 1024}KB)...") # pre_import pre = json.loads(run(f'mcporter call tencent-docs manage.pre_import --args {{"file_name":"{file_path.name}","file_size":{size},"file_md5":"{md5}"}}')) ``` Index records are serialized and then inserted into an unquoted shell command: ```python args = json.dumps({"file_id": INDEX_FILE_ID, "sheet_id": INDEX_SHEET_ID, "records": [{"fields": record}]}) run(f'mcporter call tencent-docs smartsheet.add_records --args {args}') ``` ### Technical Analysis The common `run()` helper invokes `subprocess.run()` with `shell=True`. Several callers construct command strings using URLs, file paths, filenames, environment-derived identifiers, or JSON records. Wrapping a value in double quotes is not sufficient shell escaping. An attacker can include a closing quote followed by platform-spec ...[truncated 1900 chars]- Remediation
View remediation
str: result = subprocess.run( cmd, shell=False, capture_output=True, text=True, timeout=timeout, check=False, ) if result.returncode != 0: raise RuntimeError( f"Command failed: {cmd[0]}\n{result.stderr[:300]}" ) return result.stdout.strip() ``` 2. Pass every argument as a separate list element: ```python dump = json.loads(run( ["yt-dlp", "--dump-json", "--no-playlist", url], timeout=30, )) ``` 3. Pass serialized JSON as one process argument: ```python payload = json.dumps(args, ensure_ascii=False) run([ "mcporter", "call", "tencent-docs", "smartsheet.add_records", "--args", payload, ]) ``` 4. Replace the shell-based `certutil` pipeline with Python's `hashlib`, as already implemented in `upload_to_docs.py`. 5. Validate source URLs before invoking `yt-dlp`. Permit only `https` and the explicitly supported platform hostnames. 6. Validate file paths and require the target to be a regular file in an expected user-selected location. 7. Add regression tests containing quotes, command separators, variable expansions, newlines, and platform-specific shell metacharacters. ]]>