Skip to content

[Process] On Windows, don't rely on the OS to find executables #58710

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

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 7.2
Bug fix? no
New feature? no
Deprecations? no
Issues -
License MIT

Porting part of composer/composer#12180 here:

On Windows, when searching for an executable, the OS always looks at the current directory before using the PATH variable. This makes it easier than desired to hijack executables. Unix-like OSes don't have this issue.

This PR proposes to rely on ExecutableFinder instead.

@Seldaek
Copy link
Member

Seldaek commented Oct 30, 2024

Also how about patching this on 5.4+ as bugfix?

@nicolas-grekas
Copy link
Member Author

Also how about patching this on 5.4+ as bugfix?

Would work for me, this is welcomed hardening I think for 5.4 also. Any other opinion?

@xabbuh
Copy link
Member

xabbuh commented Oct 30, 2024

Not sure if shipping this with a patch release is a good idea. This would break application that rely on the current behaviour.

@nicolas-grekas nicolas-grekas force-pushed the process-safer branch 4 times, most recently from 3732d2b to 62d7067 Compare October 30, 2024 21:30
@nicolas-grekas
Copy link
Member Author

PR ready, with tests.

@nicolas-grekas nicolas-grekas merged commit a86878f into symfony:7.2 Nov 4, 2024
8 of 10 checks passed
@nicolas-grekas nicolas-grekas deleted the process-safer branch November 4, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants