-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Upgrading x/crypto to v0.35.0 to solve CVE-2025-22869 #5997
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
Conversation
|
Hey, thanks for the PR and for jumping on this issue. Dependabot should offer PRs for these kinds of issues too, but I think you've pre-empted it! Fortunately, as with #5941 this is another case where the vulnerability doesn't affect the Git LFS client. This new vulnerability, addressed by the update to the The Git LFS client never acts as an SSH server, so there is no immediate necessity for us to upgrade the module and release a new version of Git LFS. While some of the Go packages we use indirectly import packages from the Nevertheless, we might as well upgrade the module, so that when we do release v3.7.0 of the Git LFS client, security scanners will not generate false positive reports about the version of the For reference, the details of the vulnerability and its mitigation are described in GO-2025-3487 and also in the release announcements for version 0.35.0 of the As for when we'll merge this PR, I think we'll probably wait a few weeks, since it's not urgent and because we'll need to upgrade our CI and Docker builds to Go 1.24 first, and that in turn is likely to expose some other issues which we should address first (see #5968). I have a set of changes in progress to resolve those, but until the other member of our core team is available to review them, they'll have to wait. |
When we build Debian and RPM Linux packages, we define the minimum versions of Git and Go required by the Git LFS client. However, the minimum versions we specify are at present somewhat out of date. Specifically, both the "control" file for our Debian packages and the SPEC file for our RPM packages state that we require at least Git version 1.8.2, and the former also specifies that we require at least Go version 1.12.0. In practice, though, since we introduced the "git lfs migrate" command in PR git-lfs#2353, Git v2.0.0 has been the earliest version of Git we support, as per commit 5aea841 of that PR. We have also required at least Go v1.23.0 to build the Git LFS client since commit 70e23fa of PR git-lfs#5997, when we updated the minimum version of the x/crypto Go module specified in our "go.mod" file and the "go mod tidy" command then also updated the minimum required version of Go to 1.23.0. Because we anticipate making a v3.7.0 release of the Git LFS client in the near future, we now update the "control" file for our Debian packages and the SPEC file for our RPM packages to indicate that the Git LFS client requires at least Git v2.0.0 and Go v1.23.0.
In commit 8607cd4 of PR git-lfs#3780 we revised the SmudgeToFile() method of the GitFilter structure in our "lfs" package to construct an absolute path from its "filename" parameter and use this path when creating or truncating the file it creates and populates with the content of a Git LFS object. We made this change with the goal of resolving the problem reported in git-lfs#3772 regarding Windows systems where the maximum length of a file path is often restricted to the MAX_PATH limit of 260 characters. Beginning with certain versions of Windows 10, system administrators may now change the maximum file path length to a significantly higher value, but this change only takes effect for applications which opt into the new setting. For reference, see: https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation#enable-long-paths-in-windows-10-version-1607-and-later Since the inclusion of Change List (CL) 32451 in Go version 1.8, many of the functions and methods in the "os" package of the Go standard library have supported the use of absolute paths longer than the MAX_PATH limit on Windows by internally converting such paths into "root local" DOS device paths before passing them to Windows API system calls. These types of DOS device paths begin with a "\\?\" prefix, and are the only file path format which Windows guarantees will always be accepted even if they exceed the MAX_PATH length limit: https://go.dev/cl/32451 https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#win32-file-namespaces At the time of our change in PR git-lfs#3780, we were using Go v1.11.1 to build the Git LFS client. By changing our SmudgeToFile() method to construct an absolute path with the Abs() function from the "path/filepath" package of the Go standard library, we were able to take advantage of the fact that when we passed the resultant path to the Create() function of the "os" package, it would translate that path into one beginning with "\\?\" and thus the final file path could exceed the MAX_PATH length limit. Note that the Abs() function itself does not convert paths into the "root local" DOS device format, because it is implemented as a wrapper around a system call to the GetFullPathName() Windows API function. That function merges the full path to the current working directory, which may be a conventional DOS path beginning with a disk designator like "C:\", to a relative path, and returns the result without converting it into a DOS device path: https://github.com/golang/go/blob/go1.25.3/src/path/filepath/path.go#L162 https://github.com/golang/go/blob/go1.25.3/src/path/filepath/path_windows.go#L62 https://github.com/golang/go/blob/go1.25.3/src/syscall/exec_windows.go#L162 https://learn.microsoft.com/en-ca/windows/win32/api/fileapi/nf-fileapi-getfullpathnamea However, because our SmudgeToFile() method passed the absolute path returned by the Abs() function to the Create() function of the "os" package, our change in PR git-lfs#3780 was effective in allowing file paths longer than the MAX_PATH limit to be created on Windows, at least in some cases. The Create() function, like others in the "os" package, invokes a chain of internal functions which ultimately pass the input path to the fixLongPath() function, and that function converts any absolute path into one beginning with "\\?\": https://github.com/golang/go/blob/go1.11.1/src/os/file.go#L274 https://github.com/golang/go/blob/go1.11.1/src/os/file.go#L284 https://github.com/golang/go/blob/go1.11.1/src/os/file_windows.go#L158 https://github.com/golang/go/blob/go1.11.1/src/os/file_windows.go#L97 https://github.com/golang/go/blob/go1.11.1/src/os/path_windows.go#L131-L209 While the changes in PR git-lfs#3780 resolved some instances where a long relative path caused errors on Windows when running "git lfs checkout" or "git lfs pull" commands, they were likely insufficient to resolve all such cases. Prior to commit 0cffe93 of our "main" development branch and commit 2902e9d of our "release-3.7" branch, the SmudgeToFile() method continued to pass relative paths to the MkdirAll() function in our "tools" package, which in turn calls the MkdirAll() function in the "os" package of the standard Go library. If the relative path provided to the SmudgeToFile() method was longer than the MAX_PATH limit even with the trailing file name removed, and some of the directories named in that path did not exist, then the MkdirAll() method would have failed, at least with versions of Go prior to v1.23.0. Fortunately, ever since commit 70e23fa of PR git-lfs#5997, when we upgraded the version of Go we use to build the Git LFS client to Go v1.23.0, this problem and any similar issues should now be effectively eliminated. Go v1.23.0 includes the revisions to its internal fixLongPath() function from CLs (Change Lists) 570995 and 574695, particularly commits golang/go@2e8d84f and golang/go@7c89ad6, which ensure that the function now correctly handles both UNC paths and, most importantly for our purposes here, relative paths: https://go.dev/cl/570995 https://go.dev/cl/574695 https://github.com/golang/go/blob/go1.23.0/src/os/path_windows.go#L41-L152 These improvements in the Go standard library therefore apply to our use of the Remove() and OpenFile() functions from the "os" package in our SmudgeToFile() method. Further, they also apply to our use of the "os" package's Mkdir() function to create any missing directories between the root of the current Git working tree and the file path provided to the SmudgeToFile() method. As noted above, we no longer call the MkdirAll() function since commits 0cffe93 and 2902e9d. Instead, the walk() method of the new DirWalker structure in our "tools" package invokes the Mkdir() function in the same package for each missing directory, and this helper function then calls the "os" package's Mkdir() function. As a consequence, we can simply remove the call to the Abs() function from our SmudgeToFile() method and use the relative path provided in its "filename" parameter without any adjustment.
In commit 8607cd4 of PR git-lfs#3780 we revised the SmudgeToFile() method of the GitFilter structure in our "lfs" package to construct an absolute path from its "filename" parameter and use this path when creating or truncating the file it creates and populates with the content of a Git LFS object. We made this change with the goal of resolving the problem reported in git-lfs#3772 regarding Windows systems where the maximum length of a file path is often restricted to the MAX_PATH limit of 260 characters. Beginning with certain versions of Windows 10, system administrators may now change the maximum file path length to a significantly higher value, but this change only takes effect for applications which opt into the new setting. For reference, see: https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation#enable-long-paths-in-windows-10-version-1607-and-later Since the inclusion of Change List (CL) 32451 in Go version 1.8, many of the functions and methods in the "os" package of the Go standard library have supported the use of absolute paths longer than the MAX_PATH limit on Windows by internally converting such paths into "root local" DOS device paths before passing them to Windows API system calls. These types of DOS device paths begin with a "\\?\" prefix, and are the only file path format which Windows guarantees will always be accepted even if they exceed the MAX_PATH length limit: https://go.dev/cl/32451 https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#win32-file-namespaces At the time of our change in PR git-lfs#3780, we were using Go v1.11.1 to build the Git LFS client. By changing our SmudgeToFile() method to construct an absolute path with the Abs() function from the "path/filepath" package of the Go standard library, we were able to take advantage of the fact that when we passed the resultant path to the Create() function of the "os" package, it would translate that path into one beginning with "\\?\" and thus the final file path could exceed the MAX_PATH length limit. Note that the Abs() function itself does not convert paths into the "root local" DOS device format, because it is implemented as a wrapper around a system call to the GetFullPathName() Windows API function. That function merges the full path to the current working directory, which may be a conventional DOS path beginning with a disk designator like "C:\", to a relative path, and returns the result without converting it into a DOS device path: https://github.com/golang/go/blob/go1.25.3/src/path/filepath/path.go#L162 https://github.com/golang/go/blob/go1.25.3/src/path/filepath/path_windows.go#L62 https://github.com/golang/go/blob/go1.25.3/src/syscall/exec_windows.go#L162 https://learn.microsoft.com/en-ca/windows/win32/api/fileapi/nf-fileapi-getfullpathnamea However, because our SmudgeToFile() method passed the absolute path returned by the Abs() function to the Create() function of the "os" package, our change in PR git-lfs#3780 was effective in allowing file paths longer than the MAX_PATH limit to be created on Windows, at least in some cases. The Create() function, like others in the "os" package, invokes a chain of internal functions which ultimately pass the input path to the fixLongPath() function, and that function converts any absolute path into one beginning with "\\?\": https://github.com/golang/go/blob/go1.11.1/src/os/file.go#L274 https://github.com/golang/go/blob/go1.11.1/src/os/file.go#L284 https://github.com/golang/go/blob/go1.11.1/src/os/file_windows.go#L158 https://github.com/golang/go/blob/go1.11.1/src/os/file_windows.go#L97 https://github.com/golang/go/blob/go1.11.1/src/os/path_windows.go#L131-L209 While the changes in PR git-lfs#3780 resolved some instances where a long relative path caused errors on Windows when running "git lfs checkout" or "git lfs pull" commands, they were likely insufficient to resolve all such cases. Prior to commit 0cffe93 of our "main" development branch and commit 2902e9d of our "release-3.7" branch, the SmudgeToFile() method continued to pass relative paths to the MkdirAll() function in our "tools" package, which in turn calls the MkdirAll() function in the "os" package of the standard Go library. If the relative path provided to the SmudgeToFile() method was longer than the MAX_PATH limit even with the trailing file name removed, and some of the directories named in that path did not exist, then the MkdirAll() method would have failed, at least with versions of Go prior to v1.23.0. Fortunately, ever since commit 70e23fa of PR git-lfs#5997, when we upgraded the version of Go we use to build the Git LFS client to Go v1.23.0, this problem and any similar issues should now be effectively eliminated. Go v1.23.0 includes the revisions to its internal fixLongPath() function from CLs (Change Lists) 570995 and 574695, particularly commits golang/go@2e8d84f and golang/go@7c89ad6, which ensure that the function now correctly handles both UNC paths and, most importantly for our purposes here, relative paths: https://go.dev/cl/570995 https://go.dev/cl/574695 https://github.com/golang/go/blob/go1.23.0/src/os/path_windows.go#L41-L152 These improvements in the Go standard library therefore apply to our use of the Remove() and OpenFile() functions from the "os" package in our SmudgeToFile() method. Further, they also apply to our use of the "os" package's Mkdir() function to create any missing directories between the root of the current Git working tree and the file path provided to the SmudgeToFile() method. As noted above, we no longer call the MkdirAll() function since commits 0cffe93 and 2902e9d. Instead, the walk() method of the new DirWalker structure in our "tools" package invokes the Mkdir() function in the same package for each missing directory, and this helper function then calls the "os" package's Mkdir() function. As a consequence, we can simply remove the call to the Abs() function from our SmudgeToFile() method and use the relative path provided in its "filename" parameter without any adjustment.
No description provided.