T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/imap.js:16
- Finding
- Attachment Download Can Overwrite Files and Follow Symbolic Links<![CDATA[ ## Vulnerability Details **File Location**: `scripts/imap.js`, lines 16–35 and 322–323 **Vulnerability Type**: Symlink-following path validation and unrestricted file overwrite **Risk Level**: Medium ### Vulnerable Code ```javascript function validateWritePath(dirPath) { const allowedDirsStr = process.env.ALLOWED_WRITE_DIRS; if (!allowedDirsStr) { throw new Error('ALLOWED_WRITE_DIRS not set in .env. Attachment download is disabled.'); } const resolved = path.resolve(dirPath.replace(/^~/, os.homedir())); const allowedDirs = allowedDirsStr.split(',').map(d => path.resolve(d.trim().replace(/^~/, os.homedir())) ); const allowed = allowedDirs.some(dir => resolved === dir || resolved.startsWith(dir + path.sep) ); if (!allowed) { throw new Error(`Access denied: '${dirPath}' is outside allowed write directories`); } return resolved; } ``` ```javascript for (const attachment of parsed.attachments) { // If specificFilename is provided, only download matching attachment if (specificFilename && attachment.filename !== specificFilename) { continue; } if (attachment.content) { const filePath = path.join(resolvedDir, sanitizeFilename(attachment.filename)); fs.writeFileSync(filePath, attachment.content); downloaded.push({ filename: attachment.filename, path: filePath, size: attachment.size, }); } } ``` ### Technical Analysis The output-directory validation uses `path.resolve()`, which only performs lexical path normalization. It does not resolve symbolic links or verify that the actual filesystem destination remains beneath an authorized directory. Although `sanitizeFilename()` removes directory traversal components by retaining the basename, the final destination is passed directly to `fs.writeFileSync()`. This operation: - Follows an existing symbolic link at the destination. - Silently truncates and overwrites an existing file. - Does not use exclusive creation. - Does ...[truncated 2762 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Canonicalize allowed directories and the destination parent** - Resolve configured directories with `fs.realpathSync()`. - After creating the output directory, resolve it again and verify that its canonical path is equal to or beneath a canonical allowed directory. - Reject output directories containing unexpected symbolic-link components. 2. **Reject symbolic-link destinations** - Call `fs.lstatSync()` when a destination already exists. - Refuse to write if the destination is a symbolic link. - Where supported, open files with `O_NOFOLLOW`. 3. **Prevent silent replacement** - Create attachment files with exclusive mode: ```javascript fs.writeFileSync(filePath, attachment.content, { flag: 'wx', mode: 0o600, }); ``` - If a filename already exists, fail safely or generate a collision-resistant filename. 4. **Use a dedicated download directory** - Do not recommend the Agent workspace as the default writable directory. - Create a private attachment directory that contains no executable code, configuration, instructions, or persistent Agent state. - Apply restrictive directory permissions. 5. **Validate the final destination immediately before writing** - Recheck containment after resolving the parent directory. - Keep validation and file creation in one operation where possible to reduce time-of-check/time-of-use race conditions. 6. **Apply file-size and count limits** - Limit attachment size and the number of attachments written per message to reduce resource-exhaustion risk. ]]>