T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/detect-command.js:174
- Finding
- Command Injection During PATH-Based Command Detection<![CDATA[ ## Vulnerability Details **File Location**: `scripts/detect-command.js:174-184` **Vulnerability Type**: OS command injection in detection logic **Risk Level**: Critical ### Vulnerable Code ```javascript function existsInPath(cmd) { try { const initCmd = getShellInitCommand(); if (IS_WINDOWS) { execSync(`where ${cmd}`, { stdio: 'ignore' }); } else { // Source user's shell init files to get full PATH execSync(`${initCmd}which ${cmd}`, { stdio: 'ignore', timeout: 5000 }); } return true; } catch (e) { return false; } } ``` The value passed as `cmd` comes from the first whitespace-delimited portion of user-controlled input: ```javascript function getFirstWord(input) { return input.trim().split(/\s+/)[0].toLowerCase(); } ``` ### Technical Analysis The detector interpolates `cmd` directly into a shell command executed through `execSync`. No quoting, character validation, or argument separation is applied. Shell metacharacters contained in the first token are therefore interpreted by the shell. This flaw occurs during classification, before `isDangerous()` can affect the final decision. Consequently, the supposedly detection-only interface can execute commands even when the final classification would be `ASK` or `LLM`. The Windows branch has the same underlying issue because it constructs `where ${cmd}` using untrusted input. ### Attack Path 1. An attacker supplies input whose first token contains a shell separator, such as: ```text nonexistent;id ``` 2. `getFirstWord()` returns `nonexistent;id`. 3. `calculateScore()` calls `existsInPath()` with that value. 4. On Unix-like systems, the generated shell text is equivalent to: ```bash source ~/.zshrc 2>/dev/null; which nonexistent;id ``` 5. The shell runs `id`, even though the Skill is only attempting to determine whether the first word exists in `PATH`. 6. The same primitive can be replaced with file modification, credential acce ...[truncated 625 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Do not construct a shell command for executable discovery. - Use an argument-vector API without a shell: ```javascript const { spawnSync } = require('child_process'); function existsInPath(cmd) { if (!/^[A-Za-z0-9._+-]+$/.test(cmd)) { return false; } const program = process.platform === 'win32' ? 'where' : 'which'; const result = spawnSync(program, [cmd], { shell: false, stdio: 'ignore', timeout: 5000, env: buildRestrictedEnvironment() }); return result.status === 0; } ``` - Reject command names containing shell operators, control characters, quotes, substitutions, redirections, or path traversal. - Do not source shell initialization files during detection. - Run detection in a side-effect-free process with a minimal, allowlisted environment. - Add regression tests for inputs containing `;`, `&&`, `||`, `|`, backticks, `$()`, newlines, redirections, and Windows command separators. ]]>