|
| 1 | +From e9d2c032b14c17083be0f8f0c822565199d2994f Mon Sep 17 00:00:00 2001 |
| 2 | +From: Roland Shoemaker < [email protected]> |
| 3 | +Date: Mon, 9 Jun 2025 11:23:46 -0700 |
| 4 | +Subject: [PATCH] [release-branch.go1.23] cmd/go: disable support for multiple |
| 5 | + vcs in one module |
| 6 | +Upstream Patch Reference: https://github.com/golang/go/commit/e9d2c032b14c17083be0f8f0c822565199d2994f.patch |
| 7 | + |
| 8 | +Removes the somewhat redundant vcs.FromDir, "allowNesting" argument, |
| 9 | +which was always enabled, and disallow multiple VCS metadata folders |
| 10 | +being present in a single directory. This makes VCS injection attacks |
| 11 | +much more difficult. |
| 12 | + |
| 13 | +Also adds a GODEBUG, allowmultiplevcs, which re-enables this behavior. |
| 14 | + |
| 15 | +Thanks to RyotaK (https://ryotak.net) of GMO Flatt Security Inc for |
| 16 | +reporting this issue. |
| 17 | + |
| 18 | +Updates #74380 |
| 19 | +Fixes #74382 |
| 20 | +Fixes CVE-2025-4674 |
| 21 | + |
| 22 | +Change-Id: I2db79f2baacfacfec331ee7c6978c4057d483eba |
| 23 | +Reviewed-on: https://go-review.googlesource.com/c/go/+/686337 |
| 24 | +LUCI-TryBot-Result: Go LUCI < [email protected]> |
| 25 | +Reviewed-by: David Chase < [email protected]> |
| 26 | +Reviewed-by: Carlos Amedee < [email protected]> |
| 27 | +Commit-Queue: Carlos Amedee < [email protected]> |
| 28 | +--- |
| 29 | + SECURITY.md | 5 ++ |
| 30 | + src/cmd/go/internal/get/get.go | 3 +- |
| 31 | + src/cmd/go/internal/load/pkg.go | 14 ++--- |
| 32 | + src/cmd/go/internal/vcs/vcs.go | 36 +++++++++---- |
| 33 | + src/cmd/go/internal/vcs/vcs_test.go | 2 +- |
| 34 | + src/cmd/go/testdata/script/test_multivcs.txt | 54 +++++++++++++++++++ |
| 35 | + .../script/version_buildvcs_nested.txt | 19 +++++-- |
| 36 | + src/runtime/metrics/doc.go | 5 ++ |
| 37 | + 8 files changed, 114 insertions(+), 24 deletions(-) |
| 38 | + create mode 100644 src/cmd/go/testdata/script/test_multivcs.txt |
| 39 | + |
| 40 | +diff --git a/SECURITY.md b/SECURITY.md |
| 41 | +index 9e92e8b..a0c7d15 100644 |
| 42 | +--- a/SECURITY.md |
| 43 | ++++ b/SECURITY.md |
| 44 | +@@ -10,4 +10,9 @@ part of that page. |
| 45 | + |
| 46 | + ## Reporting a Vulnerability |
| 47 | + |
| 48 | ++CVE-2025-4674: |
| 49 | ++Go 1.23.11 disabled build information stamping when multiple VCS are detected due |
| 50 | ++to concerns around VCS injection attacks. This behavior can be renabled with the |
| 51 | ++setting `allowmultiplevcs=1`. |
| 52 | ++ |
| 53 | + See https://golang.org/security for how to report a vulnerability. |
| 54 | +diff --git a/src/cmd/go/internal/get/get.go b/src/cmd/go/internal/get/get.go |
| 55 | +index 8cf8fe6..24aec66 100644 |
| 56 | +--- a/src/cmd/go/internal/get/get.go |
| 57 | ++++ b/src/cmd/go/internal/get/get.go |
| 58 | +@@ -446,8 +446,7 @@ func downloadPackage(p *load.Package) error { |
| 59 | + |
| 60 | + if p.Internal.Build.SrcRoot != "" { |
| 61 | + // Directory exists. Look for checkout along path to src. |
| 62 | +- const allowNesting = false |
| 63 | +- repoDir, vcsCmd, err = vcs.FromDir(p.Dir, p.Internal.Build.SrcRoot, allowNesting) |
| 64 | ++ repoDir, vcsCmd, err = vcs.FromDir(p.Dir, p.Internal.Build.SrcRoot) |
| 65 | + if err != nil { |
| 66 | + return err |
| 67 | + } |
| 68 | +diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go |
| 69 | +index 5b5b5ef..373c056 100644 |
| 70 | +--- a/src/cmd/go/internal/load/pkg.go |
| 71 | ++++ b/src/cmd/go/internal/load/pkg.go |
| 72 | +@@ -2392,9 +2392,8 @@ func (p *Package) setBuildInfo(includeVCS bool) { |
| 73 | + var repoDir string |
| 74 | + var vcsCmd *vcs.Cmd |
| 75 | + var err error |
| 76 | +- const allowNesting = true |
| 77 | + if includeVCS && cfg.BuildBuildvcs != "false" && p.Module != nil && p.Module.Version == "" && !p.Standard && !p.IsTestOnly() { |
| 78 | +- repoDir, vcsCmd, err = vcs.FromDir(base.Cwd(), "", allowNesting) |
| 79 | ++ repoDir, vcsCmd, err = vcs.FromDir(base.Cwd(), "") |
| 80 | + if err != nil && !errors.Is(err, os.ErrNotExist) { |
| 81 | + setVCSError(err) |
| 82 | + return |
| 83 | +@@ -2417,10 +2416,11 @@ func (p *Package) setBuildInfo(includeVCS bool) { |
| 84 | + } |
| 85 | + if repoDir != "" && vcsCmd.Status != nil { |
| 86 | + // Check that the current directory, package, and module are in the same |
| 87 | +- // repository. vcs.FromDir allows nested Git repositories, but nesting |
| 88 | +- // is not allowed for other VCS tools. The current directory may be outside |
| 89 | +- // p.Module.Dir when a workspace is used. |
| 90 | +- pkgRepoDir, _, err := vcs.FromDir(p.Dir, "", allowNesting) |
| 91 | ++ // repository. vcs.FromDir disallows nested VCS and multiple VCS in the |
| 92 | ++ // same repository, unless the GODEBUG allowmultiplevcs is set. The |
| 93 | ++ // current directory may be outside p.Module.Dir when a workspace is |
| 94 | ++ // used. |
| 95 | ++ pkgRepoDir, _, err := vcs.FromDir(p.Dir, "") |
| 96 | + if err != nil { |
| 97 | + setVCSError(err) |
| 98 | + return |
| 99 | +@@ -2432,7 +2432,7 @@ func (p *Package) setBuildInfo(includeVCS bool) { |
| 100 | + } |
| 101 | + goto omitVCS |
| 102 | + } |
| 103 | +- modRepoDir, _, err := vcs.FromDir(p.Module.Dir, "", allowNesting) |
| 104 | ++ modRepoDir, _, err := vcs.FromDir(p.Module.Dir, "") |
| 105 | + if err != nil { |
| 106 | + setVCSError(err) |
| 107 | + return |
| 108 | +diff --git a/src/cmd/go/internal/vcs/vcs.go b/src/cmd/go/internal/vcs/vcs.go |
| 109 | +index 2acabf7..39f38fb 100644 |
| 110 | +--- a/src/cmd/go/internal/vcs/vcs.go |
| 111 | ++++ b/src/cmd/go/internal/vcs/vcs.go |
| 112 | +@@ -767,11 +767,25 @@ type vcsPath struct { |
| 113 | + schemelessRepo bool // if true, the repo pattern lacks a scheme |
| 114 | + } |
| 115 | + |
| 116 | ++// allowMultipleVCS checks if GODEBUG=allowmultiplevcs=1 is set |
| 117 | ++func allowMultipleVCS() bool { |
| 118 | ++ val := os.Getenv("GODEBUG") |
| 119 | ++ for _, part := range strings.Split(val, ",") { |
| 120 | ++ part = strings.TrimSpace(part) // trim whitespace |
| 121 | ++ if strings.HasPrefix(part, "allowmultiplevcs=") { |
| 122 | ++ if strings.HasSuffix(part, "=1") { |
| 123 | ++ return true |
| 124 | ++ } |
| 125 | ++ } |
| 126 | ++ } |
| 127 | ++ return false |
| 128 | ++} |
| 129 | ++ |
| 130 | + // FromDir inspects dir and its parents to determine the |
| 131 | + // version control system and code repository to use. |
| 132 | + // If no repository is found, FromDir returns an error |
| 133 | + // equivalent to os.ErrNotExist. |
| 134 | +-func FromDir(dir, srcRoot string, allowNesting bool) (repoDir string, vcsCmd *Cmd, err error) { |
| 135 | ++func FromDir(dir, srcRoot string) (repoDir string, vcsCmd *Cmd, err error) { |
| 136 | + // Clean and double-check that dir is in (a subdirectory of) srcRoot. |
| 137 | + dir = filepath.Clean(dir) |
| 138 | + if srcRoot != "" { |
| 139 | +@@ -785,25 +798,30 @@ func FromDir(dir, srcRoot string, allowNesting bool) (repoDir string, vcsCmd *Cm |
| 140 | + for len(dir) > len(srcRoot) { |
| 141 | + for _, vcs := range vcsList { |
| 142 | + if _, err := statAny(dir, vcs.RootNames); err == nil { |
| 143 | +- // Record first VCS we find. |
| 144 | +- // If allowNesting is false (as it is in GOPATH), keep looking for |
| 145 | +- // repositories in parent directories and report an error if one is |
| 146 | +- // found to mitigate VCS injection attacks. |
| 147 | + if vcsCmd == nil { |
| 148 | + vcsCmd = vcs |
| 149 | + repoDir = dir |
| 150 | +- if allowNesting { |
| 151 | ++ if allowMultipleVCS() { |
| 152 | + return repoDir, vcsCmd, nil |
| 153 | + } |
| 154 | ++ // If allowmultiplevcs is not set, keep looking for |
| 155 | ++ // repositories in current and parent directories and report |
| 156 | ++ // an error if one is found to mitigate VCS injection |
| 157 | ++ // attacks. |
| 158 | ++ continue |
| 159 | ++ } |
| 160 | ++ if vcsCmd == vcsGit && vcs == vcsGit { |
| 161 | ++ // Nested Git is allowed, as this is how things like |
| 162 | ++ // submodules work. Git explicitly protects against |
| 163 | ++ // injection against itself. |
| 164 | + continue |
| 165 | + } |
| 166 | + // Allow .git inside .git, which can arise due to submodules. |
| 167 | + if vcsCmd == vcs && vcs.Cmd == "git" { |
| 168 | + continue |
| 169 | + } |
| 170 | +- // Otherwise, we have one VCS inside a different VCS. |
| 171 | +- return "", nil, fmt.Errorf("directory %q uses %s, but parent %q uses %s", |
| 172 | +- repoDir, vcsCmd.Cmd, dir, vcs.Cmd) |
| 173 | ++ return "", nil, fmt.Errorf("multiple VCS detected: %s in %q, and %s in %q", |
| 174 | ++ vcsCmd.Cmd, repoDir, vcs.Cmd, dir) |
| 175 | + } |
| 176 | + } |
| 177 | + |
| 178 | +diff --git a/src/cmd/go/internal/vcs/vcs_test.go b/src/cmd/go/internal/vcs/vcs_test.go |
| 179 | +index 943d520..fa122ca 100644 |
| 180 | +--- a/src/cmd/go/internal/vcs/vcs_test.go |
| 181 | ++++ b/src/cmd/go/internal/vcs/vcs_test.go |
| 182 | +@@ -243,7 +243,7 @@ func TestFromDir(t *testing.T) { |
| 183 | + } |
| 184 | + |
| 185 | + wantRepoDir := filepath.Dir(dir) |
| 186 | +- gotRepoDir, gotVCS, err := FromDir(dir, tempDir, false) |
| 187 | ++ gotRepoDir, gotVCS, err := FromDir(dir, tempDir) |
| 188 | + if err != nil { |
| 189 | + t.Errorf("FromDir(%q, %q): %v", dir, tempDir, err) |
| 190 | + continue |
| 191 | +diff --git a/src/cmd/go/testdata/script/test_multivcs.txt b/src/cmd/go/testdata/script/test_multivcs.txt |
| 192 | +new file mode 100644 |
| 193 | +index 0000000..538cbf7 |
| 194 | +--- /dev/null |
| 195 | ++++ b/src/cmd/go/testdata/script/test_multivcs.txt |
| 196 | +@@ -0,0 +1,54 @@ |
| 197 | ++# To avoid VCS injection attacks, we should not accept multiple different VCS metadata |
| 198 | ++# folders within a single module (either in the same directory, or nested in different |
| 199 | ++# directories.) |
| 200 | ++# |
| 201 | ++# This behavior should be disabled by setting the allowmultiplevcs GODEBUG. |
| 202 | ++ |
| 203 | ++[short] skip |
| 204 | ++[!git] skip |
| 205 | ++ |
| 206 | ++cd samedir |
| 207 | ++ |
| 208 | ++exec git init . |
| 209 | ++ |
| 210 | ++# Without explicitly requesting buildvcs, the go command should silently continue |
| 211 | ++# without determining the correct VCS. |
| 212 | ++go test -c -o $devnull . |
| 213 | ++ |
| 214 | ++# If buildvcs is explicitly requested, we expect the go command to fail |
| 215 | ++! go test -buildvcs -c -o $devnull . |
| 216 | ++stderr '^error obtaining VCS status: multiple VCS detected:' |
| 217 | ++ |
| 218 | ++env GODEBUG=allowmultiplevcs=1 |
| 219 | ++go test -buildvcs -c -o $devnull . |
| 220 | ++ |
| 221 | ++env GODEBUG= |
| 222 | ++cd ../nested |
| 223 | ++exec git init . |
| 224 | ++# cd a |
| 225 | ++go test -c -o $devnull ./a |
| 226 | ++! go test -buildvcs -c -o $devnull ./a |
| 227 | ++stderr '^error obtaining VCS status: multiple VCS detected:' |
| 228 | ++# allowmultiplevcs doesn't disable the check that the current directory, package, and |
| 229 | ++# module are in the same repository. |
| 230 | ++env GODEBUG=allowmultiplevcs=1 |
| 231 | ++! go test -buildvcs -c -o $devnull ./a |
| 232 | ++stderr '^error obtaining VCS status: main package is in repository' |
| 233 | ++ |
| 234 | ++-- samedir/go.mod -- |
| 235 | ++module example |
| 236 | ++ |
| 237 | ++go 1.18 |
| 238 | ++-- samedir/example.go -- |
| 239 | ++package main |
| 240 | ++-- samedir/.bzr/test -- |
| 241 | ++hello |
| 242 | ++ |
| 243 | ++-- nested/go.mod -- |
| 244 | ++module example |
| 245 | ++ |
| 246 | ++go 1.18 |
| 247 | ++-- nested/a/example.go -- |
| 248 | ++package main |
| 249 | ++-- nested/a/.bzr/test -- |
| 250 | ++hello |
| 251 | +diff --git a/src/cmd/go/testdata/script/version_buildvcs_nested.txt b/src/cmd/go/testdata/script/version_buildvcs_nested.txt |
| 252 | +index a0c69f9..1eafff8 100644 |
| 253 | +--- a/src/cmd/go/testdata/script/version_buildvcs_nested.txt |
| 254 | ++++ b/src/cmd/go/testdata/script/version_buildvcs_nested.txt |
| 255 | +@@ -9,25 +9,34 @@ cd root |
| 256 | + go mod init example.com/root |
| 257 | + exec git init |
| 258 | + |
| 259 | +-# Nesting repositories in parent directories are ignored, as the current |
| 260 | +-# directory main package, and containing main module are in the same repository. |
| 261 | +-# This is an error in GOPATH mode (to prevent VCS injection), but for modules, |
| 262 | +-# we assume users have control over repositories they've checked out. |
| 263 | ++# Nesting repositories in parent directories are an error, to prevent VCS injection. |
| 264 | ++# This can be disabled with the allowmultiplevcs GODEBUG. |
| 265 | + mkdir hgsub |
| 266 | + cd hgsub |
| 267 | + exec hg init |
| 268 | + cp ../../main.go main.go |
| 269 | + ! go build |
| 270 | ++stderr '^error obtaining VCS status: multiple VCS detected: hg in ".*hgsub", and git in ".*root"$' |
| 271 | ++stderr '^\tUse -buildvcs=false to disable VCS stamping.$' |
| 272 | ++env GODEBUG=allowmultiplevcs=1 |
| 273 | ++! go build |
| 274 | + stderr '^error obtaining VCS status: main module is in repository ".*root" but current directory is in repository ".*hgsub"$' |
| 275 | + stderr '^\tUse -buildvcs=false to disable VCS stamping.$' |
| 276 | + go build -buildvcs=false |
| 277 | ++env GODEBUG= |
| 278 | + go mod init example.com/root/hgsub |
| 279 | ++! go build |
| 280 | ++stderr '^error obtaining VCS status: multiple VCS detected: hg in ".*hgsub", and git in ".*root"$' |
| 281 | ++stderr '^\tUse -buildvcs=false to disable VCS stamping.$' |
| 282 | ++env GODEBUG=allowmultiplevcs=1 |
| 283 | + go build |
| 284 | ++env GODEBUG= |
| 285 | + cd .. |
| 286 | + |
| 287 | + # It's an error to build a package from a nested Git repository if the package |
| 288 | + # is in a separate repository from the current directory or from the module |
| 289 | +-# root directory. |
| 290 | ++# root directory. Otherwise nested Git repositories are allowed, as this is |
| 291 | ++# how Git implements submodules (and protects against Git based VCS injection.) |
| 292 | + mkdir gitsub |
| 293 | + cd gitsub |
| 294 | + exec git init |
| 295 | +diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go |
| 296 | +index 91ef030..27ae51c 100644 |
| 297 | +--- a/src/runtime/metrics/doc.go |
| 298 | ++++ b/src/runtime/metrics/doc.go |
| 299 | +@@ -102,6 +102,11 @@ Below is the full list of supported metrics, ordered lexicographically. |
| 300 | + /gc/pauses:seconds |
| 301 | + Distribution individual GC-related stop-the-world pause latencies. |
| 302 | + |
| 303 | ++ /godebug/non-default-behavior/allowmultiplevcs:events |
| 304 | ++ The number of non-default behaviors executed by the cmd/go |
| 305 | ++ package due to a non-default GODEBUG=allowmultiplevcs=... |
| 306 | ++ setting. |
| 307 | ++ |
| 308 | + /memory/classes/heap/free:bytes |
| 309 | + Memory that is completely free and eligible to be returned to |
| 310 | + the underlying system, but has not been. This metric is the |
| 311 | +-- |
| 312 | +2.45.4 |
| 313 | + |
0 commit comments