T09 · Insecure Skill Coding Practices
Error
- Location
- index.js:14
- Finding
- Path Traversal Enables File Overwrite Outside the Skills Root## Vulnerability Details **File Location**: `index.js`, lines 7 and 14–16, with the write operation at line 63 **Vulnerability Type**: Path traversal and unsafe file overwrite **Risk Level**: High ### Vulnerable Code ```js const SKILLS_ROOT = path.resolve(__dirname, '..'); const TARGET_SKILL = process.argv[2]; ``` ```js const skillPath = path.join(SKILLS_ROOT, TARGET_SKILL); const indexJsPath = path.join(skillPath, 'index.js'); const testJsPath = path.join(skillPath, 'test.js'); if (!fs.existsSync(indexJsPath)) { console.error(`Skill ${TARGET_SKILL} not found or index.js missing.`); process.exit(1); } ``` ```js fs.writeFileSync(testJsPath, testContent); ``` ### Technical Analysis `TARGET_SKILL` is accepted directly from the command line and passed to `path.join` without validation or a containment check. Values containing traversal components such as `..` can cause `skillPath` to reference a directory outside `SKILLS_ROOT`. The existence check only confirms that an `index.js` file exists at the selected location. It does not establish that the target is an authorized skill directory or that its canonical path remains beneath `SKILLS_ROOT`. After this check, the program unconditionally writes `test.js`, overwriting an existing file with that name. Symbolic links can also undermine simple lexical path assumptions unless canonical paths are checked. Consequently, a caller who controls the CLI argument can select another accessible directory containing `index.js` and overwrite its `test.js`. ### Attack Path 1. The attacker identifies or creates a writable directory outside the intended skills root that contains an `index.js` file. 2. The attacker invokes the generator with a traversal value such as `../../attacker-controlled-directory`, adjusted for the actual directory layout. 3. `path.join(SKILLS_ROOT, TARGET_SKILL)` resolves the traversal components and selects the external directory. 4. The ...[truncated 968 chars]
- Remediation
- ## Remediation Suggestions 1. Restrict skill identifiers to a conservative allowlist: ```js if (!/^[A-Za-z0-9_-]+$/.test(TARGET_SKILL)) { throw new Error('Invalid skill name'); } ``` 2. Resolve and verify the target path before accessing it: ```js const skillPath = path.resolve(SKILLS_ROOT, TARGET_SKILL); const relative = path.relative(SKILLS_ROOT, skillPath); if ( relative === '' || relative.startsWith(`..${path.sep}`) || relative === '..' || path.isAbsolute(relative) ) { throw new Error('Target must be a child of the skills root'); } ``` 3. If symbolic links are permitted in the directory tree, compare canonical paths obtained with `fs.realpathSync` and ensure the canonical target remains inside the canonical skills root. 4. Refuse to replace an existing `test.js` by default. Use an explicit overwrite option or an exclusive write flag such as `{ flag: 'wx' }`. 5. Consider writing generated tests to a safely created temporary directory instead of modifying the target project. 6. Verify that the target is a direct child of the skills root if nested paths are not required.