T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/verify_execute_verify.py:9
- Finding
- Arbitrary Shell Command Execution with Fail-Open Triggering## Vulnerability Details **File Location**: `scripts/verify_execute_verify.py:9-10, 24-25, 31-38` **Vulnerability Type**: OS command injection and unsafe failure handling **Risk Level**: High ### Vulnerable Code ```python def run(cmd: str): p = subprocess.run(cmd, shell=True, capture_output=True, text=True) return { "cmd": cmd, "code": p.returncode, "stdout": p.stdout.strip(), "stderr": p.stderr.strip(), } ``` ```python ap.add_argument("--verify-cmd", required=True, help="command that outputs verifier JSON") ap.add_argument("--execute-cmd", required=True, help="executor command to run when no progress") ``` ```python before_raw = run(args.verify_cmd) before = parse_json(before_raw["stdout"]) or {"progress_detected": False, "parse_error": True} triggered = False exec_raw = None if not before.get("progress_detected", False): triggered = True exec_raw = run(args.execute_cmd) time.sleep(args.sleep_sec) ``` ### Technical Analysis The script accepts `--verify-cmd` and `--execute-cmd` as command strings and passes both directly to `subprocess.run()` with `shell=True`. Consequently, shell metacharacters, command separators, redirections, pipelines, and command substitutions embedded in either argument are interpreted by the operating-system shell. The execution decision also fails open. If the verifier produces malformed JSON, no output, or output that cannot be decoded, `parse_json()` returns `None`. The fallback object sets `progress_detected` to `False`, which automatically enters the executor branch. The script does not require the verifier process to return a successful exit status before trusting its output, and the final result reports `"ok": true` regardless of subprocess failures. This is exploitable when an untrusted user, automation component, configuration source, or agent-generated value can influence either command argument. Direct command-line access already permits intentional command exe ...[truncated 2100 chars]
- Remediation
- ## Remediation Suggestions 1. **Eliminate shell interpretation.** Accept commands as argument arrays and invoke them with `shell=False`: ```python def run(argv: list[str]): p = subprocess.run( argv, shell=False, capture_output=True, text=True, check=False, ) ``` 2. **Prefer dedicated structured options.** Instead of accepting an unrestricted verifier command, call the known verifier script directly and expose only validated parameters such as project directory and time window. 3. **Use an allowlist for executor operations.** If multiple executors must be supported, map fixed identifiers to predefined argument arrays. Do not concatenate user-controlled values into command strings. 4. **Fail closed on verifier errors.** Abort without invoking the executor if: - The verifier returns a nonzero exit code. - Standard output is empty or malformed. - The decoded value is not a JSON object. - `progress_detected` is absent or is not a Boolean. 5. **Return accurate status information.** Set the top-level `ok` field to `false` when verification, execution, parsing, or re-verification fails. 6. **Apply execution safeguards.** Add subprocess timeouts, restrict the working directory and environment, run under a least-privileged service account, and avoid exposing unnecessary secrets to child processes. 7. **If compatibility requires parsing a trusted command string**, parse it once into an argument list with `shlex.split()` and still use `shell=False`. This is not a substitute for validation when the command source is untrusted.