T09 · Insecure Skill Coding Practices
- Location
SKILL.md:154- Finding
OS Command Injection in Diagnostic Helpers
- Content
View full analysis
Vulnerability Details
File Location:
SKILL.md, lines 154–158 and 170–174
Vulnerability Type: OS command injection through unvalidated shell interpolation
Risk Level: HighVulnerable Code
javascript checkPort(port) { try { const isWindows = process.platform === 'win32'; const cmd = isWindows ? `netstat -ano | findstr :${port}` : `lsof -i :${port}`; const output = execSync(cmd, { encoding: 'utf-8', timeout: this.config.timeout });javascript checkProcess(processName) { try { const isWindows = process.platform === 'win32'; const cmd = isWindows ? `powershell -Command "Get-Process -Name '${processName}' -ErrorAction SilentlyContinue | Select-Object Id"` : `pgrep -x "${processName}"`; const output = execSync(cmd, { encoding: 'utf-8' });Technical Analysis
The
checkPort()andcheckProcess()methods interpolateportandprocessNamedirectly into command strings passed tochild_process.execSync(). Neither value is validated, constrained to an expected format, nor safely escaped.Because
execSync()executes the resulting string through a command shell, shell metacharacters embedded in either argument can alter the intended command structure. AlthoughcheckFile()performs limited input validation, equivalent controls are absent from these command-execution methods.The code appears as an implementation example in the skill document. The vulnerability becomes exploitable when an agent or developer copies or implements the documented component and passes untrusted task input to these methods.
Attack Path
- An attacker supplies a crafted port value or process name through a task, configuration, API parameter, or other input consumed by the diagnostic helper.
- The value is passed to
checkPort()orcheckProcess(). - The method inserts the val ...[truncated 838 chars]
- Remediation
View remediation
Remediation Suggestions
- Replace shell-based
execSync()calls withexecFileSync()orspawnSync()and provide arguments as an array with shell execution disabled. - Validate ports as integers and require the inclusive range
1–65535. - Apply a strict allowlist to process names, permitting only characters required by supported platforms, such as letters, digits, periods, underscores, and hyphens.
- Reject values containing whitespace, quotes, command separators, redirection operators, substitutions, or other shell metacharacters.
- Set explicit timeouts and output-size limits for every child process, including
checkProcess(). - Return an explicit validation error rather than treating every command failure as proof that a port is available or a process is absent.
- Add security tests covering malformed values and shell-injection payloads on Windows and Unix-like platforms.
A safer pattern is:
javascript const { execFileSync } = require('child_process'); function validatePort(port) { const value = Number(port); if (!Number.isInteger(value) || value < 1 || value > 65535) { throw new TypeError('Port must be an integer between 1 and 65535'); } return String(value); } function validateProcessName(processName) { if ( typeof processName !== 'string' || !/^[A-Za-z0-9._-]{1,255}$/.test(processName) ) { throw new TypeError('Invalid process name'); } return processName; } // Unix-like examples; use platform-specific executable arguments on Windows. const output = execFileSync( 'lsof', ['-i', `:${validatePort(port)}`], { encoding: 'utf-8', timeout: 5000 } ); const processOutput = execFileSync( 'pgrep', ['-x', validateProcessName(processName)], { encoding: 'utf-8', timeout: 5000 } );- Replace shell-based