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

"Git Bash" Windows Terminal fragment profile uses unquoted commandline path #5100

Closed
DHowett opened this issue Aug 9, 2024 · 2 comments · Fixed by git-for-windows/build-extra#573
Milestone

Comments

@DHowett
Copy link

DHowett commented Aug 9, 2024

When the installer generates the git-bash Windows Terminal profile, it does so by constructing a commandline based on the install location:

https://github.com/git-for-windows/build-extra/blob/0cacd22fe04363b5350ea70fb3c884778dfde48e/installer/install.iss#L2854-L2865

For most installs, this will result in a commandline of C:/Program Files/git/bin/bash.exe -i -l

Unquoted commandline strings containing spaces are parsed incrementally, and may lead to unintended execution.

Quotes would tell Windows which part of the text represented the executable's path and which parts were command line arguments, if any. In the absence of quotes, Windows assumes that space characters are delimiters.
It rather involved being on the other side of this airtight hatchway: Unquoted service paths (2014)

first, it parses out C:\Program and looks for C:\Program.com, C:\Program.exe, C:\Program.Bat, etc. When it finds none of those, it assumes that the first space is not in fact a delimiter, treats the characters up to the next space character as part of the file path
ibid.

dscho added a commit to dscho/build-extra that referenced this issue Aug 12, 2024
Without quoting the path, the door is open for executing unintended
executables.

This fixes git-for-windows/git#5100.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member

dscho commented Aug 12, 2024

@DHowett I opened git-for-windows/build-extra#573. Would you mind verifying that this works for you?

@DHowett
Copy link
Author

DHowett commented Aug 12, 2024

@dscho Yep, this works perfectly. Thank you for the quick patch!

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 a pull request may close this issue.

2 participants