T09 · Insecure Skill Coding Practices
Error
- Location
- index.js:288
- Finding
- Shell Command Injection Through Untrusted Backup Configuration<![CDATA[ ## Vulnerability Details **File Location**: `index.js:25-35`, `index.js:288-301` **Vulnerability Type**: OS command injection through shell command construction **Risk Level**: Critical ### Vulnerable Code ```js function getInstanceId() { const envFile = path.join(OPENCLAW_DIR, '.backup.env'); if (fs.existsSync(envFile)) { const content = fs.readFileSync(envFile, 'utf8'); const match = content.match(/INSTANCE_ID=(.+)/); if (match) return match[1].trim(); } return os.hostname(); } ``` ```js const repoUrl = config.BACKUP_REPO.replace( 'https://', `https://${config.BACKUP_TOKEN}@` ); process.chdir(STAGING_DIR); safeExec('git init'); safeExec('git config user.email "sync@elegant.local"'); safeExec('git config user.name "Elegant Sync"'); safeExec('git add -A'); safeExec('git commit -m "Sync: ' + new Date().toISOString() + '"'); const tag = generateTag(); const branch = getInstanceId(); safeExec(`git push ${repoUrl} HEAD:${branch} --force`); safeExec(`git tag ${tag}`); safeExec(`git push ${repoUrl} ${tag}`); ``` The invoked helper executes each constructed string through a shell: ```js function safeExec(cmd, options = {}) { try { return execSync(cmd, { ...options, encoding: 'utf8', stdio: 'pipe' }); } catch (err) { const config = loadConfig(); const token = config.BACKUP_TOKEN; let msg = err.message; if (token) msg = msg.replace(token, '***TOKEN***'); error(`Git error: ${msg}`); } } ``` ### Technical Analysis `INSTANCE_ID`, `BACKUP_REPO`, and `BACKUP_TOKEN` are loaded from `~/.openclaw/.backup.env`. They are interpolated into strings passed to `execSync`, which executes strings through a command shell. The repository URL validation only checks the parsed hostname. It does not make shell interpolation safe and does not validate the token or instance identifier. Shell metacharacters in an attacker-controlled configuration value can therefore terminate or extend the intended G ...[truncated 1121 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace shell-string execution with argument-array execution: ```js const { execFileSync } = require('child_process'); execFileSync( 'git', ['push', authenticatedRepoUrl, `HEAD:${branch}`, '--force'], { cwd: STAGING_DIR, encoding: 'utf8', stdio: 'pipe', shell: false } ); ``` 2. Validate `INSTANCE_ID` with an allowlist suitable for Git branch names, such as a restrictive alphanumeric pattern, and additionally validate it with `git check-ref-format --branch`. 3. Reject control characters, whitespace, and shell metacharacters in configuration fields. 4. Do not place credentials in the repository URL. Use a credential helper or temporary `GIT_ASKPASS` integration. 5. Apply the same argument-array approach to every Git invocation, even commands that currently use constant values. 6. Restrict the configuration file to the owning user, ideally with mode `0600`. ]]>