T09 · Insecure Skill Coding Practices
Error
- Location
scripts/virtualbox-utils.ts:14- Finding
Arbitrary Host Command Execution Through Shell Command Injection
- Content
View full analysis
{ try { const { stdout, stderr } = await execAsync(`${VBOXMANAGE} ${args}`); return { stdout, stderr }; } catch (error: any) { throw new Error(`VBoxManage command failed: ${error.message}`); } } ``` Representative vulnerable call sites include: ```ts export async function getVMInfo(vmName: string): Promise> { const { stdout } = await vboxCommand(`showvminfo "${vmName}" --machinereadable`); const info: Record = {}; ``` ```ts export async function takeSnapshot(vmName: string, snapshotName: string, description?: string): Promise { const desc = description ? `--description "${description}"` : ''; await vboxCommand(`snapshot "${vmName}" take "${snapshotName}" ${desc}`); } ``` ```ts export async function exportVM(vmName: string, outputPath: string): Promise { await vboxCommand(`export "${vmName}" --output "${outputPath}"`); } ``` ### Technical Analysis The shared `vboxCommand` helper builds one command string and passes it to `child_process.exec`. Node.js `exec` runs the supplied string through a system shell rather than invoking `VBoxManage` directly with a separated argument array. Numerous exported functions interpolate caller-controlled data into this shell command. Affected values include: - VM names and UUIDs - S ...[truncated 2876 chars]- Remediation
View remediation
{ const { stdout, stderr } = await execFileAsync(VBOXMANAGE, [...args], { shell: false }); return { stdout, stderr }; } ``` 2. Convert every call site to an argument array rather than building command strings: ```ts await vboxCommand(['showvminfo', vmName, '--machinereadable']); await vboxCommand([ 'snapshot', vmName, 'take', snapshotName, ...(description ? ['--description', description] : []) ]); await vboxCommand(['export', vmName, '--output', outputPath]); ``` 3. Treat `VBOXMANAGE_PATH` strictly as one executable path. Do not allow it to contain command-line options or shell syntax. Where practical, resolve it from a trusted configuration source and verify that it references an expected executable. 4. Add runtime validation because TypeScript types do not protect against untrusted JavaScript or deserialized input: - Require finite integers for CPU, memory, port, adapter, timeout, metric period, sample, and disk-size values. - Enforce valid ranges for ports and adapter indices. - Restrict enum-like fields to explicit allowlists. - Reject NUL bytes and malformed path values. 5. Apply least privilege to the process running the Skill. It should use a dedicated account with access only to the VMs and host paths required for the task. 6. Add regression tests containing values with spaces, quotes, `$()`, backticks, semicolons, pipes, and newlines. Verify that these values are passed only as literal arguments and nev ...[truncated 202 chars]