Skip to content

Security follow-ups for paste-image endpoint (#84) #87

@Ark0N

Description

@Ark0N

The paste-image endpoint introduced in #84 ships with three known security gaps that didn't block merge but should be addressed. Listed in priority order.

1. CSRF on POST /api/sessions/:id/paste-image (highest)

The session cookie is SameSite=lax (src/web/middleware/auth.ts:135), and multipart/form-data is a CORS-simple request that fires without preflight. No Origin/Referer check or CSRF token on this state-changing endpoint, so a cross-origin <form> POST from a malicious page can plant a ≤10MB file into the victim's ${workingDir}/.claude-images/ as long as their browser holds a valid session cookie. The path is server-generated so traversal is blocked, and the file is not automatically piped to Claude — but it remains a "plant arbitrary file in a user-trusted dir" primitive that compounds with any future feature that reads the dir.

Fix: require an Origin header match against an allowlist on state-changing routes, or require an X-Codeman-CSRF custom header that <form> cannot send.

2. No magic-byte validation

Extension and Content-Type are both attacker-controlled (src/web/routes/session-routes.ts:1508-1524). A PNG-named file with arbitrary bytes lands on disk. Defence-in-depth issue today; pairs poorly with the CSRF gap above.

Fix: sniff the first 16 bytes against a known-header table (PNG 89 50 4E 47, JPEG FF D8 FF, GIF GIF8[79], WebP RIFF…WEBP, BMP BM) and reject mismatches with 415.

3. No symlink check on imageDir

existsSync(imageDir) + mkdirSync(imageDir, {recursive:true}) (src/web/routes/session-routes.ts:~1540) both follow symlinks. If a process inside workingDir (the agent itself, a dep in node_modules/.bin, etc.) plants .claude-images → ~/.ssh/, subsequent paste-images write into the symlink target.

Fix: lstat(imageDir) and reject if it's a symlink, or fs.realpath and verify the resolved path is still under session.workingDir. Mirror the symlink-aware approach already used in validateSessionFilePath in src/web/route-helpers.ts.


Caught in the review batch that landed alongside #84 (commit 94bcf52). The pre-merge SVG-removal patch (a60a9a7) closed the stored-XSS path; these are the remaining gaps from that review.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions