T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:27
- Finding
- Shell Command Injection Through Unsafely Interpolated Tool Parameters## Vulnerability Details **File Location**: `SKILL.md`, lines 27-32, 47-55, and 66-69 **Vulnerability Type**: OS command injection through shell command templates **Risk Level**: High The skill directly interpolates caller-controlled values into shell commands. Several values are enclosed in double quotes, while `status` and `priority` are not quoted at all. ```bash /home/david/.cargo/bin/tickrs task list --json \ {{#if project}}--project-name "{{project}}"{{/if}} \ {{#if status}}--status {{status}}{{/if}} ``` ```bash /home/david/.cargo/bin/tickrs task create --json \ --title "{{title}}" \ {{#if content}}--content "{{content}}"{{/if}} \ {{#if date}}--date "{{date}}"{{/if}} \ {{#if project}}--project-name "{{project}}"{{/if}} \ {{#if priority}}--priority {{priority}}{{/if}} ``` ```bash /home/david/.cargo/bin/tickrs task complete "{{id}}" --json ``` ### Technical Analysis The dynamic parameters `project`, `status`, `title`, `content`, `date`, `priority`, and `id` are embedded in command strings without shell-safe argument handling. Double quotes do not provide adequate protection when an attacker can inject a double quote that terminates the intended argument. The attacker may then append shell metacharacters and an arbitrary command. For example, a malicious title shaped like: ```text "; arbitrary_command; # ``` can terminate the `--title` value, introduce another command, and comment out the remaining generated command. The unquoted `status` and `priority` substitutions are even more directly exposed to shell separators, expansions, redirections, and substitutions. The documented enum restrictions for `status` and `priority` do not establish that runtime validation is enforced. Therefore, they cannot be relied upon as a security boundary. ### Attack Path 1. An attacker or untrusted caller supplies a crafted value for a tool parameter such as `title`, `content`, `project`, `id`, ...[truncated 1526 chars]
- Remediation
- ## Remediation Suggestions 1. Do not construct shell command strings with interpolated parameters. Invoke `/home/david/.cargo/bin/tickrs` directly through an execution API that accepts a program path and an argument array, with shell processing disabled. ```text executable: /home/david/.cargo/bin/tickrs arguments: ["task", "create", "--json", "--title", title] shell: false ``` 2. Append optional flags and their values as separate argument-array elements. Never concatenate user input with flag syntax. 3. Enforce strict allowlists for enumerated fields: - `status`: only `incomplete` or `complete` - `priority`: only `none`, `low`, `medium`, or `high` 4. Validate task IDs against the exact format emitted by `tickrs`. Reject values containing whitespace, quotes, control characters, or shell metacharacters. 5. Apply reasonable length and character restrictions to project names, titles, content, and dates. Treat these checks as defense in depth rather than a substitute for shell-free execution. 6. If a shell is absolutely unavoidable, use a proven argument-escaping routine for the exact target shell on every dynamic value. Do not implement escaping through simple quote replacement. 7. Add negative tests using quotes, semicolons, command substitutions, newlines, redirections, and backticks to verify that malicious input is passed only as literal data.