aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBryan C. Mills <bcmills@google.com>2023-05-12 14:15:16 -0400
committerDavid Chase <drchase@google.com>2023-06-06 20:24:31 +0000
commit4dae3bbe0e6a5700037bb996ae84d6f457c4f58a (patch)
treeb634f59329fc656b15bbd22560a1b37f952a3d52
parentbbeb55f5faf93659e1cfd6ab073ab3c9d126d195 (diff)
downloadgo-4dae3bbe0e6a5700037bb996ae84d6f457c4f58a.tar.gz
go-4dae3bbe0e6a5700037bb996ae84d6f457c4f58a.zip
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. Fixes #60167. Fixes CVE-2023-29402. Change-Id: I64572e9f454bce7b685d00e2e6a1c96cd33d53df 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> Reviewed-on: https://go-review.googlesource.com/c/go/+/501226 Run-TryBot: David Chase <drchase@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
-rw-r--r--src/cmd/go/internal/load/pkg.go4
-rw-r--r--src/cmd/go/internal/work/exec.go6
-rw-r--r--src/cmd/go/script_test.go1
-rw-r--r--src/cmd/go/testdata/script/build_cwd_newline.txt104
4 files changed, 115 insertions, 0 deletions
diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go
index 65af063059..a4b523f2c9 100644
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -1995,6 +1995,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 a570e755eb..eb05c91f30 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -514,6 +514,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 f7829cb918..a2c1087bd8 100644
--- a/src/cmd/go/script_test.go
+++ b/src/cmd/go/script_test.go
@@ -246,6 +246,7 @@ func scriptEnv(srv *vcstest.Server, srvCertFile string) ([]string, error) {
"goversion=" + version,
"CMDGO_TEST_RUN_MAIN=true",
"HGRCPATH=",
+ "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
index 0000000000..574464cc18
--- /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()
+}