From f47e163b52e1987771c9165616cfedda9ea35fee Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Sat, 13 May 2023 02:15:16 +0800 Subject: [PATCH 1/3] [Backport] cmd/go: disallow package directories containing newlines Offering: Cloud Core Network CVE: CVE-2023-29402 Reference: https://go-review.googlesource.com/c/go/+/501218 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 #60515. Fixes CVE-2023-29402. Change-Id: If55d0400c02beb7a5da5eceac60f1abeac99f064 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1882606 Reviewed-by: Roland Shoemaker Run-TryBot: Roland Shoemaker Reviewed-by: Russ Cox Reviewed-by: Damien Neil (cherry picked from commit 41f9046495564fc728d6f98384ab7276450ac7e2) Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1902229 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1904343 Reviewed-by: Michael Knyszek Reviewed-by: Bryan Mills Reviewed-on: https://go-review.googlesource.com/c/go/+/501218 Run-TryBot: David Chase Auto-Submit: Michael Knyszek TryBot-Result: Gopher Robot Signed-off-by: Tang Xi tangxi6@huawei.com --- src/cmd/go/internal/load/pkg.go | 4 + src/cmd/go/internal/work/exec.go | 6 ++ src/cmd/go/script_test.go | 1 + .../go/testdata/script/build_cwd_newline.txt | 100 ++++++++++++++++++ 4 files changed, 111 insertions(+) create mode 100644 src/cmd/go/testdata/script/build_cwd_newline.txt diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 2b5fbb1c5b..07795a4c70 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -1791,6 +1791,10 @@ func (p *Package) load(path string, stk *ImportStack, importPos []token.Position 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 eb1efd9f82..3745c688cb 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -457,6 +457,12 @@ func (b *Builder) build(a *Action) (err error) { b.Print(a.Package.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 a.Package.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 2e8f18a897..5c2a8d1409 100644 --- a/src/cmd/go/script_test.go +++ b/src/cmd/go/script_test.go @@ -140,6 +140,7 @@ func (ts *testScript) setup() { "devnull=" + os.DevNull, "goversion=" + goVersion(ts), ":=" + string(os.PathListSeparator), + "newline=\n", } if runtime.GOOS == "plan9" { 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..61c6966b02 --- /dev/null +++ b/src/cmd/go/testdata/script/build_cwd_newline.txt @@ -0,0 +1,100 @@ +[windows] skip 'filesystem normalizes / to \' +[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' + + +# 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() +} -- 2.33.0