T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:151
- Finding
- Shell Command Injection Through Unescaped Review Finding Data<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md:151-163` **Vulnerability Type**: Shell command injection through unsafe interpolation **Risk Level**: High ### Vulnerable Code ```markdown **Deferred capture for backlog findings:** Findings that are triaged to the backlog (out-of-scope for the current review or deferred by the team) should be preserved so they are not lost between review cycles. For each finding assigned to the backlog, run: ```bash python3 scripts/deferred_capture.py \ --title "<finding title>" \ --source review \ --context "Review dimension: <dimension>. <finding description>" ``` The `<dimension>` value should match the review skill that surfaced the finding (e.g. `bug-review`, `api-review`, `architecture-review`). This runs automatically after the action plan is finalised, without prompting the user. ``` ### Technical Analysis The Skill instructs the agent to interpolate finding titles, dimensions, and descriptions into a shell command. These values can be derived from content in the repository under review and therefore may be attacker-controlled. Wrapping a value in double quotes does not neutralize shell metacharacters. Shell command substitutions such as `$(command)` and backtick expressions are still evaluated inside double-quoted strings. If a malicious value is inserted verbatim into either `--title` or `--context`, the shell can execute the embedded command before `deferred_capture.py` receives its arguments. For example, a finding description containing `$(attacker_command)` could produce a command structurally equivalent to: ```bash python3 scripts/deferred_capture.py \ --title "Example finding" \ --source review \ --context "Review dimension: bug-review. $(attacker_command)" ``` The vulnerability is aggravated by the explicit instruction to perform the operation automatically without asking the user for confirmation. Exploitation requires the reviewing agent to reproduce attacker-controlled rep ...[truncated 1740 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Do not construct shell command strings using review-derived values. 1. Invoke the capture script through an argument-array API with shell processing disabled. For example: ```python subprocess.run( [ "python3", "scripts/deferred_capture.py", "--title", finding_title, "--source", "review", "--context", f"Review dimension: {dimension}. {finding_description}", ], shell=False, check=True, ) ``` 2. Prefer a structured interface, such as passing a JSON document over standard input, so finding text is handled as data rather than executable shell syntax. 3. If only a shell tool is available, apply a proven shell-escaping function independently to every dynamic argument. Do not rely on double quotes or ad hoc character replacement. 4. Validate `dimension` against a strict allowlist of supported review-skill identifiers. Apply reasonable length limits to titles and descriptions. 5. Remove the instruction to execute automatically without confirmation, particularly when the command contains content originating from an untrusted repository. 6. Add regression tests using values containing command substitutions, backticks, quotes, newlines, semicolons, redirection operators, and option-like prefixes. Verify that every value reaches the capture script literally and that no secondary command is executed. 7. Audit `scripts/deferred_capture.py` separately when it is available, including its handling of arguments, file paths, subprocesses, and output destinations. ]]>