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

Revisit use of Go's standard library to locate Git executable #5612

Open
chrisd8088 opened this issue Jan 10, 2024 · 0 comments
Open

Revisit use of Go's standard library to locate Git executable #5612

chrisd8088 opened this issue Jan 10, 2024 · 0 comments

Comments

@chrisd8088
Copy link
Member

chrisd8088 commented Jan 10, 2024

In our remediation of the security issues CVE-2020-27955, CVE-2021-21237, and CVE-2022-24826, we addressed a series of problems stemming from the fact that on Windows the Go language's os/exec package would search for executable programs in the current working directory before searching other directories specified by the PATH environment variable, and so Git LFS could be convinced to run a malicious executable when it attempted to run Git.

As of Go 1.19, the os/exec package in the Go standard library will no longer execute programs found in the current working directory, and in fact will report a new ErrDot error if it is passed a file name to execute and Go discovers an executable in the current working directory which matches that file name, as described in the code comments. The full explanation for this change is discussed in an accompanying blog article:

https://go.dev/blog/path-security

Our current approach uses a version of the internal lookPath() functions from the os/exec package which we have merged into a single common function and imported into Git LFS, which we call to set the Path member of an exec.Cmd structure after we have created it. This ensures Go will execute the program we have located as opposed to whatever it first locates when the Cmd structure is initialized.

However, as noted in PR #5611, as of Go 1.19 if the executable file name matches a file in the current working directory then an ErrDot error is thrown regardless of whether we later change the Path structure member or not.

Also, the current versions of the lookPath() function for Unix and Windows have diverged from older ones we originally imported into this project and modified. This means we are not benefitting from any of the subsequent enhancements or bug fixes in this code since we "forked" it, not just the introduction of the new ErrDot checks but any others as well.

We should determine if we are comfortable reverting to use of the os/exec package to search for executables, and if so, make that change. We do, however, want to retain all of our existing tests which validate that Git LFS will not run a Git binary or any other executable if it appears in the current working directory.

/cc #5611

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Enhancements
Development

No branches or pull requests

2 participants