From e3f057275aa3d0ac42e93e2adedcdfedeb72046b Mon Sep 17 00:00:00 2001 From: Chris Darroch Date: Tue, 9 Jan 2024 15:08:42 -0800 Subject: [PATCH] t/t-path.sh: avoid test setup failure on Windows In commit 10c4ffc6b888eee8f2134a7009a0db1bc393e17b we added the "does not look in current directory for git with credential helper" test in order to validate changes made to remediate the issues reported in CVE-2021-21237, and revised it as part of our response to CVE-2022-24826 in commit 11092ef2b17eaa67f3363edc68b59331b979f7ee. This test checks that Git LFS will not run a potentially malicious Git executable found in the working tree of a repository. As noted in commit 7e8d3ba84ad8ee5314cb74a32b80622b919163fe, in order to avoid invoking this executable while setting up the test conditions we generally remove it from the working directory as soon as possible, but we have to at least leave it there while running "git add git.exe" in order to add it to the current Git index. Therefore we explicitly set the PATH environment variable before running this command to include a minimal set of necessary directories, such as the ones for the real Git and Git LFS executables. As of Go 1.19 we now also need to specify the GODEBUG environment variable with a value of "execerrdot=0" in order to avoid occasional failures of the "git add git.exe" command on Windows. These failures occur due to a specific set of conditions. First, if the last-modified time of the .git/index file is within a second of that of z.dat, the "git add" command will refresh the Git index (assuming Git was compiled with USE_NSEC=0, as appears to be the case for Git for Windows), and Git LFS will be invoked to "clean" the z.dat file again. For more details, see the "racy Git" documentation and the is_racy_stat() function in Git: https://git-scm.com/docs/racy-git/en https://github.com/git/git/blob/a54a84b333adbecf7bc4483c0e36ed5878cac17b/read-cache.c#L356 If Git LFS is rerun by the "git add git.exe" command, then when Git LFS executes it looks for Git, and until we revise Git LFS to rely on Go's os/exec package to not execute programs found in the current working directory the os/exec package will detect our malicious Git program in the current working directory and report an error. This occurs when Git LFS first initializes a new exec.Cmd structure, even though Git LFS would then locate the true Git executable from our custom PATH and reset the Path member of the Cmd structure before trying to execute the program. See, for reference, the Go blog article on the changes in Go 1.19 and the Windows version of the lookPath() function in the os/exec package: https://go.dev/blog/path-security https://github.com/golang/go/blob/dcbe77246922fe7ef41f07df228f47a37803f360/src/os/exec/lp_windows.go#L163 https://github.com/golang/go/blob/dcbe77246922fe7ef41f07df228f47a37803f360/src/os/exec/exec.go#L24-L90 Since we explicitly test Git LFS's avoidance of programs in the current working directory using the "git-lfs pull" command later in our test, we just want the "git add git.exe" command to succeed, so for the time being we disable Go's new security checks for this command only using the execerrdot=0 setting in GODEBUG. We will revisit this when we address the larger issue of re-adopting Go's own logic for locating executable programs safely and without security issues. --- t/t-path.sh | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/t/t-path.sh b/t/t-path.sh index 0c2be0ee..cb49d52e 100755 --- a/t/t-path.sh +++ b/t/t-path.sh @@ -52,8 +52,35 @@ begin_test "does not look in current directory for git with credential helper" # below when we are populating LFS objects into a clone of this repo # (which contains the malicious Git), so for now we remove the malicious # Git as soon as possible. + # + # As of Go 1.19 we also need to specify the GODEBUG environment variable + # with a value of "execerrdot=0" in order to avoid occasional failures + # our "git add" command below. These failures occur due to a specific + # set of conditions. First, if the last-modified time of the .git/index + # file is within a second of that of z.dat, the "git add" command will + # refresh the Git index (assuming Git was compiled with USE_NSEC=0, as + # appears to be the case for Git for Windows), and Git LFS will be invoked + # to "clean" the z.dat file again. + # + # If that occurs, then when Git LFS runs it looks for Git, and until we + # revise Git LFS to rely on Go's os/exec package to not execute programs + # found in the current working directory (as described in + # https://go.dev/blog/path-security), the os/exec package will detect our + # malicious Git program in the current working directory and report an + # error. This occurs when Git LFS first initializes a new exec.Cmd + # structure, even though Git LFS would then locate the true Git executable + # from our custom PATH and reset the Path member of the Cmd structure + # before trying to execute the program. + # + # Since we explicitly test Git LFS's avoidance of programs in the current + # working directory using the "git-lfs pull" command further below, here + # we just want "git add" to succeed, and so for the time being we disable + # Go's new security checks for this command only. We will revisit this + # when we address the larger issue of re-adopting Go's own logic for + # locating executable programs. cp "$BINPATH/lfstest-badpathcheck$X" "git$X" - PATH="$BINPATH:$GITPATH:$SHELLPATH" "$GITPATH/git$X" add "git$X" + GODEBUG=execerrdot=0 \ + PATH="$BINPATH:$GITPATH:$SHELLPATH" "$GITPATH/git$X" add "git$X" rm "git$X" git commit -m "Add files"