T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/deploy.js:147
- Finding
- Arbitrary JavaScript Execution Through Unsafe Generated Source Code<![CDATA[ ## Vulnerability Details **File Location**: `scripts/deploy.js:69-82, 147-149` **Vulnerability Type**: Code injection through unsafe source-code generation **Risk Level**: High ### Complete Vulnerable Code Configuration values are accepted without schema validation or escaping: ```javascript function buildConfigFromJson(p) { const raw = fs.readFileSync(p, 'utf8'); const json = JSON.parse(raw); const roleIds = json.roles && json.roles.length ? json.roles : ROLES.map(r => r.id); const selectedRoles = ROLES.filter(r => roleIds.includes(r.id)); const names = {}; for (const r of selectedRoles) names[r.id] = (json.roleNames && json.roleNames[r.id]) || r.dname; return { teamName: json.teamName || 'Alpha Team', workDir: json.workspaceDir || path.join(home, '.openclaw', 'workspace-team'), tz: json.timezone || 'Asia/Shanghai', mh: json.morningHour || 8, eh: json.eveningHour || 18, tm: json.thinkingModel || suggestModel('think', detectModels()) || 'zai/glm-5', em: json.executionModel || suggestModel('exec', detectModels()) || 'zai/glm-4.7', ceoTitle: json.ceoTitle || 'Boss', selectedRoles, names, }; } ``` These values are then inserted directly into executable JavaScript: ```javascript const wsPath = cfg.workDir.replace(/\\/g, '/').replace(home.replace(/\\/g, '/'), '~'); const agentList = prefixedRoles.map(r => ` { id: "${r.pid}", name: "${cfg.names[r.id]}", workspace: "${wsPath}", model: { primary: "${r.think ? cfg.tm : cfg.em}" }, identity: { name: "${cfg.names[r.id]}" } }`).join(',\n'); const allIds = ['main', ...prefixedRoles.map(r => `"${r.pid}"`)].join(', '); w(path.join(cfg.workDir, 'apply-config.js'), `#!/usr/bin/env node\nconst fs = require('fs');\nconst path = require('path');\nconst cfgPath = path.join(process.env.HOME || process.env.USERPROFILE, '.openclaw', 'openclaw.json');\nlet config = JSON.parse(fs.readFileSync(cfgPath, 'utf8'));\nif (!config.agents) config.agents = {};\nif (!Array ...[truncated 2933 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not generate JavaScript by concatenating configuration values into source code. 2. Generate a data-only JSON file with `JSON.stringify`, and use a fixed, reviewed `apply-config.js` implementation to read it. 3. If source generation remains necessary, serialize every inserted string with `JSON.stringify` rather than manually surrounding it with quotation marks. 4. Apply strict schema validation to: - Team prefixes and role IDs. - Role names. - Model identifiers. - Workspace paths. - Numeric hour fields. - Time-zone identifiers. 5. Reject unexpected control characters, line breaks, null bytes, and invalid field types. 6. Display the generated script path and require the user to review it before execution. 7. Add tests containing quotation marks, backslashes, newlines, comment delimiters, and template-literal metacharacters to ensure generated code remains data rather than executable syntax. ]]>