T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/employee-manager.js:53
- Finding
- Path Traversal Through Attacker-Controlled Employee Role and Identifier<![CDATA[ ## Vulnerability Details **File Location**: `scripts/employee-manager.js`, lines 53–82 **Vulnerability Type**: Path traversal leading to unauthorized directory creation and file writes **Risk Level**: High ### Vulnerable Code ```javascript hire(config) { const { name, role, skills = [], model = 'gpt-4', template } = config; if (!name || !role) { throw new Error('员工名称和角色不能为空'); } const id = `${role.toLowerCase().replace(/\s+/g, '-')}-${Date.now()}`; const employee = { id, name, role, skills, model, status: 'idle', createdAt: new Date().toISOString(), currentTask: null, completedTasks: 0, memoryPath: `./memory/${id}` }; // 创建记忆目录 const memDir = path.join(MEMORY_DIR, id); if (!fs.existsSync(memDir)) { fs.mkdirSync(memDir, { recursive: true }); // 创建初始记忆文件 fs.writeFileSync( path.join(memDir, 'memory.md'), `# ${name} - 记忆存储\n\n创建时间: ${new Date().toISOString()}\n\n## 工作记录\n\n`, 'utf8' ); } ``` The unsafe identifier is also reused by other filesystem operations: ```javascript const memDir = path.join(MEMORY_DIR, id); fs.renameSync(memDir, path.join(archiveDir, id)); ``` ```javascript const memFile = path.join(MEMORY_DIR, id, 'memory.md'); fs.appendFileSync(memFile, entry, 'utf8'); ``` ### Technical Analysis The employee identifier is derived directly from the caller-controlled `role` value. The only normalization replaces whitespace with hyphens; it does not reject `..` components, forward slashes, backslashes, or other path syntax. The resulting identifier is passed to `path.join(MEMORY_DIR, id)` and subsequently used by directory creation, file creation, memory append, and archive operations. A role containing traversal components can therefore cause the normalized path to escape the intended `memory/` directory. Appending the current timestamp to the identifier does not prevent traversal. It only changes the final path component after the attacke ...[truncated 1655 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Generate employee identifiers independently of user-controlled fields, preferably with `crypto.randomUUID()`: ```javascript const { randomUUID } = require('crypto'); const id = randomUUID(); ``` 2. Store the human-readable role only as metadata and never use it as a directory or filename. 3. Introduce a centralized containment check for every filesystem operation: ```javascript function safeChildPath(baseDir, childName) { const base = path.resolve(baseDir); const target = path.resolve(base, childName); if (target !== base && !target.startsWith(base + path.sep)) { throw new Error('Path escapes the permitted directory'); } return target; } ``` 4. If legacy identifiers must be supported, validate them against a strict allowlist such as `/^[a-zA-Z0-9_-]+$/` and reject path separators, `..`, null bytes, and absolute paths. 5. Apply validation not only during hiring, but also before `fire()`, `writeMemory()`, `readMemory()`, and all archive operations. Persisted configuration must be treated as untrusted input. 6. Run the process under a dedicated, least-privileged operating-system account and restrict filesystem permissions to the project’s required data directories. 7. Add tests covering traversal inputs such as `../`, `../../target`, mixed separators, encoded separators, and malicious identifiers loaded from `employees.json`. ]]>