Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for Windows: do not run binaries from CWD #1724

Merged
merged 2 commits into from Jul 12, 2021

Conversation

sharkdp
Copy link
Owner

@sharkdp sharkdp commented Jul 12, 2021

This fixes a bug on Windows where Command::new would also run
executables from the current working directory, possibly resulting in
accidental runs of programs called less.

closes #1472

This fixes a bug on Windows where `Command::new` would also run
executables from the current working directory, possibly resulting in
accidental runs of programs called `less`.
@sharkdp sharkdp merged commit 3fa09db into master Jul 12, 2021
@sharkdp sharkdp deleted the fix-relative-path-execution branch July 12, 2021 21:17
@sharkdp
Copy link
Owner Author

sharkdp commented Jul 13, 2021

Thanks a lot to @BurntSushi for providing this fix as a re-usable function in grep-cli (BurntSushi/ripgrep@229d1a8)!

This fix has been released in bat v0.18.2

rhysd added a commit to rhysd/actionlint that referenced this pull request Jul 14, 2021
by using execabs.LookPath instead of exec.LookPath

context: sharkdp/bat#1724
@Enselic
Copy link
Collaborator

Enselic commented Oct 9, 2021

For future reference, here is the PR that will fix this in the Rust std lib: rust-lang/rust#87704

@ChrisDenton
Copy link

I would however note that the std has to be cautious about (potentially) breaking compatibility with prior versions of Rust. This means that even if that PR gets merged, it will still do more than just search PATH. I would love to further simplify it in the future but in the meantime it may still be worth it for applications to use a third party resolver (such as grep_cli).

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.

[Security] Vulnerability report
3 participants