Skip to content

fix: sanitize escape sequence injection - #1976

Merged
tmccombs merged 9 commits into
sharkdp:masterfrom
curious-rabbit:master
Jul 1, 2026
Merged

fix: sanitize escape sequence injection#1976
tmccombs merged 9 commits into
sharkdp:masterfrom
curious-rabbit:master

Conversation

@curious-rabbit

@curious-rabbit curious-rabbit commented Apr 18, 2026

Copy link
Copy Markdown
  • Filenames can contain terminal escape codes (like ESC). fd prints them raw to your terminal, so a file named innocent\x1b]52;c;...\x1b\.txt silently rewrites your clipboard. Fix: strip control bytes before writing to a TTY.

  • fd -x rm {/} in a dir with a file named -rf runs rm -rf. Fix: prepend ./ when a path placeholder produces a leading dash.

  • fd -x '{}' lets you use {} as the command to run, not just an argument to one. It's inconsistent with fd -X which rejects this. The patch rejects placeholder in args[0] for both modes.

Also added tests to ensure changes don't break this in the future

Comment thread src/fmt/mod.rs Outdated
Comment thread src/main.rs Outdated
Comment thread src/sanitize.rs Outdated
Comment thread src/fmt/mod.rs Outdated
Comment thread src/fmt/mod.rs Outdated
@curious-rabbit
curious-rabbit requested a review from tmccombs April 20, 2026 12:28
Comment thread src/dir_entry.rs Outdated
Comment thread src/sanitize.rs Outdated
Comment thread src/exec/mod.rs Outdated
@curious-rabbit
curious-rabbit requested a review from tavianator May 3, 2026 22:13

@tmccombs tmccombs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would you mind adding an entry to the changelog?

Comment thread src/dir_entry.rs Outdated
Comment thread src/error.rs Outdated
Comment thread src/sanitize.rs
Comment thread src/sanitize.rs
curious-rabbit added 2 commits May 5, 2026 21:27
curious-rabbit added 2 commits May 17, 2026 20:35
@curious-rabbit

Copy link
Copy Markdown
Author

Is there anything left to do for the merge?

@tmccombs
tmccombs merged commit b126064 into sharkdp:master Jul 1, 2026
19 checks passed
@decathorpe

Copy link
Copy Markdown

It looks like this PR removed jemalloc from the default features. I don't see any references to this happening so it appears to have been accidental? Note that this has leaked into the 10.5.0 release too.

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.

4 participants