From 54c9d776302d53ab1907645cb67fa4a948e1500c Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 9 Jun 2025 11:23:46 -0700 Subject: [PATCH] cmd/go: disable support for multiple vcs in one module Removes the somewhat redundant vcs.FromDir, "allowNesting" argument, which was always enabled, and disallow multiple VCS metadata folders being present in a single directory. This makes VCS injection attacks much more difficult. Also adds a GODEBUG, allowmultiplevcs, which re-enables this behavior. Thanks to RyotaK (https://ryotak.net) of GMO Flatt Security Inc for reporting this issue. Fixes #74380 Fixes CVE-2025-4674 Change-Id: I5787d90cdca8deb3aca6f154efb627df1e7d2789 Reviewed-on: https://go-review.googlesource.com/c/go/+/686515 LUCI-TryBot-Result: Go LUCI Reviewed-by: David Chase Commit-Queue: Carlos Amedee Reviewed-by: Carlos Amedee --- doc/godebug.md | 5 ++ src/cmd/go/internal/load/pkg.go | 14 ++--- src/cmd/go/internal/modfetch/repo.go | 2 +- src/cmd/go/internal/vcs/vcs.go | 28 ++++++---- src/cmd/go/internal/vcs/vcs_test.go | 2 +- src/cmd/go/testdata/script/test_multivcs.txt | 54 +++++++++++++++++++ .../script/version_buildvcs_nested.txt | 20 +++++-- src/internal/godebugs/table.go | 1 + src/runtime/metrics/doc.go | 5 ++ 9 files changed, 108 insertions(+), 23 deletions(-) create mode 100644 src/cmd/go/testdata/script/test_multivcs.txt diff --git a/doc/godebug.md b/doc/godebug.md index d107b1baf1..aaa0f9dd55 100644 --- a/doc/godebug.md +++ b/doc/godebug.md @@ -189,6 +189,11 @@ crypto/x509.CreateCertificate. The setting `x509sha256skid=0` reverts to SHA-1. Go 1.25 corrected the semantics of contention reports for runtime-internal locks, and so removed the [`runtimecontentionstacks` setting](/pkg/runtime#hdr-Environment_Variables). +Go 1.25 (starting with Go 1.25 RC 2) disabled build information stamping when +multiple VCS are detected due to concerns around VCS injection attacks. This +behavior and setting was backported to Go 1.24.5 and Go 1.23.11. This behavior +can be renabled with the setting `allowmultiplevcs=1`. + ### Go 1.24 Go 1.24 added a new `fips140` setting that controls whether the Go diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index e913f98852..1f791546f9 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -2496,7 +2496,6 @@ func (p *Package) setBuildInfo(ctx context.Context, autoVCS bool) { var repoDir string var vcsCmd *vcs.Cmd var err error - const allowNesting = true wantVCS := false switch cfg.BuildBuildvcs { @@ -2516,7 +2515,7 @@ func (p *Package) setBuildInfo(ctx context.Context, autoVCS bool) { // (so the bootstrap toolchain packages don't even appear to be in GOROOT). goto omitVCS } - repoDir, vcsCmd, err = vcs.FromDir(base.Cwd(), "", allowNesting) + repoDir, vcsCmd, err = vcs.FromDir(base.Cwd(), "") if err != nil && !errors.Is(err, os.ErrNotExist) { setVCSError(err) return @@ -2539,10 +2538,11 @@ func (p *Package) setBuildInfo(ctx context.Context, autoVCS bool) { } if repoDir != "" && vcsCmd.Status != nil { // Check that the current directory, package, and module are in the same - // repository. vcs.FromDir allows nested Git repositories, but nesting - // is not allowed for other VCS tools. The current directory may be outside - // p.Module.Dir when a workspace is used. - pkgRepoDir, _, err := vcs.FromDir(p.Dir, "", allowNesting) + // repository. vcs.FromDir disallows nested VCS and multiple VCS in the + // same repository, unless the GODEBUG allowmultiplevcs is set. The + // current directory may be outside p.Module.Dir when a workspace is + // used. + pkgRepoDir, _, err := vcs.FromDir(p.Dir, "") if err != nil { setVCSError(err) return @@ -2554,7 +2554,7 @@ func (p *Package) setBuildInfo(ctx context.Context, autoVCS bool) { } goto omitVCS } - modRepoDir, _, err := vcs.FromDir(p.Module.Dir, "", allowNesting) + modRepoDir, _, err := vcs.FromDir(p.Module.Dir, "") if err != nil { setVCSError(err) return diff --git a/src/cmd/go/internal/modfetch/repo.go b/src/cmd/go/internal/modfetch/repo.go index 0fdb2a8736..5d4d679e83 100644 --- a/src/cmd/go/internal/modfetch/repo.go +++ b/src/cmd/go/internal/modfetch/repo.go @@ -235,7 +235,7 @@ func LookupLocal(ctx context.Context, codeRoot string, path string, dir string) return lookupLocalCache.Do(path, func() Repo { return newCachingRepo(ctx, path, func(ctx context.Context) (Repo, error) { - repoDir, vcsCmd, err := vcs.FromDir(dir, "", true) + repoDir, vcsCmd, err := vcs.FromDir(dir, "") if err != nil { return nil, err } diff --git a/src/cmd/go/internal/vcs/vcs.go b/src/cmd/go/internal/vcs/vcs.go index ebcb2efb34..7e081eb41a 100644 --- a/src/cmd/go/internal/vcs/vcs.go +++ b/src/cmd/go/internal/vcs/vcs.go @@ -8,6 +8,7 @@ import ( "bytes" "errors" "fmt" + "internal/godebug" "internal/lazyregexp" "internal/singleflight" "io/fs" @@ -869,11 +870,13 @@ type vcsPath struct { schemelessRepo bool // if true, the repo pattern lacks a scheme } +var allowmultiplevcs = godebug.New("allowmultiplevcs") + // FromDir inspects dir and its parents to determine the // version control system and code repository to use. // If no repository is found, FromDir returns an error // equivalent to os.ErrNotExist. -func FromDir(dir, srcRoot string, allowNesting bool) (repoDir string, vcsCmd *Cmd, err error) { +func FromDir(dir, srcRoot string) (repoDir string, vcsCmd *Cmd, err error) { // Clean and double-check that dir is in (a subdirectory of) srcRoot. dir = filepath.Clean(dir) if srcRoot != "" { @@ -887,21 +890,28 @@ func FromDir(dir, srcRoot string, allowNesting bool) (repoDir string, vcsCmd *Cm for len(dir) > len(srcRoot) { for _, vcs := range vcsList { if isVCSRoot(dir, vcs.RootNames) { - // Record first VCS we find. - // If allowNesting is false (as it is in GOPATH), keep looking for - // repositories in parent directories and report an error if one is - // found to mitigate VCS injection attacks. if vcsCmd == nil { + // Record first VCS we find. vcsCmd = vcs repoDir = dir - if allowNesting { + if allowmultiplevcs.Value() == "1" { + allowmultiplevcs.IncNonDefault() return repoDir, vcsCmd, nil } + // If allowmultiplevcs is not set, keep looking for + // repositories in current and parent directories and report + // an error if one is found to mitigate VCS injection + // attacks. + continue + } + if vcsCmd == vcsGit && vcs == vcsGit { + // Nested Git is allowed, as this is how things like + // submodules work. Git explicitly protects against + // injection against itself. continue } - // Otherwise, we have one VCS inside a different VCS. - return "", nil, fmt.Errorf("directory %q uses %s, but parent %q uses %s", - repoDir, vcsCmd.Cmd, dir, vcs.Cmd) + return "", nil, fmt.Errorf("multiple VCS detected: %s in %q, and %s in %q", + vcsCmd.Cmd, repoDir, vcs.Cmd, dir) } } diff --git a/src/cmd/go/internal/vcs/vcs_test.go b/src/cmd/go/internal/vcs/vcs_test.go index c143154948..361d85bcfb 100644 --- a/src/cmd/go/internal/vcs/vcs_test.go +++ b/src/cmd/go/internal/vcs/vcs_test.go @@ -239,7 +239,7 @@ func TestFromDir(t *testing.T) { } wantRepoDir := filepath.Dir(dir) - gotRepoDir, gotVCS, err := FromDir(dir, tempDir, false) + gotRepoDir, gotVCS, err := FromDir(dir, tempDir) if err != nil { t.Errorf("FromDir(%q, %q): %v", dir, tempDir, err) continue diff --git a/src/cmd/go/testdata/script/test_multivcs.txt b/src/cmd/go/testdata/script/test_multivcs.txt new file mode 100644 index 0000000000..538cbf700b --- /dev/null +++ b/src/cmd/go/testdata/script/test_multivcs.txt @@ -0,0 +1,54 @@ +# To avoid VCS injection attacks, we should not accept multiple different VCS metadata +# folders within a single module (either in the same directory, or nested in different +# directories.) +# +# This behavior should be disabled by setting the allowmultiplevcs GODEBUG. + +[short] skip +[!git] skip + +cd samedir + +exec git init . + +# Without explicitly requesting buildvcs, the go command should silently continue +# without determining the correct VCS. +go test -c -o $devnull . + +# If buildvcs is explicitly requested, we expect the go command to fail +! go test -buildvcs -c -o $devnull . +stderr '^error obtaining VCS status: multiple VCS detected:' + +env GODEBUG=allowmultiplevcs=1 +go test -buildvcs -c -o $devnull . + +env GODEBUG= +cd ../nested +exec git init . +# cd a +go test -c -o $devnull ./a +! go test -buildvcs -c -o $devnull ./a +stderr '^error obtaining VCS status: multiple VCS detected:' +# allowmultiplevcs doesn't disable the check that the current directory, package, and +# module are in the same repository. +env GODEBUG=allowmultiplevcs=1 +! go test -buildvcs -c -o $devnull ./a +stderr '^error obtaining VCS status: main package is in repository' + +-- samedir/go.mod -- +module example + +go 1.18 +-- samedir/example.go -- +package main +-- samedir/.bzr/test -- +hello + +-- nested/go.mod -- +module example + +go 1.18 +-- nested/a/example.go -- +package main +-- nested/a/.bzr/test -- +hello diff --git a/src/cmd/go/testdata/script/version_buildvcs_nested.txt b/src/cmd/go/testdata/script/version_buildvcs_nested.txt index 6dab8474b5..22cd71c454 100644 --- a/src/cmd/go/testdata/script/version_buildvcs_nested.txt +++ b/src/cmd/go/testdata/script/version_buildvcs_nested.txt @@ -9,25 +9,35 @@ cd root go mod init example.com/root exec git init -# Nesting repositories in parent directories are ignored, as the current -# directory main package, and containing main module are in the same repository. -# This is an error in GOPATH mode (to prevent VCS injection), but for modules, -# we assume users have control over repositories they've checked out. + +# Nesting repositories in parent directories are an error, to prevent VCS injection. +# This can be disabled with the allowmultiplevcs GODEBUG. mkdir hgsub cd hgsub exec hg init cp ../../main.go main.go ! go build +stderr '^error obtaining VCS status: multiple VCS detected: hg in ".*hgsub", and git in ".*root"$' +stderr '^\tUse -buildvcs=false to disable VCS stamping.$' +env GODEBUG=allowmultiplevcs=1 +! go build stderr '^error obtaining VCS status: main module is in repository ".*root" but current directory is in repository ".*hgsub"$' stderr '^\tUse -buildvcs=false to disable VCS stamping.$' go build -buildvcs=false +env GODEBUG= go mod init example.com/root/hgsub +! go build +stderr '^error obtaining VCS status: multiple VCS detected: hg in ".*hgsub", and git in ".*root"$' +stderr '^\tUse -buildvcs=false to disable VCS stamping.$' +env GODEBUG=allowmultiplevcs=1 go build +env GODEBUG= cd .. # It's an error to build a package from a nested Git repository if the package # is in a separate repository from the current directory or from the module -# root directory. +# root directory. Otherwise nested Git repositories are allowed, as this is +# how Git implements submodules (and protects against Git based VCS injection.) mkdir gitsub cd gitsub exec git init diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go index 38dc7b0fac..2d00882545 100644 --- a/src/internal/godebugs/table.go +++ b/src/internal/godebugs/table.go @@ -26,6 +26,7 @@ type Info struct { // Note: After adding entries to this table, update the list in doc/godebug.md as well. // (Otherwise the test in this package will fail.) var All = []Info{ + {Name: "allowmultiplevcs", Package: "cmd/go"}, {Name: "asynctimerchan", Package: "time", Changed: 23, Old: "1"}, {Name: "containermaxprocs", Package: "runtime", Changed: 25, Old: "0"}, {Name: "dataindependenttiming", Package: "crypto/subtle", Opaque: true}, diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go index 32fc436e1a..a1902bc6d7 100644 --- a/src/runtime/metrics/doc.go +++ b/src/runtime/metrics/doc.go @@ -230,6 +230,11 @@ Below is the full list of supported metrics, ordered lexicographically. /gc/stack/starting-size:bytes The stack size of new goroutines. + /godebug/non-default-behavior/allowmultiplevcs:events + The number of non-default behaviors executed by the cmd/go + package due to a non-default GODEBUG=allowmultiplevcs=... + setting. + /godebug/non-default-behavior/asynctimerchan:events The number of non-default behaviors executed by the time package due to a non-default GODEBUG=asynctimerchan=... setting. -- 2.50.0