T09 · Insecure Skill Coding Practices
Error
- Location
- push.js:81
- Finding
- Unrestricted Recursive Upload and Symbolic-Link Traversal Can Disclose Local Files<![CDATA[ ## Vulnerability Details **File Location**: `push.js:23-28`, `push.js:81-107`, `push.js:158-175`, `push.js:183-190` **Vulnerability Type**: Uncontrolled file collection, symbolic-link traversal, and unintended sensitive-data transmission **Risk Level**: High ### Vulnerable Code ```js // Files/directories excluded from publication const EXCLUDE_PATTERNS = [ '.git', 'node_modules', '.DS_Store', '*.log', ]; ``` ```js async function getSkillFiles(skillPath) { const files = []; async function walk(dir, relativePath = '') { try { const items = await fs.readdir(dir); for (const item of items) { if (shouldExclude(item)) continue; const fullPath = path.join(dir, item); const relPath = relativePath ? `${relativePath}/${item}` : item; const stat = await fs.stat(fullPath); if (stat.isDirectory()) { await walk(fullPath, relPath); } else { const content = await fs.readFile(fullPath, 'utf8'); files.push({ path: relPath, content, fullPath }); } } } catch (e) { console.error(`Error reading ${dir}: ${e.message}`); } } await walk(skillPath); return files; } ``` ```js const formData = new FormData(); formData.append('payload', JSON.stringify(payload)); // Add files for (const file of files) { const blob = new Blob([file.content], { type: 'text/plain' }); formData.append('files', blob, file.path); } // Call API const url = `${API_BASE}/skills`; try { const response = await fetch(url, { method: 'POST', headers: { 'Authorization': `Bearer ${token}`, }, body: formData, }); ``` ### Technical Analysis The publisher recursively reads every file beneath the user-supplied Skill directory, except for a small denylist containing `.git`, `node_modules`, `.DS_Store`, and log files. It does not exclude common sensitive resources such as: - ...[truncated 3074 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Reject symbolic links explicitly** - Replace `fs.stat()` with `fs.lstat()`. - Refuse to publish any entry for which `stat.isSymbolicLink()` is true. - Do not recursively traverse directory links. 2. **Enforce the canonical Skill-root boundary** - Resolve the Skill root once with `fs.realpath()`. - Resolve every candidate file with `fs.realpath()`. - Confirm that the candidate remains beneath the canonical root using a boundary-aware relative-path check. - Reject paths that escape the root. 3. **Adopt an allowlist or publication manifest** - Prefer an explicit manifest listing the files to publish. - Alternatively, allow only expected Skill file types and directories. - Do not rely exclusively on a short denylist. 4. **Exclude sensitive file patterns** - At minimum, reject `.env*`, private keys, credentials, tokens, certificates, backup files, and common cloud-provider credential files. - Use correctly anchored glob matching rather than dynamically constructing a partially anchored regular expression. 5. **Require informed user confirmation** - Print the complete normalized file list before transmission. - Clearly display total file count and upload size. - Require confirmation unless an explicit noninteractive option is supplied. 6. **Apply resource limits** - Limit maximum recursion depth, individual file size, total file count, and aggregate upload size. - Track visited canonical directories to prevent cycles. 7. **Fail closed** - Abort publication if traversal encounters a symbolic link, inaccessible entry, path escape, or validation error. - Do not silently continue with an uncertain upload set. ]]>