T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:21
- Finding
- Shell Command Injection Through Untrusted Workflow Parameters<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md:21-58` **Vulnerability Type**: Shell command injection caused by unsafe interpolation of user-controlled values **Risk Level**: High ### Vulnerable Code ```bash ls <docs_dir>/.cache/index.json ``` ```bash python3 ~/.claude/skills/doc-search/scripts/build_index.py <docs_dir> ``` ```bash python3 ~/.claude/skills/doc-search/scripts/search.py "<expanded query>" \ --docs-dir <docs_dir> --topk 5 ``` ```bash grep -ni -e "keyword1" -e "keyword2" /path/to/doc.md ``` ### Technical Analysis The Skill instructs the Agent to construct shell commands by substituting document directories, expanded queries, original keywords, and result paths directly into command templates. The directory and result-path placeholders are unquoted. Consequently, whitespace, command separators, redirection operators, pipelines, and other shell metacharacters in these values can alter the command's structure. Although the expanded query and grep patterns are shown inside double quotes, double quotes do not neutralize shell command substitution. Constructs such as `$(command)` and backtick substitution can still be evaluated by a shell when the Agent builds and executes a command string. Embedded quotation marks may also terminate the intended quoting context if values are interpolated without robust argument-level escaping. The risk applies both to explicitly supplied user input and to filenames originating from a document collection that may not be fully trusted. The Python scripts themselves do not invoke a shell, but the Skill workflow directs the Agent to do so. ### Attack Path 1. An attacker supplies a document directory, search query, keyword, or indexed filename containing shell syntax. 2. The Agent replaces a placeholder in one of the documented Bash templates with that value. 3. The resulting command is passed to a shell rather than executed as a fixed argument array. 4. The shell interprets command substituti ...[truncated 956 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not build shell command strings by interpolating user-controlled values. 2. Invoke the Python scripts through an API that accepts an argument array, for example: ```python subprocess.run( ["python3", script_path, expanded_query, "--docs-dir", docs_dir, "--topk", "5"], shell=False, check=True, ) ``` 3. Use a dedicated file-search API instead of generating a `grep` shell command. If `grep` must be used, pass every pattern and file path as a separate process argument with `shell=False`. 4. Place `--` before file operands so paths beginning with a hyphen cannot be interpreted as options: ```python subprocess.run( ["grep", "-ni", "-e", keyword1, "-e", keyword2, "--", file_path], shell=False, check=False, ) ``` 5. Resolve and validate the document directory before use. Reject paths containing null bytes and ensure that result paths remain within the approved document root. 6. Do not rely on model-generated quoting or escaping. Safe process invocation must be enforced structurally by the implementation. 7. Update `SKILL.md` to explicitly prohibit shell-string execution and require argument-array invocation for all untrusted values. ]]>