T09 · Insecure Skill Coding Practices
Warning
- Location
- index.js:429
- Finding
- Unrestricted Local File Read Through User-Controlled Quiz Log Path## Vulnerability Details **File Location**: `index.js:429-445`; file content is read at `lib/parser.js:9-10` **Vulnerability Type**: Path traversal and unrestricted local file access **Risk Level**: Medium ### Vulnerable Code ```javascript // index.js:429-445 async hsk_parse_quiz_log({ filePath }) { try { const absolutePath = path.isAbsolute(filePath) ? filePath : path.join(__dirname, '..', '..', filePath); if (!fs.existsSync(absolutePath)) { return { success: false, error: `File not found: ${absolutePath}` }; } const items = parseQuizLog(absolutePath); ``` ```javascript // lib/parser.js:9-10 function parseQuizLog(filePath) { const content = fs.readFileSync(filePath, 'utf8'); ``` ### Technical Analysis The `filePath` argument is externally controlled. If it is absolute, the implementation accepts it without restriction. If it is relative, it is joined to a base directory without subsequently canonicalizing the result and confirming that it remains inside an approved quiz-log directory. Consequently, absolute paths such as `/etc/hosts` and relative traversal paths containing `../` can reach files outside the directory required by the skill's declared quiz-log parsing functionality. `fs.existsSync()` only checks existence and does not establish authorization. The resolved path is passed directly to `fs.readFileSync()`. The parser does not return arbitrary raw file content verbatim. Disclosure through a successful response is constrained to file lines matching its quiz-log patterns, including bold vocabulary entries and recognized vocabulary-list formats. Nevertheless, any process-readable file is opened and parsed, matching content may be returned in structured results, and error responses can expose path or parser details. This violates least-privilege filesystem access and creates a local file disclosure primitive. ### Att ...[truncated 1701 chars]
- Remediation
- ## Remediation Suggestions 1. Define a single approved directory for quiz-performance logs, such as the OpenClaw `memory` directory or a dedicated `memory/quiz-logs` subdirectory. 2. Reject absolute paths supplied by callers. 3. Resolve both the approved directory and requested target with `path.resolve()`, then verify that the target remains inside the approved directory using `path.relative()` or a separator-safe prefix check. 4. Restrict accepted files to the expected `.md` extension and, where practical, the documented quiz-performance filename pattern. 5. Use `fs.statSync()` or its asynchronous equivalent to require a regular file. Consider rejecting symbolic links or validating the real path with `fs.realpath()` to prevent symlink escapes. 6. Do not return absolute paths, raw stack traces, or internal parser errors to callers. Log detailed diagnostics only to a protected server-side log. 7. Prefer asynchronous, size-limited reads to reduce denial-of-service risk from very large files. Example hardening pattern: ```javascript const allowedDir = path.resolve(__dirname, '..', '..', 'memory'); if (path.isAbsolute(filePath)) { return { success: false, error: 'Absolute paths are not allowed' }; } if (!filePath.endsWith('.md') || !path.basename(filePath).includes('quiz-performance')) { return { success: false, error: 'Invalid quiz log filename' }; } const candidate = path.resolve(allowedDir, filePath); const relative = path.relative(allowedDir, candidate); if (relative.startsWith('..') || path.isAbsolute(relative)) { return { success: false, error: 'Path is outside the allowed directory' }; } const realAllowedDir = fs.realpathSync(allowedDir); const realCandidate = fs.realpathSync(candidate); const realRelative = path.relative(realAllowedDir, realCandidate); if (realRelative.startsWith('..') || path.isAbsolute(realRelative)) { return { success: false, error: 'Path is outside the allowed directory' ...[truncated 352 chars]