T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/send.js:49
- Finding
- Shell Command Injection Through Untrusted Configuration Values## Vulnerability Details **File Location**: `scripts/send.js`, lines 49–50 **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```js function sendImage(userId, imageUrl) { const cmd = `openclaw message send --channel=feishu --target=${userId} --media="${imageUrl}"`; execSync(cmd, { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }); } ``` The values passed into this function originate from `config.json`. The relevant configuration loading and validation occur in `scripts/send.js`: ```js const userId = config.feishu?.userId; const baseUrl = config.settings?.baseUrl || 'https://img.owspace.com/Public/uploads/Download'; validateUserId(userId); const { label, url } = getTodayImageUrl(baseUrl); ``` The validation at lines 29–34 only requires the user ID to begin with `ou_`: ```js function validateUserId(id) { if (!id || !id.startsWith('ou_')) { console.error('❌ 飞书用户 ID 无效，请重新配置：node scripts/setup.js'); process.exit(1); } } ``` ### Technical Analysis `execSync()` receives a single command string and executes it through a shell. Both `userId` and `imageUrl` are interpolated into that string without shell-safe argument handling. The `userId` is unquoted and is only subject to a prefix check. A value beginning with `ou_` can therefore still contain shell control operators such as `;`, `&&`, `|`, redirects, or command substitutions. Although `imageUrl` appears between double quotes, this is not sufficient shell escaping. Command substitution forms such as `$(...)` and backticks are still evaluated inside double quotes. The configurable `baseUrl` is not parsed or validated before it becomes part of `imageUrl`. Consequently, anyone able to influence `config.json` or provide a crafted ID during interactive setup can cause arbitrary shell commands to execute when the calendar script runs. ### Attack Path 1. An attacker gains the ability to modify `config.json`, influences configuration deployment, or convinc ...[truncated 1493 chars]
- Remediation
- ## Remediation Suggestions 1. **Remove shell-based command construction.** Use `execFileSync()` or `spawnSync()` with a fixed executable and separate argument array: ```js const { execFileSync } = require('child_process'); function sendImage(userId, imageUrl) { execFileSync( 'openclaw', [ 'message', 'send', '--channel=feishu', `--target=${userId}`, `--media=${imageUrl}`, ], { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], shell: false, } ); } ``` This prevents configuration values from being interpreted as shell syntax. 2. **Apply strict allowlist validation to the Feishu user ID.** Replace the prefix-only check with a complete format check, based on the exact format Feishu guarantees. For example: ```js function validateUserId(id) { if (typeof id !== 'string' || !/^ou_[A-Za-z0-9]+$/.test(id)) { throw new Error('Invalid Feishu user ID'); } } ``` 3. **Validate the configured base URL.** Parse it with the standard `URL` class, require HTTPS, reject embedded credentials, and preferably restrict the hostname to the intended provider: ```js function validateBaseUrl(value) { const parsed = new URL(value); if ( parsed.protocol !== 'https:' || parsed.hostname !== 'img.owspace.com' || parsed.username || parsed.password ) { throw new Error('Invalid image base URL'); } return parsed.toString().replace(/\/$/, ''); } ``` 4. **Validate the final media URL** before passing it to OpenClaw, including its scheme, hostname, and expected path structure. 5. **Protect configuration integrity.** Create `config.json` with restrictive permissions appropriate to the platform and ensure only the owning user can modify it. 6. **Add regression tests** covering semicolons, pipes, redirects, whitespace, quotes, backticks, `$()` substitutions, newl ...[truncated 77 chars]