T09 · Insecure Skill Coding Practices
Error
- Location
- src/ssh/config.ts:13
- Finding
- Local Command Injection Through Unsanitized SSH Host Alias## Vulnerability Details **File Location**: `src/ssh/config.ts:13-20` **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```ts /** Use `ssh -G <host>` to resolve effective SSH config for a host */ function resolveWithSshG(alias: string): Partial<HostConfig> | null { try { const output = execSync(`ssh -G ${alias} 2>/dev/null`, { encoding: 'utf-8', timeout: 5000, }); ``` Relevant unsanitized input flow from `src/cli.ts:195-208`: ```ts case 'run': { if (positional.length < 2) { process.stderr.write('Usage: ssh-lab run <host|all> <command...>\n'); process.exit(1); } const host = positional[0]; const cmd = positional.slice(1).join(' '); const result = await runCommand(host, cmd, timeoutFor('standard'), { concurrency }); render(result, mode); process.exit(exitCode(result)); break; } ``` ### Technical Analysis The `alias` value is incorporated directly into a command string passed to Node.js `execSync`. Unlike `execFileSync` with a discrete argument array, `execSync` runs the string through a shell. Consequently, shell metacharacters in an attacker-controlled alias—such as semicolons, command substitutions, backticks, pipes, redirections, or newlines—are interpreted as local shell syntax. Host aliases originate from CLI positional arguments and are not validated before reaching `resolveHost()` and then `resolveWithSshG()`. The same vulnerable resolution path can be reached through multiple host-taking commands, including `run`, `doctor`, `tail`, `ls`, `df`, `watch`, `sync`, `status`, and `compare`. The command is executed locally, not on the intended remote SSH server. Redirecting standard error does not neutralize the injected syntax. ### Attack Path 1. An attacker persuades a user or AI Agent to invoke a host-taking command with a crafted alias. 2. The CLI stores the supplied al ...[truncated 1446 chars]
- Remediation
- ## Remediation Suggestions Replace shell-string execution with an API that passes arguments directly to the executable: ```ts import { execFileSync } from 'node:child_process'; function resolveWithSshG(alias: string): Partial<HostConfig> | null { try { const output = execFileSync('ssh', ['-G', '--', alias], { encoding: 'utf-8', timeout: 5000, stdio: ['ignore', 'pipe', 'ignore'], }); // Parse the output as before. } catch { return null; } } ``` Apply defense in depth: 1. Validate aliases against a conservative allowlist appropriate for SSH host aliases. 2. Reject empty aliases, control characters, whitespace, shell metacharacters, and values beginning with `-`. 3. Use `spawn`, `execFile`, or `execFileSync` with argument arrays for every local program invocation. 4. Never attempt to fix this solely by manually adding shell quotes; eliminating the shell is safer. 5. Add regression tests covering aliases containing `;`, `|`, `&`, backticks, `$()`, redirections, quotes, newlines, and option-like prefixes. 6. Review every host-taking command to ensure all paths use the same validated resolution function. 7. Consider validating custom hostnames, usernames, identity paths, ports, rsync paths, and exclusions separately according to their destination parser.