T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/telegram-handler.js:98
- Finding
- Shell Command Injection Through Telegram Management Arguments<![CDATA[ ## Vulnerability Details **File Location**: `scripts/telegram-handler.js:98-105, 108-116, 132-138, 358-412, 477-482` **Vulnerability Type**: OS command injection through unsafe shell interpolation **Risk Level**: Critical ### Vulnerable Code ```javascript function handleAdd(identifier, name, prompt) { execSync(`node "${MANAGE_SCRIPT}" add "${identifier}" "${prompt}" "${name}"`, { stdio: 'inherit' }); return `✅ Added **${name}** (\`${identifier}\`) to watch list.\n\nRestart watcher to apply changes.`; } function handleRemove(identifier) { const config = loadConfig(); const contact = config.watchList.find(c => c.identifier === identifier); const name = contact ? contact.name : identifier; execSync(`node "${MANAGE_SCRIPT}" remove "${identifier}"`, { stdio: 'inherit' }); return `✅ Removed **${name}** (\`${identifier}\`) from watch list.\n\nRestart watcher to apply changes.`; } function handleDelay(identifier, minutes) { execSync(`node "${MANAGE_SCRIPT}" set-delay "${identifier}" ${minutes}`, { stdio: 'inherit' }); ``` Additional affected handlers use the same construction: ```javascript function handleSetTimeWindow(identifier, start, end) { try { execSync(`node "${MANAGE_SCRIPT}" set-time-window "${identifier}" "${start}" "${end}"`, { encoding: 'utf8' }); ``` ```javascript function handleAddKeyword(identifier, keyword) { try { execSync(`node "${MANAGE_SCRIPT}" add-keyword "${identifier}" "${keyword}"`, { encoding: 'utf8' }); ``` ```javascript function handleRemoveKeyword(identifier, keyword) { try { execSync(`node "${MANAGE_SCRIPT}" remove-keyword "${identifier}" "${keyword}"`, { encoding: 'utf8' }); ``` ```javascript function handleSetDailyCap(identifier, maxReplies) { try { execSync(`node "${MANAGE_SCRIPT}" set-daily-cap "${identifier}" ${maxReplies}`, { encoding: 'utf8' }); ``` The arguments originate directly from the command line: ```javascript const [,, command, ...args] = proces ...[truncated 2767 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Eliminate all shell-string invocation. Use `execFileSync()` or `spawnSync()` with an argument array: ```javascript const { execFileSync } = require('child_process'); execFileSync(process.execPath, [ MANAGE_SCRIPT, 'add', identifier, prompt, name ], { stdio: 'inherit', shell: false }); ``` 2. Apply this change to every affected management handler, including add, remove, delay, time-window, keyword, bulk-delay, and daily-cap operations. 3. Prefer importing management functions as a local module rather than launching another process. 4. Validate contact identifiers against an explicit E.164 policy, such as `^\+[1-9]\d{1,14}$`, before use. 5. Validate numeric inputs with `Number.isInteger()` and enforce safe ranges rather than passing `parseInt()` results unchecked. 6. Restrict time values to the documented `HH:MM` format before process invocation. 7. Limit names, prompts, and keywords by length and reject control characters and null bytes. Validation is defense in depth and must not replace shell-free execution. 8. Ensure the Telegram integration authenticates an explicit owner or allowlist before exposing management commands. 9. After remediation, test all handlers with quotes, backticks, `$()`, semicolons, newlines, and leading option-like values to verify they remain literal arguments. ]]>