T09 · Insecure Skill Coding Practices
- Location
scripts/get-hot-trend.js:13- Finding
OS Command Injection Through an Unvalidated CLI Argument
- Content
View full analysis
Vulnerability Details
File Location:
scripts/get-hot-trend.js, lines 13–19 and line 73
Vulnerability Type: OS command injection
Risk Level: HighVulnerable Code
javascript function getHotTrend(limit = 10) { try { const scriptPath = path.join(__dirname, 'douyin.js'); const output = execSync(`node "${scriptPath}" hot ${limit}`, { encoding: 'utf-8', cwd: path.dirname(scriptPath) });The value passed to the vulnerable function originates directly from a command-line argument:
javascript const limit = process.argv[2] || 10; console.log('开始获取抖音热榜...'); const hotTrendData = getHotTrend(limit);Technical Analysis
The
limitvalue is obtained fromprocess.argv[2]without type validation, range enforcement, or shell escaping. It is then interpolated into a command string passed tochild_process.execSync().execSync()executes the supplied string through a system shell. Consequently, shell metacharacters contained inlimit, such as command separators or command substitutions, are interpreted by the shell rather than treated as a single numeric argument.Although the normal purpose of this value is to specify the number of hot-trend entries, an attacker who can influence the script's CLI arguments can append an arbitrary operating-system command.
Attack Path
-
An attacker gains the ability to invoke the helper script directly or influence arguments supplied by an Agent, cron wrapper, automation workflow, or other caller.
-
The attacker supplies a crafted second argument containing a valid-looking limit followed by a shell command. A harmless proof of concept would be:
bash node scripts/get-hot-trend.js '10; touch /tmp/douyin-command-injection' -
Line 73 assigns the entire crafted string to
limit. -
getHotTrend()interpolates that string into the command passed toexecSync(). -
The system shell e ...[truncated 1199 chars]
-
- Remediation
View remediation
Remediation Suggestions
-
Avoid invoking a shell. Replace
execSync()withexecFileSync()orspawnSync()and pass arguments as a separate array:javascript const { execFileSync } = require('child_process'); function getHotTrend(limit = 10) { try { const scriptPath = path.join(__dirname, 'douyin.js'); const output = execFileSync( process.execPath, [scriptPath, 'hot', String(limit)], { encoding: 'utf-8', cwd: path.dirname(scriptPath) } ); return output; } catch (error) { console.error('Failed to retrieve the Douyin hot list:', error.message); return null; } } -
Validate the argument before execution and reject anything other than a bounded positive integer:
javascript const rawLimit = process.argv[2] ?? '10'; if (!/^\d+$/.test(rawLimit)) { throw new Error('The limit must be a positive integer'); } const limit = Number(rawLimit); if (!Number.isSafeInteger(limit) || limit < 1 || limit > 50) { throw new Error('The limit must be between 1 and 50'); } -
Prefer importing the retrieval logic as a JavaScript module instead of launching another Node.js process. This removes the command-execution boundary entirely.
-
Run the Skill under a least-privileged account with restricted filesystem access and without unnecessary secrets in its environment.
-
Add regression tests using shell metacharacters, command substitutions, whitespace, negative values, excessively large values, and non-numeric input. Tests should verify that invalid values are rejected and that no secondary command is executed.
-