T09 · Insecure Skill Coding Practices
Error
- Location
- agent.js:186
- Finding
- Local Command Injection Through Unsanitized SSH Key Paths and VM Names<![CDATA[ ## Vulnerability Details **File Location**: `agent.js:186-199`, with attacker-controlled values accepted at `agent.js:377-382` **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```js async function ensureDir(d) { try { await execCmd(`mkdir -p ${d}`); } catch (e) {} } async function genSshKey(vmName, keyDir, keyType = 'ed25519') { const base = path.join(keyDir, vmName); const priv = base, pub = base + '.pub'; try { await execCmd(`ssh-keygen -t ${keyType} -f ${base} -N ''`); } catch (e) { const uniqueId = `${vmName}-${Date.now()}`; await writeFile(priv, `-----BEGIN PRIVATE KEY-----\nPlaceholder for ${uniqueId}\n-----END PRIVATE KEY-----`); await writeFile(pub, `${keyType} AAAAC3NzaC1lZDI1NTE5AAAAI... placeholder - ${uniqueId}`); } await execCmd(`chmod 600 ${priv} ${pub}`); return { privateKey: priv, publicKey: pub }; } ``` The affected values are obtained from configuration or interactive input: ```js let sshKeysDir = env['Key Path'] || path.join(process.env.HOME, '.ssh', 'pve-builder'); const customDir = await ask(`17. SSH keys directory (default: ${sshKeysDir}): `); if (customDir && customDir.trim() !== '') sshKeysDir = customDir.trim(); await ensureDir(sshKeysDir); console.log(`Generating SSH key for ${vmName} in ${sshKeysDir}...`); const keys = await genSshKey(vmName, sshKeysDir, env['Key Type'] || 'ed25519'); ``` ### Technical Analysis The SSH key directory, VM name, and configured key type are interpolated directly into shell command strings. `execCmd()` ultimately invokes `child_process.exec()` in its local fallback, which executes its argument through a shell. Neither shell escaping nor strict input validation is applied. Consequently, shell metacharacters, command substitutions, redirections, whitespace, and option-like values can change the meaning of the resulting commands. Path construction with `path.join()` does not make a value safe for shell interpretation. The SSH ...[truncated 1867 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace shell-based directory creation with the Node.js filesystem API: ```js await fs.promises.mkdir(sshKeysDir, { recursive: true, mode: 0o700 }); ``` 2. Replace `child_process.exec()` with `spawn()` or `execFile()` and pass every argument separately: ```js const { execFile } = require('child_process'); execFile('ssh-keygen', [ '-t', validatedKeyType, '-f', base, '-N', '' ], callback); ``` 3. Use `fs.chmod()` for permissions rather than executing `chmod` through a shell. 4. Restrict VM names to an explicit Proxmox-compatible pattern, such as letters, digits, periods, underscores, and hyphens, with a conservative maximum length. 5. Permit only an explicit set of supported key types, such as `ed25519`, rather than accepting arbitrary configuration content. 6. Resolve and normalize the key directory, reject control characters, and optionally constrain it to an approved base directory. 7. Remove the placeholder-key fallback. If key generation fails, abort and report the error without claiming that a valid key pair was created. 8. Set the key directory to mode `0700`, the private key to `0600`, and use an appropriate public-key mode such as `0644`. ]]>