T09 · Insecure Skill Coding Practices
- Location
push-news.js:44- Finding
OS Command Injection Through the Keyword Argument
- Content
View full analysis
Vulnerability Details
File Location:
push-news.js:38-49, with attacker-controlled input originating atpush-news.js:155-160
Vulnerability Type: OS command injection
Risk Level: HighVulnerable code:
javascript function fetchNewsRaw(keyword = 'openclaw', count = 5, maxChars = 500) { const scriptPath = path.join(__dirname, 'new.js'); try { const result = execSync(`node "${scriptPath}" "${keyword}" ${count} ${maxChars}`, { encoding: 'utf-8', timeout: 30000 }); return result.trim(); } catch (error) { console.error('获取资讯失败:', error.message); return null; } }Input source:
javascript async function main() { const keyword = process.argv[2] || 'openclaw'; const count = parseInt(process.argv[3]) || 5; const maxChars = parseInt(process.argv[4]) || 500; const target = process.argv[5] || DEFAULT_TARGET; console.log(`[${new Date().toISOString()}] 开始获取 ${keyword} 资讯...`); const rawNews = fetchNewsRaw(keyword, count, maxChars);Technical Analysis
The
keywordvalue is taken directly fromprocess.argv[2]and interpolated into a command string passed tochild_process.execSync. BecauseexecSyncexecutes the string through a shell, shell syntax embedded in the keyword is interpreted before Node.js invokesnew.js.Wrapping the value in double quotes is insufficient. Shell constructs such as command substitution remain active inside double quotes, and an embedded quote can terminate the intended argument and introduce additional commands. The code performs no shell escaping, allowlist validation, or argument separation.
The timeout only limits execution duration and does not prevent injected commands from reading files, modifying data, accessing credentials, or starting background processes.
Attack Path
- An attacker obtains the ability to invoke
push-news.jsor influence its keyword argument ...[truncated 1155 chars]
- An attacker obtains the ability to invoke
- Remediation
View remediation
Remediation Suggestions
Replace shell-based execution with an API that passes arguments as a discrete array:
javascript const { execFileSync } = require('child_process'); const result = execFileSync( process.execPath, [scriptPath, keyword, String(count), String(maxChars)], { encoding: 'utf-8', timeout: 30000, shell: false } );Prefer importing and calling the news-fetching functionality directly rather than starting another process. Export the necessary functions from
new.jsand keep command-line handling in a separate entry point.In addition:
- Validate
keywordlength and permitted characters according to business requirements. - Apply strict upper bounds to
countandmaxChars. - Run the Skill under a dedicated, unprivileged operating-system account.
- Provide only the minimum required environment variables and filesystem permissions.
- Add automated tests containing shell metacharacters, quotes, command substitutions, and newline characters to ensure they are treated strictly as data.
- Validate