T09 · Insecure Skill Coding Practices
Error
- Location
- install.js:758
- Finding
- Shell Command Injection Through Unescaped CLI Arguments and Repository URLs<![CDATA[ ## Vulnerability Details **File Location**: `install.js:758-771`, `install.js:793-797`, and `install.js:818-840` **Vulnerability Type**: OS command injection **Risk Level**: Critical ### Vulnerable Code ```javascript const flags = args.filter(a => a.startsWith('--')); const rawTarget = process.argv[2]; execSync(`ldm install ${rawTarget} ${flags.join(' ')}`, { stdio: 'inherit' }); ``` The same unsafe delegation is repeated after bootstrapping LDM OS: ```javascript const flags = args.filter(a => a.startsWith('--')); const rawTarget = process.argv[2]; try { execSync(`ldm install ${rawTarget} ${flags.join(' ')}`, { stdio: 'inherit' }); process.exit(0); } catch (delegateErr) { if (!JSON_OUTPUT) console.error(' ldm install failed. Falling back to standalone installer.'); } ``` The standalone clone fallback also interpolates an untrusted URL into shell commands: ```javascript if (target.startsWith('http') || target.startsWith('git@') || target.match(/^[\w-]+\/[\w.-]+$/)) { const isShorthand = target.match(/^[\w-]+\/[\w.-]+$/); const httpsUrl = isShorthand ? `https://github.com/${target}.git` : target; const sshUrl = isShorthand ? `git@github.com:${target}.git` : target.replace(/^https:\/\/github\.com\//, 'git@github.com:'); const repoName = basename(httpsUrl).replace('.git', ''); repoPath = join('/tmp', `wip-install-${repoName}`); try { if (existsSync(repoPath)) { execSync(`rm -rf "${repoPath}"`); } try { execSync(`git clone "${httpsUrl}" "${repoPath}"`, { stdio: 'pipe' }); } catch { if (existsSync(repoPath)) execSync(`rm -rf "${repoPath}"`); execSync(`git clone "${sshUrl}" "${repoPath}"`, { stdio: 'pipe' }); } } } ``` ### Technical Analysis `execSync()` executes string commands through a shell. `rawTarget`, arbitrary arguments beginning with `--`, `httpsUrl`, and `sshUrl` are incorporated without shell escaping or strict validation. Quotation marks around the clone UR ...[truncated 1298 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Replace every string-form `execSync()` invocation with `execFileSync()` or `spawnSync()` and pass arguments as an array: ```javascript execFileSync('ldm', ['install', target, ...validatedFlags], { stdio: 'inherit', }); execFileSync('git', ['clone', '--', httpsUrl, repoPath], { stdio: 'pipe', }); ``` - Allowlist supported flags instead of accepting every argument beginning with `--`. - Parse repository URLs with `new URL()` and allow only explicitly supported protocols and hosts. - Reject targets containing control characters, NUL bytes, or unsupported URL syntax. - Replace `rm -rf` shell commands with `fs.rmSync(path, { recursive: true, force: true })`. - Add automated regression tests using semicolons, quotes, command substitutions, newlines, and option-like repository names. ]]>