Skip to content

xftp: prevent sender from writing files outside the destination folder - #1850

Merged
epoberezkin merged 1 commit into
masterfrom
f/sanitize-xftp-file-name
Aug 12, 2026
Merged

xftp: prevent sender from writing files outside the destination folder#1850
epoberezkin merged 1 commit into
masterfrom
f/sanitize-xftp-file-name

Conversation

@spaced4ndy

Copy link
Copy Markdown
Collaborator

No description provided.

@spaced4ndy
spaced4ndy marked this pull request as ready for review August 12, 2026 12:37

@simplex-chat-agent simplex-chat-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, minimal path-traversal fix for XFTP file reception.

A malicious sender controls the received file name via the file description; the CLI recv command used it verbatim to name the output (getFilePathuniqueCombine), so a name like ../../escaped.txt could write outside the destination folder.

The fix sanitizes at the single choke point where names become paths:
safeFileNameStr = notDots . makeValid . takeFileName. takeFileName drops every directory component (using the recipient's platform separators), makeValid maps """_", and notDots maps ./.."_". Those three ("", ., ..) are exactly the traversal-relevant residues left after takeFileName, so the combined path can no longer escape filePath.

Placing the sanitization inside uniqueCombine rather than at the parse boundary is a reasonable choice: it makes every name→path construction safe by construction, and the other callers all pass trusted bare names (show chunkNo, "redirect.yaml", timestamps), so their behavior is unchanged. The only caller that bypasses uniqueCombine (getFilePath's pure path) uses the recipient's own explicitly chosen path, not sender input.

Tests cover empty, ., .., ..., relative and absolute traversal, trailing slash, and ordinary names, plus an end-to-end uniqueCombine "../../escaped.txt" assertion.

No correctness, concurrency, or security gaps found. Approving.

@epoberezkin
epoberezkin merged commit c377f2c into master Aug 12, 2026
3 of 6 checks passed
@epoberezkin
epoberezkin deleted the f/sanitize-xftp-file-name branch August 12, 2026 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants