[release-branch.go1.20] cmd/go: disallow package directories containing newlines

Directory or file paths containing newlines may cause tools (such as
cmd/cgo) that emit "//line" or "#line" -directives to write part of
the path into non-comment lines in generated source code. If those
lines contain valid Go code, it may be injected into the resulting
binary.

(Note that Go import paths and file paths within module zip files
already could not contain newlines.)

Thanks to Juho Nurminen of Mattermost for reporting this issue.

Updates #60167.
Fixes #60516.
Fixes CVE-2023-29402.

Change-Id: Ic3c7d8d1f6460993bd93a27035d61bff7dd68832
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1882606
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Russ Cox <rsc@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 41f9046495564fc728d6f98384ab7276450ac7e2)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1902230
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1904347
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/501222
Run-TryBot: David Chase <drchase@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go
index 56a4e5eaedd57577d649d424a0c71815940ef48d..f427e29c0d01ffaeb31213f34791ef5a6f8ce37d 100644 (file)
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -1960,6 +1960,10 @@ func (p *Package) load(ctx context.Context, opts PackageOpts, path string, stk *
                setError(fmt.Errorf("invalid input directory name %q", name))
                return
        }
+       if strings.ContainsAny(p.Dir, "\r\n") {
+               setError(fmt.Errorf("invalid package directory %q", p.Dir))
+               return
+       }
 
        // Build list of imported packages and full dependency list.
        imports := make([]*Package, 0, len(p.Imports))

diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 02a59e48e9a77ac6b039f47f10deecf9a24e1f60..9cf3362fbfb96f0db2fd3b1fd228f54b0e8a04bb 100644 (file)
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -516,6 +516,12 @@ func (b *Builder) build(ctx context.Context, a *Action) (err error) {
                b.Print(p.ImportPath + "\n")
        }
 
+       if p.Error != nil {
+               // Don't try to build anything for packages with errors. There may be a
+               // problem with the inputs that makes the package unsafe to build.
+               return p.Error
+       }
+
        if p.BinaryOnly {
                p.Stale = true
                p.StaleReason = "binary-only packages are no longer supported"

diff --git a/src/cmd/go/script_test.go b/src/cmd/go/script_test.go
index 4211fb6121c674af33e60c0c7406609a5206ec7a..ddd2b57737c2451c5c9d5f689f371e92a0d08465 100644 (file)
--- a/src/cmd/go/script_test.go
+++ b/src/cmd/go/script_test.go
@@ -222,6 +222,7 @@ func scriptEnv(srv *vcstest.Server, srvCertFile string) ([]string, error) {
                "devnull=" + os.DevNull,
                "goversion=" + version,
                "CMDGO_TEST_RUN_MAIN=true",
+               "newline=\n",
        }
 
        if testenv.Builder() != "" || os.Getenv("GIT_TRACE_CURL") == "1" {

diff --git a/src/cmd/go/testdata/script/build_cwd_newline.txt b/src/cmd/go/testdata/script/build_cwd_newline.txt
new file mode 100644 (file)
index 0000000..574464c
--- /dev/null
+++ b/src/cmd/go/testdata/script/build_cwd_newline.txt
@@ -0,0 +1,104 @@
+[GOOS:windows] skip 'filesystem normalizes / to \'
+[GOOS:plan9] skip 'filesystem disallows \n in paths'
+
+# If the directory path containing a package to be built includes a newline,
+# the go command should refuse to even try to build the package.
+
+env DIR=$WORK${/}${newline}'package main'${newline}'func main() { panic("uh-oh")'${newline}'/*'
+
+mkdir $DIR
+cd $DIR
+exec pwd
+cp $WORK/go.mod ./go.mod
+cp $WORK/main.go ./main.go
+cp $WORK/main_test.go ./main_test.go
+
+! go build -o $devnull .
+stderr 'package example: invalid package directory .*uh-oh'
+
+! go build -o $devnull main.go
+stderr 'package command-line-arguments: invalid package directory .*uh-oh'
+
+! go run .
+stderr 'package example: invalid package directory .*uh-oh'
+
+! go run main.go
+stderr 'package command-line-arguments: invalid package directory .*uh-oh'
+
+! go test .
+stderr 'package example: invalid package directory .*uh-oh'
+
+! go test -v main.go main_test.go
+stderr 'package command-line-arguments: invalid package directory .*uh-oh'
+
+go list -compiled -e -f '{{with .CompiledGoFiles}}{{.}}{{end}}' .
+! stdout .
+! stderr .
+
+
+# Since we do preserve $PWD (or set it appropriately) for commands, and we do
+# not resolve symlinks unnecessarily, referring to the contents of the unsafe
+# directory via a safe symlink should be ok, and should not inject the data from
+# the symlink target path.
+
+[!symlink] stop 'remainder of test checks symlink behavior'
+[short] stop 'links and runs binaries'
+
+symlink $WORK${/}link -> $DIR
+
+go run $WORK${/}link${/}main.go
+! stdout panic
+! stderr panic
+stderr '^ok$'
+
+go test -v $WORK${/}link${/}main.go $WORK${/}link${/}main_test.go
+! stdout panic
+! stderr panic
+stdout '^ok$'   # 'go test' combines the test's stdout into stderr
+
+cd $WORK/link
+
+! go run $DIR${/}main.go
+stderr 'package command-line-arguments: invalid package directory .*uh-oh'
+
+go run .
+! stdout panic
+! stderr panic
+stderr '^ok$'
+
+go run main.go
+! stdout panic
+! stderr panic
+stderr '^ok$'
+
+go test -v
+! stdout panic
+! stderr panic
+stdout '^ok$'  # 'go test' combines the test's stdout into stderr
+
+go test -v .
+! stdout panic
+! stderr panic
+stdout '^ok$'  # 'go test' combines the test's stdout into stderr
+
+
+-- $WORK/go.mod --
+module example
+go 1.19
+-- $WORK/main.go --
+package main
+
+import "C"
+
+func main() {
+       /* nothing here */
+       println("ok")
+}
+-- $WORK/main_test.go --
+package main
+
+import "testing"
+
+func TestMain(*testing.M) {
+       main()
+}