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 <beuc@debian.org>
Last-Update: 2021-03-02

From 9a97c3bfe41d1ed768ea3bd3d8f0f52b8a51bb62 Mon Sep 17 00:00:00 2001
From: Russ Cox <rsc@golang.org>
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 <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Gbp-Pq: Name CVE-2017-15041.patch

diff --git a/src/cmd/go/get.go b/src/cmd/go/get.go
index 19858f7e55db928bbaaef5961f2229f59a1f9bdd..4f87543c1307f1f64edd6acd2cb316758df69a3a 100644 (file)
--- 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 66c641347cb27b5e8fd68d7aaf282c354b64a07b..f11cb67b8b6601ae9c76f43170160a8bd30649cb 100644 (file)
--- 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 9cd46884ff4d7986e9e40676b597f0666b61f183..436c0ce1c009e0d205223a850dd8687042a69b77 100644 (file)
--- 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 f7a6a833fc372209881fcc48347b0363e6ae9f8f..0e030df430f37d54592663d7b0adefa6559c4431 100644 (file)
--- 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{