T09 · Insecure Skill Coding Practices
Error
- Location
- src/self-repair.js:162
- Finding
- Workspace Repair Path Traversal Allows Writes Outside the Workspace<![CDATA[ ## Vulnerability Details **File Location**: `src/self-repair.js:162-177` **Vulnerability Type**: Path traversal and arbitrary file write **Risk Level**: High ### Vulnerable Code ```javascript for (const backupPath of this.backupPaths) { for (const item of missing) { const source = path.join(backupPath, item); const dest = path.join(this.workspacePath, item); if (fs.existsSync(source)) { const dir = path.dirname(dest); if (!fs.existsSync(dir)) fs.mkdirSync(dir, { recursive: true }); if (item.endsWith('/')) { fs.mkdirSync(dest, { recursive: true }); } else { fs.copyFileSync(source, dest); } this.log('repair', `Restored ${item} from backup`); } } } ``` ### Technical Analysis The values in `requiredFiles`, `requiredDirs`, and `backupPaths` are accepted from configuration and used to construct filesystem paths without validating their canonical locations. An entry containing parent-directory components, such as `../../target/file`, can cause `dest` to resolve outside `workspacePath`. The same issue affects `source`, allowing the repair process to read from locations outside the intended backup directory. For directory entries, the code can create directories outside the workspace. For file entries, `fs.copyFileSync()` can overwrite any destination file writable by the current process. Checking `fs.existsSync(source)` does not establish that the source remains within an approved backup root. The implementation also does not protect against symbolic-link traversal, where a path lexically inside the workspace or backup directory resolves to a location outside it. ### Attack Path 1. An attacker gains influence over the `SelfRepair` configuration, such as through an application configuration file, plugin input, deployment parameter, or other integration that constructs the instance. 2. The attacker supplies a crafted required item containing traversal components, for ex ...[truncated 1504 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Reject absolute paths and any item containing `..` path components. 2. Resolve each destination with `path.resolve()` and verify that it remains beneath the canonical workspace root: ```javascript const workspaceRoot = fs.realpathSync(this.workspacePath); const dest = path.resolve(workspaceRoot, item); const workspacePrefix = workspaceRoot.endsWith(path.sep) ? workspaceRoot : workspaceRoot + path.sep; if (dest !== workspaceRoot && !dest.startsWith(workspacePrefix)) { throw new Error(`Required item escapes workspace: ${item}`); } ``` 3. Apply the same containment validation to every source path relative to its approved backup root. 4. Canonicalize existing source paths with `fs.realpathSync()` before reading them to detect symbolic-link escapes. 5. Before writing, inspect existing parent components with `lstatSync()` and reject symbolic links unless explicitly permitted. 6. Define a strict schema for required items, such as normalized workspace-relative paths with an explicit file or directory type, rather than identifying directories by a trailing slash. 7. Fail the repair cycle when an invalid path is detected and record the rejected path without attempting a partial repair. 8. Run the service under a least-privileged account with write permission limited to the intended workspace. 9. Add tests covering `../`, nested traversal, absolute paths, symbolic links, mixed path separators, and backup-root escapes. ]]>