From e9d2c032b14c17083be0f8f0c822565199d2994f Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 9 Jun 2025 11:23:46 -0700 Subject: [PATCH] [release-branch.go1.23] 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. Updates #74380 Fixes #74382 Fixes CVE-2025-4674 Change-Id: I2db79f2baacfacfec331ee7c6978c4057d483eba Reviewed-on: https://go-review.googlesource.com/c/go/+/686337 LUCI-TryBot-Result: Go LUCI Reviewed-by: David Chase Reviewed-by: Carlos Amedee Commit-Queue: Carlos Amedee --- doc/godebug.md | 4 ++ src/cmd/go/internal/load/pkg.go | 14 ++--- 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/godebugs_test.go | 3 +- 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 b3a43664c4..9d3c810cdc 100644 --- a/doc/godebug.md +++ b/doc/godebug.md @@ -209,6 +209,10 @@ when serving an error. This behavior is controlled by the [`httpservecontentkeepheaders` setting](/pkg/net/http#ServeContent). Using `httpservecontentkeepheaders=1` restores the pre-Go 1.23 behavior. +Go 1.23.11 disabled build information stamping when multiple VCS are detected due +to concerns around VCS injection attacks. This behavior can be renabled with the +setting `allowmultiplevcs=1`. + ### Go 1.22 Go 1.22 adds a configurable limit to control the maximum acceptable RSA key size diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 7c402b419e..d45b296599 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -2409,7 +2409,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 { @@ -2429,7 +2428,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 @@ -2452,10 +2451,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 @@ -2467,7 +2467,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/vcs/vcs.go b/src/cmd/go/internal/vcs/vcs.go index 19a6a5ef6b..f2f362b57c 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" @@ -831,11 +832,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 != "" { @@ -849,21 +852,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 2ce85ea210..06e63c2952 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/godebugs_test.go b/src/internal/godebugs/godebugs_test.go index 046193b5c6..168acc134a 100644 --- a/src/internal/godebugs/godebugs_test.go +++ b/src/internal/godebugs/godebugs_test.go @@ -46,7 +46,8 @@ func TestAll(t *testing.T) { if info.Old != "" && info.Changed == 0 { t.Errorf("Name=%s has Old, missing Changed", info.Name) } - if !strings.Contains(doc, "`"+info.Name+"`") { + if !strings.Contains(doc, "`"+info.Name+"`") && + !strings.Contains(doc, "`"+info.Name+"=") { t.Errorf("Name=%s not documented in doc/godebug.md", info.Name) } if !info.Opaque && !incs[info.Name] { diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go index 473a0992df..f5831bc54c 100644 --- a/src/internal/godebugs/table.go +++ b/src/internal/godebugs/table.go @@ -25,6 +25,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: "execerrdot", Package: "os/exec"}, {Name: "gocachehash", Package: "cmd/go"}, diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go index da3d956d48..3575ca02d8 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