T09 · Insecure Skill Coding Practices
Error
- Location
- tv.js:209
- Finding
- OS Command Injection Through ADB Device and Application Parameters<![CDATA[ ## Vulnerability Details **File Location**: `tv.js:209-216`, `tv.js:242`, `tv.js:294-300`; attacker-controlled application input originates from `cli.js:137` **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```js function adbExec(command, throwOnError = true) { const device = TV_CONFIG.adbDevice; const prefix = device ? `adb -s ${device}` : 'adb'; const fullCmd = `${prefix} ${command}`; try { return execSync(fullCmd, { encoding: 'utf8', timeout: 10000, stdio: ['pipe', 'pipe', 'pipe'] }).trim(); } catch (e) { if (throwOnError) { throw new Error(`ADB failed: ${fullCmd}\n${e.stderr || e.message}`); } return ''; } } ``` The configured device is also inserted into an ADB subcommand: ```js function adbConnect() { const device = TV_CONFIG.adbDevice; if (!device) { throw new Error('ADB_DEVICE not configured. Set it to "TV_IP:5555" (e.g. "192.168.1.100:5555").'); } const devices = adbExec('devices', false); if (devices.includes(device) && !devices.includes('offline')) return; const result = adbExec(`connect ${device}`); ``` The application ID supplied through the CLI is inserted into the shell command: ```js async launchApp(appId) { adbCheckInstalled(); adbConnect(); // monkey is the most reliable universal launcher const result = adbExec(`shell monkey -p ${appId} -c android.intent.category.LAUNCHER 1`, false); if (result.includes('No activities found')) { // Fallback: leanback launcher intent (Android TV specific) adbExec( `shell am start -a android.intent.action.MAIN -c android.intent.category.LEANBACK_LAUNCHER ${appId}`, false ); } console.log(`✅ Launched app: ${appId}`); }, ``` The CLI passes a user-controlled value to this operation: ```js case 'launch': await tv. ...[truncated 2892 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace shell-based execution with argument-based process invocation. For example, use `execFileSync` or `spawnSync`: ```js const { execFileSync } = require('child_process'); function adbExec(args, throwOnError = true) { const device = TV_CONFIG.adbDevice; const adbArgs = device ? ['-s', device, ...args] : args; try { return execFileSync('adb', adbArgs, { encoding: 'utf8', timeout: 10000, stdio: ['pipe', 'pipe', 'pipe'] }).trim(); } catch (e) { if (throwOnError) { throw new Error(`ADB failed: ${e.stderr || e.message}`); } return ''; } } ``` 2. Pass every ADB token as a separate array element, such as: ```js adbExec(['connect', device]); adbExec([ 'shell', 'monkey', '-p', appId, '-c', 'android.intent.category.LAUNCHER', '1' ]); ``` 3. Validate `ADB_DEVICE` against a strict expected format. Permit only a valid IPv4/IPv6 address or approved hostname and a numeric port. Reject whitespace and all shell metacharacters. 4. Validate application package names with a restrictive allowlist, for example: ```js if (!/^[A-Za-z0-9._-]+$/.test(appId)) { throw new Error('Invalid application ID'); } ``` 5. Prefer a configured allowlist of launchable application IDs instead of accepting arbitrary package names from agent or CLI input. 6. Do not send concatenated free-form ADB commands through Home Assistant. If the integration cannot accept structured arguments, strictly validate `appId` before constructing the command and limit the Home Assistant token and Android TV integration to the least privileges possible. 7. Add automated tests that verify shell metacharacters, whitespace, substitutions, redirections, and newline characters are rejected and cannot create side effects. ]]>