T09 · Insecure Skill Coding Practices
Error
- Location
- skill.yaml:14
- Finding
- OS Command Injection Through Unsanitized Git Operation Parameters## Vulnerability Details **File Location**: `skill.yaml`, lines 14-17 **Vulnerability Type**: OS command injection **Risk Level**: High The affected handlers directly interpolate caller-controlled parameters into shell commands: ```javascript 'create-branch': async (name) => exec(`git checkout -b ${name}`), 'merge': async (source, target) => exec(`git merge ${source} into ${target}`), 'rebase': async (branch) => exec(`git rebase ${branch}`), 'cherry-pick': async (commit) => exec(`git cherry-pick ${commit}`), ``` ### Technical Analysis The `name`, `source`, `target`, `branch`, and `commit` values are inserted into command strings passed to `exec()`. This API normally executes the supplied string through a system shell. No validation, escaping, or separation between executable arguments and shell syntax is performed. Consequently, a parameter containing shell metacharacters such as semicolons, command substitutions, pipes, or redirection operators can alter the intended command and append arbitrary shell operations. For example, a branch parameter shaped like `main; id` would result in a shell command equivalent to: ```bash git rebase main; id ``` The merge implementation has an additional correctness issue: it generates `git merge <source> into <target>`, which is not valid Git merge syntax. Moreover, the dispatcher passes only `options.params` to each handler, so the declared `target` parameter may remain undefined. These defects do not prevent injection through the interpolated `source` value. ### Attack Path 1. An attacker or untrusted caller invokes the Skill through one of its exposed Git operation triggers. 2. The caller selects a parameterized operation such as `create-branch`, `merge`, `rebase`, or `cherry-pick`. 3. The caller supplies an operation parameter containing shell syntax, such as `main; id`. 4. The handler inserts that value directly into a template literal. 5. `exec()` sends th ...[truncated 777 chars]
- Remediation
- ## Remediation Suggestions 1. Replace shell-string `exec()` calls with a non-shell process API such as `execFile()` or `spawn()`, passing every Git argument as a separate array element: ```javascript execFile('git', ['checkout', '-b', name]); execFile('git', ['rebase', branch]); execFile('git', ['cherry-pick', commit]); ``` 2. Validate branch and reference names before execution. Prefer invoking `git check-ref-format --branch` without a shell, or enforce an equivalently strict validation policy. 3. Restrict cherry-pick input to an expected Git object identifier or validated revision expression. If only commit hashes are required, use a strict hexadecimal length policy and verify the object through Git. 4. Reject control characters, whitespace where unnecessary, shell metacharacters, option-like values beginning with `-`, and unexpected parameter types. Do not rely on shell escaping as the primary defense. 5. Redesign the merge handler and its input contract. A standard merge normally requires only the source reference in the currently checked-out target branch. If a target branch is supported, check it out separately using argument-array execution and then merge the validated source. 6. Add tests using hostile inputs containing semicolons, command substitution, pipes, redirections, newlines, and leading hyphens. Verify that such input is rejected and cannot create files or execute secondary commands. 7. Run the Skill with the least-privileged operating-system account required for repository access to reduce the impact of any future command-execution defect.