T09 · Insecure Skill Coding Practices
Error
- Location
- safe-shell.js:62
- Finding
- Shell command validation can be bypassed through unhandled control operators<![CDATA[ ## Vulnerability Details **File Location**: `safe-shell.js:62-75` **Additional Relevant Location**: `safe-shell.js:100-105`, `SKILL.md:185` **Vulnerability Type**: Command validation bypass and potential shell injection **Risk Level**: High ### Vulnerable Code ```javascript // Check allowlist (must appear at the start of the command) const isAllowed = ALLOWED_COMMANDS.some(cmd => trimmed === cmd || trimmed.startsWith(cmd + ' ') || trimmed.startsWith(cmd + '\t') ); if (!isAllowed) { return { safe: false, reason: 'Command is not in the allowlist', blocked: false }; } return { safe: true }; ``` The approved, unchanged command is subsequently displayed for execution by a separate tool: ```javascript console.log(`Approved command: ${command}`); console.log('Actual execution must be performed through the exec tool'); ``` ### Technical Analysis The allowlist only verifies that the supplied string begins with an approved executable name. It does not parse the command into an executable and arguments, nor does it comprehensively reject shell syntax. The blocked-pattern list does not prohibit all of the following constructs: - Semicolon command sequencing - `&&` and `||` conditional execution - Arbitrary pipelines - Command substitution - Relative-path output redirection - Shell-specific expansion and quoting behaviors Consequently, input beginning with an allowed command can contain an additional unapproved operation. The complete string is then presented as approved and intended to be passed to an external execution tool. This also contradicts the documentation in `SKILL.md:185`, which states that command chains such as `;`, `&&`, and `||` are blocked. The JavaScript file does not directly execute the command, so exploitation depends on an agent or downstream component trusting the validator and passing the approved string to a shell. That downstream behavior is explicitly contemplated by the program's execution message. # ...[truncated 1362 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not pass user-controlled command strings to a shell. 2. Parse requests into a fixed executable and a structured argument array. 3. Invoke the executable with shell processing disabled, for example through `spawn()` or `execFile()` with `shell: false`. 4. Create a separate argument schema for every allowed command. Reject flags that can execute programs, write files, load plugins, or reference unsafe pseudo-filesystems. 5. Reject all shell control operators, including pipes, semicolons, redirections, command substitution, newlines, `&&`, and `||`. 6. Resolve the executable to an explicitly trusted absolute path rather than relying on a potentially attacker-controlled `PATH`. 7. Apply execution limits, including timeouts, output limits, and a restricted working directory. 8. Add regression tests for command chaining, newline injection, command substitution, pipelines, redirection, quoting edge cases, and platform-specific shell syntax. 9. Correct the documentation so that it accurately reflects implemented safeguards. ]]>