[release-branch.go1.24] 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 #74381
Fixes CVE-2025-4674

Change-Id: I6c7925b034d60b80d7698cca677b00bdcc67f24e
Reviewed-on: https://go-review.googlesource.com/c/go/+/686395
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Commit-Queue: Carlos Amedee <carlos@golang.org>
This commit is contained in:
Roland Shoemaker 2025-06-09 11:23:46 -07:00 committed by Carlos Amedee
parent dbf30d88f3
commit 825eeee3f7
10 changed files with 109 additions and 24 deletions

View file

@ -224,6 +224,10 @@ use and validate the CRT parameters in the encoded private key. This behavior
can be controlled with the `x509rsacrt` setting. Using `x509rsacrt=0` restores can be controlled with the `x509rsacrt` setting. Using `x509rsacrt=0` restores
the Go 1.23 behavior. the Go 1.23 behavior.
Go 1.24.5 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.23 ### Go 1.23
Go 1.23 changed the channels created by package time to be unbuffered Go 1.23 changed the channels created by package time to be unbuffered

View file

@ -2474,7 +2474,6 @@ func (p *Package) setBuildInfo(ctx context.Context, autoVCS bool) {
var repoDir string var repoDir string
var vcsCmd *vcs.Cmd var vcsCmd *vcs.Cmd
var err error var err error
const allowNesting = true
wantVCS := false wantVCS := false
switch cfg.BuildBuildvcs { switch cfg.BuildBuildvcs {
@ -2494,7 +2493,7 @@ func (p *Package) setBuildInfo(ctx context.Context, autoVCS bool) {
// (so the bootstrap toolchain packages don't even appear to be in GOROOT). // (so the bootstrap toolchain packages don't even appear to be in GOROOT).
goto omitVCS 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) { if err != nil && !errors.Is(err, os.ErrNotExist) {
setVCSError(err) setVCSError(err)
return return
@ -2517,10 +2516,11 @@ func (p *Package) setBuildInfo(ctx context.Context, autoVCS bool) {
} }
if repoDir != "" && vcsCmd.Status != nil { if repoDir != "" && vcsCmd.Status != nil {
// Check that the current directory, package, and module are in the same // Check that the current directory, package, and module are in the same
// repository. vcs.FromDir allows nested Git repositories, but nesting // repository. vcs.FromDir disallows nested VCS and multiple VCS in the
// is not allowed for other VCS tools. The current directory may be outside // same repository, unless the GODEBUG allowmultiplevcs is set. The
// p.Module.Dir when a workspace is used. // current directory may be outside p.Module.Dir when a workspace is
pkgRepoDir, _, err := vcs.FromDir(p.Dir, "", allowNesting) // used.
pkgRepoDir, _, err := vcs.FromDir(p.Dir, "")
if err != nil { if err != nil {
setVCSError(err) setVCSError(err)
return return
@ -2532,7 +2532,7 @@ func (p *Package) setBuildInfo(ctx context.Context, autoVCS bool) {
} }
goto omitVCS goto omitVCS
} }
modRepoDir, _, err := vcs.FromDir(p.Module.Dir, "", allowNesting) modRepoDir, _, err := vcs.FromDir(p.Module.Dir, "")
if err != nil { if err != nil {
setVCSError(err) setVCSError(err)
return return

View file

@ -230,7 +230,7 @@ func LookupLocal(ctx context.Context, path string) Repo {
return lookupLocalCache.Do(path, func() Repo { return lookupLocalCache.Do(path, func() Repo {
return newCachingRepo(ctx, path, func(ctx context.Context) (Repo, error) { return newCachingRepo(ctx, path, func(ctx context.Context) (Repo, error) {
repoDir, vcsCmd, err := vcs.FromDir(path, "", true) repoDir, vcsCmd, err := vcs.FromDir(path, "")
if err != nil { if err != nil {
return nil, err return nil, err
} }

View file

@ -8,6 +8,7 @@ import (
"bytes" "bytes"
"errors" "errors"
"fmt" "fmt"
"internal/godebug"
"internal/lazyregexp" "internal/lazyregexp"
"internal/singleflight" "internal/singleflight"
"io/fs" "io/fs"
@ -839,11 +840,13 @@ type vcsPath struct {
schemelessRepo bool // if true, the repo pattern lacks a scheme schemelessRepo bool // if true, the repo pattern lacks a scheme
} }
var allowmultiplevcs = godebug.New("allowmultiplevcs")
// FromDir inspects dir and its parents to determine the // FromDir inspects dir and its parents to determine the
// version control system and code repository to use. // version control system and code repository to use.
// If no repository is found, FromDir returns an error // If no repository is found, FromDir returns an error
// equivalent to os.ErrNotExist. // 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. // Clean and double-check that dir is in (a subdirectory of) srcRoot.
dir = filepath.Clean(dir) dir = filepath.Clean(dir)
if srcRoot != "" { if srcRoot != "" {
@ -857,21 +860,28 @@ func FromDir(dir, srcRoot string, allowNesting bool) (repoDir string, vcsCmd *Cm
for len(dir) > len(srcRoot) { for len(dir) > len(srcRoot) {
for _, vcs := range vcsList { for _, vcs := range vcsList {
if isVCSRoot(dir, vcs.RootNames) { 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 { if vcsCmd == nil {
// Record first VCS we find.
vcsCmd = vcs vcsCmd = vcs
repoDir = dir repoDir = dir
if allowNesting { if allowmultiplevcs.Value() == "1" {
allowmultiplevcs.IncNonDefault()
return repoDir, vcsCmd, nil 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 continue
} }
// Otherwise, we have one VCS inside a different VCS. if vcsCmd == vcsGit && vcs == vcsGit {
return "", nil, fmt.Errorf("directory %q uses %s, but parent %q uses %s", // Nested Git is allowed, as this is how things like
repoDir, vcsCmd.Cmd, dir, vcs.Cmd) // submodules work. Git explicitly protects against
// injection against itself.
continue
}
return "", nil, fmt.Errorf("multiple VCS detected: %s in %q, and %s in %q",
vcsCmd.Cmd, repoDir, vcs.Cmd, dir)
} }
} }

View file

@ -239,7 +239,7 @@ func TestFromDir(t *testing.T) {
} }
wantRepoDir := filepath.Dir(dir) wantRepoDir := filepath.Dir(dir)
gotRepoDir, gotVCS, err := FromDir(dir, tempDir, false) gotRepoDir, gotVCS, err := FromDir(dir, tempDir)
if err != nil { if err != nil {
t.Errorf("FromDir(%q, %q): %v", dir, tempDir, err) t.Errorf("FromDir(%q, %q): %v", dir, tempDir, err)
continue continue

View file

@ -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

View file

@ -9,25 +9,35 @@ cd root
go mod init example.com/root go mod init example.com/root
exec git init 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. # Nesting repositories in parent directories are an error, to prevent VCS injection.
# This is an error in GOPATH mode (to prevent VCS injection), but for modules, # This can be disabled with the allowmultiplevcs GODEBUG.
# we assume users have control over repositories they've checked out.
mkdir hgsub mkdir hgsub
cd hgsub cd hgsub
exec hg init exec hg init
cp ../../main.go main.go cp ../../main.go main.go
! go build ! 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 '^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.$' stderr '^\tUse -buildvcs=false to disable VCS stamping.$'
go build -buildvcs=false go build -buildvcs=false
env GODEBUG=
go mod init example.com/root/hgsub 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 go build
env GODEBUG=
cd .. cd ..
# It's an error to build a package from a nested Git repository if the package # 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 # 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 mkdir gitsub
cd gitsub cd gitsub
exec git init exec git init

View file

@ -46,7 +46,8 @@ func TestAll(t *testing.T) {
if info.Old != "" && info.Changed == 0 { if info.Old != "" && info.Changed == 0 {
t.Errorf("Name=%s has Old, missing Changed", info.Name) 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) t.Errorf("Name=%s not documented in doc/godebug.md", info.Name)
} }
if !info.Opaque && !incs[info.Name] { if !info.Opaque && !incs[info.Name] {

View file

@ -25,6 +25,7 @@ type Info struct {
// Note: After adding entries to this table, update the list in doc/godebug.md as well. // Note: After adding entries to this table, update the list in doc/godebug.md as well.
// (Otherwise the test in this package will fail.) // (Otherwise the test in this package will fail.)
var All = []Info{ var All = []Info{
{Name: "allowmultiplevcs", Package: "cmd/go"},
{Name: "asynctimerchan", Package: "time", Changed: 23, Old: "1"}, {Name: "asynctimerchan", Package: "time", Changed: 23, Old: "1"},
{Name: "dataindependenttiming", Package: "crypto/subtle", Opaque: true}, {Name: "dataindependenttiming", Package: "crypto/subtle", Opaque: true},
{Name: "execerrdot", Package: "os/exec"}, {Name: "execerrdot", Package: "os/exec"},

View file

@ -230,6 +230,11 @@ Below is the full list of supported metrics, ordered lexicographically.
/gc/stack/starting-size:bytes /gc/stack/starting-size:bytes
The stack size of new goroutines. 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 /godebug/non-default-behavior/asynctimerchan:events
The number of non-default behaviors executed by the time package The number of non-default behaviors executed by the time package
due to a non-default GODEBUG=asynctimerchan=... setting. due to a non-default GODEBUG=asynctimerchan=... setting.