From cc77a34f0aea4a1cad14b407faa3aea7d00aaae7 Mon Sep 17 00:00:00 2001 From: Go Compiler Team Date: Fri, 21 Jan 2022 18:45:18 +0000 Subject: [PATCH] CVE-2017-15041 Origin: https://github.com/golang/go/commit/9a97c3bfe41d1ed768ea3bd3d8f0f52b8a51bb62 Origin: https://github.com/golang/go/commit/a4544a0f8af001d1fb6df0e70750f570ec49ccf9 Origin: https://github.com/golang/go/commit/533ee44cd45c064608ee2b833af9e86ef1cb294e Reviewed-by: Sylvain Beucler Last-Update: 2021-03-02 From 9a97c3bfe41d1ed768ea3bd3d8f0f52b8a51bb62 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 13 Oct 2016 13:45:31 -0400 Subject: [PATCH] cmd/go: accept plain file for .vcs (instead of directory) Sometimes .git is a plain file; maybe others will follow. This CL matches CL 21430, made in x/tools/go/vcs. The change in the Swift test case makes the test case pass by changing the test to match current behavior, which I assume is better than the reverse. (The test only runs locally and without -short, so the builders are not seeing this particular failure.) For #10322. Change-Id: Iccd08819a01c5609a2880b9d8a99af936e20faff Reviewed-on: https://go-review.googlesource.com/30948 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor Gbp-Pq: Name CVE-2017-15041.patch --- src/cmd/go/get.go | 5 ++++ src/cmd/go/go_test.go | 28 ++++++++++++++++---- src/cmd/go/vcs.go | 60 ++++++++++++++++++++++++++++++++++++++++-- src/cmd/go/vcs_test.go | 22 ++++++++++++---- 4 files changed, 103 insertions(+), 12 deletions(-) diff --git a/src/cmd/go/get.go b/src/cmd/go/get.go index 19858f7..4f87543 100644 --- a/src/cmd/go/get.go +++ b/src/cmd/go/get.go @@ -401,6 +401,11 @@ func downloadPackage(p *Package) error { p.build.PkgRoot = filepath.Join(list[0], "pkg") } root := filepath.Join(p.build.SrcRoot, filepath.FromSlash(rootPath)) + + if err := checkNestedVCS(vcs, root, p.build.SrcRoot); err != nil { + return err + } + // If we've considered this repository already, don't do it again. if downloadRootCache[root] { return nil diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 66c6413..f11cb67 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -1099,7 +1099,7 @@ func testMove(t *testing.T, vcs, url, base, config string) { tg.runFail("get", "-d", "-u", url) tg.grepStderr("is a custom import path for", "go get -d -u "+url+" failed for wrong reason") tg.runFail("get", "-d", "-f", "-u", url) - tg.grepStderr("validating server certificate|not found", "go get -d -f -u "+url+" failed for wrong reason") + tg.grepStderr("validating server certificate|[nN]ot [fF]ound", "go get -d -f -u "+url+" failed for wrong reason") } func TestInternalPackageErrorsAreHandled(t *testing.T) { @@ -1120,10 +1120,9 @@ func TestMoveGit(t *testing.T) { testMove(t, "git", "rsc.io/pdf", "pdf", "rsc.io/pdf/.git/config") } -// TODO(rsc): Set up a test case on bitbucket for hg. -// func TestMoveHG(t *testing.T) { -// testMove(t, "hg", "rsc.io/x86/x86asm", "x86", "rsc.io/x86/.hg/hgrc") -// } +func TestMoveHG(t *testing.T) { + testMove(t, "hg", "vcs-test.golang.org/go/custom-hg-hello", "custom-hg-hello", "vcs-test.golang.org/go/custom-hg-hello/.hg/hgrc") +} // TODO(rsc): Set up a test case on SourceForge (?) for svn. // func testMoveSVN(t *testing.T) { @@ -1235,6 +1234,25 @@ func TestGetGitDefaultBranch(t *testing.T) { tg.grepStdout(`\* another-branch`, "not on correct default branch") } +func TestAccidentalGitCheckout(t *testing.T) { + testenv.MustHaveExternalNetwork(t) + if _, err := exec.LookPath("git"); err != nil { + t.Skip("skipping because git binary not found") + } + + tg := testgo(t) + defer tg.cleanup() + tg.parallel() + tg.tempDir("src") + tg.setenv("GOPATH", tg.path(".")) + + tg.runFail("get", "-u", "vcs-test.golang.org/go/test1-svn-git") + tg.grepStderr("src[\\\\/]vcs-test.* uses git, but parent .*src[\\\\/]vcs-test.* uses svn", "get did not fail for right reason") + + tg.runFail("get", "-u", "vcs-test.golang.org/go/test2-svn-git/test2main") + tg.grepStderr("src[\\\\/]vcs-test.* uses git, but parent .*src[\\\\/]vcs-test.* uses svn", "get did not fail for right reason") +} + func TestErrorMessageForSyntaxErrorInTestGoFileSaysFAIL(t *testing.T) { tg := testgo(t) defer tg.cleanup() diff --git a/src/cmd/go/vcs.go b/src/cmd/go/vcs.go index 9cd4688..436c0ce 100644 --- a/src/cmd/go/vcs.go +++ b/src/cmd/go/vcs.go @@ -479,11 +479,28 @@ func vcsFromDir(dir, srcRoot string) (vcs *vcsCmd, root string, err error) { return nil, "", fmt.Errorf("directory %q is outside source root %q", dir, srcRoot) } + var vcsRet *vcsCmd + var rootRet string + origDir := dir for len(dir) > len(srcRoot) { for _, vcs := range vcsList { - if fi, err := os.Stat(filepath.Join(dir, "."+vcs.cmd)); err == nil && fi.IsDir() { - return vcs, filepath.ToSlash(dir[len(srcRoot)+1:]), nil + if _, err := os.Stat(filepath.Join(dir, "."+vcs.cmd)); err == nil { + root := filepath.ToSlash(dir[len(srcRoot)+1:]) + // Record first VCS we find, but keep looking, + // to detect mistakes like one kind of VCS inside another. + if vcsRet == nil { + vcsRet = vcs + rootRet = root + continue + } + // Allow .git inside .git, which can arise due to submodules. + if vcsRet == vcs && vcs.cmd == "git" { + continue + } + // Otherwise, we have one VCS inside a different VCS. + return nil, "", fmt.Errorf("directory %q uses %s, but parent %q uses %s", + filepath.Join(srcRoot, rootRet), vcsRet.cmd, filepath.Join(srcRoot, root), vcs.cmd) } } @@ -496,9 +513,48 @@ func vcsFromDir(dir, srcRoot string) (vcs *vcsCmd, root string, err error) { dir = ndir } + if vcsRet != nil { + return vcsRet, rootRet, nil + } + return nil, "", fmt.Errorf("directory %q is not using a known version control system", origDir) } +// checkNestedVCS checks for an incorrectly-nested VCS-inside-VCS +// situation for dir, checking parents up until srcRoot. +func checkNestedVCS(vcs *vcsCmd, dir, srcRoot string) error { + if len(dir) <= len(srcRoot) || dir[len(srcRoot)] != filepath.Separator { + return fmt.Errorf("directory %q is outside source root %q", dir, srcRoot) + } + + otherDir := dir + for len(otherDir) > len(srcRoot) { + for _, otherVCS := range vcsList { + if _, err := os.Stat(filepath.Join(otherDir, "."+otherVCS.cmd)); err == nil { + // Allow expected vcs in original dir. + if otherDir == dir && otherVCS == vcs { + continue + } + // Allow .git inside .git, which can arise due to submodules. + if otherVCS == vcs && vcs.cmd == "git" { + continue + } + // Otherwise, we have one VCS inside a different VCS. + return fmt.Errorf("directory %q uses %s, but parent %q uses %s", dir, vcs.cmd, otherDir, otherVCS.cmd) + } + } + // Move to parent. + newDir := filepath.Dir(otherDir) + if len(newDir) >= len(otherDir) { + // Shouldn't happen, but just in case, stop. + break + } + otherDir = newDir + } + + return nil +} + // repoRoot represents a version control system, a repo, and a root of // where to put it on disk. type repoRoot struct { diff --git a/src/cmd/go/vcs_test.go b/src/cmd/go/vcs_test.go index f7a6a83..0e030df 100644 --- a/src/cmd/go/vcs_test.go +++ b/src/cmd/go/vcs_test.go @@ -102,7 +102,7 @@ func TestRepoRootForImportPath(t *testing.T) { "git.openstack.org/openstack/swift.git", &repoRoot{ vcs: vcsGit, - repo: "https://git.openstack.org/openstack/swift", + repo: "https://git.openstack.org/openstack/swift.git", }, }, { @@ -174,11 +174,23 @@ func TestFromDir(t *testing.T) { } defer os.RemoveAll(tempDir) - for _, vcs := range vcsList { + for j, vcs := range vcsList { dir := filepath.Join(tempDir, "example.com", vcs.name, "."+vcs.cmd) - err := os.MkdirAll(dir, 0755) - if err != nil { - t.Fatal(err) + if j&1 == 0 { + err := os.MkdirAll(dir, 0755) + if err != nil { + t.Fatal(err) + } + } else { + err := os.MkdirAll(filepath.Dir(dir), 0755) + if err != nil { + t.Fatal(err) + } + f, err := os.Create(dir) + if err != nil { + t.Fatal(err) + } + f.Close() } want := repoRoot{ -- 2.30.2