cmd/go: accept only limited compiler and linker flags in #cgo directives

Both gcc and clang accept an option -fplugin=code.so to load
a plugin from the ELF shared object file code.so.
Obviously that plugin can then do anything it wants
during the build. This is contrary to the goal of "go get"
never running untrusted code during the build.
(What happens if you choose to run the result of
the build is your responsibility.)

Disallow this behavior by only allowing a small set of
known command-line flags in #cgo CFLAGS directives
(and #cgo LDFLAGS, etc).

The new restrictions can be adjusted by the environment
variables CGO_CFLAGS_ALLOW, CGO_CFLAGS_DISALLOW,
and so on. See the documentation.

In addition to excluding cgo-defined flags, we also have to
make sure that when we pass file names on the command
line, they don't look like flags. So we now refuse to build
packages containing suspicious file names like -x.go.

A wrinkle in all this is that GNU binutils uniformly accept
@foo on the command line to mean "if the file foo exists,
then substitute its contents for @foo in the command line".
So we must also reject @x.go, flags and flag arguments
beginning with @, and so on.

Fixes #23672, CVE-2018-6574.

Change-Id: I59e7c1355155c335a5c5ae0d2cf8fa7aa313940a
Reviewed-on: https://team-review.git.corp.google.com/209949
Reviewed-by: Ian Lance Taylor <iant@google.com>

misc/cgo/errors/src/err1.go

@@ -5,7 +5,7 @@
 package main
 
 /*
-#cgo LDFLAGS: -c
+#cgo LDFLAGS: -L/nonexist
 
 void test() {
 	xxx;		// ERROR HERE

src/cmd/cgo/doc.go

@@ -45,8 +45,8 @@ For example:
 	// #include <png.h>
 	import "C"
 
-Alternatively, CPPFLAGS and LDFLAGS may be obtained via the pkg-config
+Alternatively, CPPFLAGS and LDFLAGS may be obtained via the pkg-config tool
-tool using a '#cgo pkg-config:' directive followed by the package names.
+using a '#cgo pkg-config:' directive followed by the package names.
 For example:
 
 	// #cgo pkg-config: png cairo
@@ -55,11 +55,21 @@ For example:
 
 The default pkg-config tool may be changed by setting the PKG_CONFIG environment variable.
 
+For security reasons, only a limited set of flags are allowed, notably -D, -I, and -l.
+To allow additional flags, set CGO_CFLAGS_ALLOW to a regular expression
+matching the new flags. To disallow flags that would otherwise be allowed,
+set CGO_CFLAGS_DISALLOW to a regular expression matching arguments
+that must be disallowed. In both cases the regular expression must match
+a full argument: to allow -mfoo=bar, use CGO_CFLAGS_ALLOW='-mfoo.*',
+not just CGO_CFLAGS_ALLOW='-mfoo'. Similarly named variables control
+the allowed CPPFLAGS, CXXFLAGS, FFLAGS, and LDFLAGS.
+
 When building, the CGO_CFLAGS, CGO_CPPFLAGS, CGO_CXXFLAGS, CGO_FFLAGS and
 CGO_LDFLAGS environment variables are added to the flags derived from
 these directives. Package-specific flags should be set using the
 directives, not the environment variables, so that builds work in
-unmodified environments.
+unmodified environments. Flags obtained from environment variables
+are not subject to the security limitations described above.
 
 All the cgo CPPFLAGS and CFLAGS directives in a package are concatenated and
 used to compile C files in that package. All the CPPFLAGS and CXXFLAGS

src/cmd/compile/internal/gc/noder.go

@@ -7,6 +7,7 @@ package gc
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"runtime"
 	"strconv"
 	"strings"
@@ -1346,6 +1347,11 @@ func (p *noder) pragma(pos src.Pos, text string) syntax.Pragma {
 		p.linknames = append(p.linknames, linkname{pos, f[1], f[2]})
 
 	case strings.HasPrefix(text, "go:cgo_"):
+		// For security, we disallow //go:cgo_* directives outside cgo-generated files.
+		// Exception: they are allowed in the standard library, for runtime and syscall.
+		if !isCgoGeneratedFile(pos) && !compiling_std {
+			p.error(syntax.Error{Pos: pos, Msg: fmt.Sprintf("//%s only allowed in cgo-generated code", text)})
+		}
 		p.pragcgobuf += p.pragcgo(pos, text)
 		fallthrough // because of //go:cgo_unsafe_args
 	default:
@@ -1367,6 +1373,16 @@ func (p *noder) pragma(pos src.Pos, text string) syntax.Pragma {
 	return 0
 }
 
+// isCgoGeneratedFile reports whether pos is in a file
+// generated by cgo, which is to say a file with name
+// beginning with "_cgo_". Such files are allowed to
+// contain cgo directives, and for security reasons
+// (primarily misuse of linker flags), other files are not.
+// See golang.org/issue/23672.
+func isCgoGeneratedFile(pos src.Pos) bool {
+	return strings.HasPrefix(filepath.Base(filepath.Clean(pos.AbsFilename())), "_cgo_")
+}
+
 func mkname(sym *types.Sym) *Node {
 	n := oldname(sym)
 	if n.Name != nil && n.Name.Pack != nil {

src/cmd/dist/build.go

@@ -784,7 +784,7 @@ func runInstall(dir string, ch chan struct{}) {
 	} else {
 		archive = b
 	}
-	compile := []string{pathf("%s/compile", tooldir), "-pack", "-o", b, "-p", pkg}
+	compile := []string{pathf("%s/compile", tooldir), "-std", "-pack", "-o", b, "-p", pkg}
 	if gogcflags != "" {
 		compile = append(compile, strings.Fields(gogcflags)...)
 	}

src/cmd/go/alldocs.go

@@ -1227,17 +1227,26 @@
 // 	CGO_CFLAGS
 // 		Flags that cgo will pass to the compiler when compiling
 // 		C code.
-// 	CGO_CPPFLAGS
+// 	CGO_CFLAGS_ALLOW
-// 		Flags that cgo will pass to the compiler when compiling
+// 		A regular expression specifying additional flags to allow
-// 		C or C++ code.
+// 		to appear in #cgo CFLAGS source code directives.
-// 	CGO_CXXFLAGS
+// 		Does not apply to the CGO_CFLAGS environment variable.
-// 		Flags that cgo will pass to the compiler when compiling
+// 	CGO_CFLAGS_DISALLOW
-// 		C++ code.
+// 		A regular expression specifying flags that must be disallowed
-// 	CGO_FFLAGS
+// 		from appearing in #cgo CFLAGS source code directives.
-// 		Flags that cgo will pass to the compiler when compiling
+// 		Does not apply to the CGO_CFLAGS environment variable.
-// 		Fortran code.
+// 	CGO_CPPFLAGS, CGO_CPPFLAGS_ALLOW, CGO_CPPFLAGS_DISALLOW
-// 	CGO_LDFLAGS
+// 		Like CGO_CFLAGS, CGO_CFLAGS_ALLOW, and CGO_CFLAGS_DISALLOW,
-// 		Flags that cgo will pass to the compiler when linking.
+// 		but for the C preprocessor.
+// 	CGO_CXXFLAGS, CGO_CXXFLAGS_ALLOW, CGO_CXXFLAGS_DISALLOW
+// 		Like CGO_CFLAGS, CGO_CFLAGS_ALLOW, and CGO_CFLAGS_DISALLOW,
+// 		but for the C++ compiler.
+// 	CGO_FFLAGS, CGO_FFLAGS_ALLOW, CGO_FFLAGS_DISALLOW
+// 		Like CGO_CFLAGS, CGO_CFLAGS_ALLOW, and CGO_CFLAGS_DISALLOW,
+// 		but for the Fortran compiler.
+// 	CGO_LDFLAGS, CGO_LDFLAGS_ALLOW, CGO_LDFLAGS_DISALLOW
+// 		Like CGO_CFLAGS, CGO_CFLAGS_ALLOW, and CGO_CFLAGS_DISALLOW,
+// 		but for the linker.
 // 	CXX
 // 		The command to use to compile C++ code.
 // 	PKG_CONFIG

src/cmd/go/go_test.go

@@ -2789,7 +2789,7 @@ func TestCgoHandlesWlORIGIN(t *testing.T) {
 	defer tg.cleanup()
 	tg.parallel()
 	tg.tempFile("src/origin/origin.go", `package origin
-		// #cgo !darwin LDFLAGS: -Wl,-rpath -Wl,$ORIGIN
+		// #cgo !darwin LDFLAGS: -Wl,-rpath,$ORIGIN
 		// void f(void) {}
 		import "C"
 		func f() { C.f() }`)
@@ -5739,3 +5739,152 @@ func TestAtomicCoverpkgAll(t *testing.T) {
 		tg.run("test", "-coverpkg=all", "-race", "x")
 	}
 }
+
+func TestBadCommandLines(t *testing.T) {
+	tg := testgo(t)
+	defer tg.cleanup()
+
+	tg.tempFile("src/x/x.go", "package x\n")
+	tg.setenv("GOPATH", tg.path("."))
+
+	tg.run("build", "x")
+
+	tg.tempFile("src/x/@y.go", "package x\n")
+	tg.runFail("build", "x")
+	tg.grepStderr("invalid input file name \"@y.go\"", "did not reject @y.go")
+	tg.must(os.Remove(tg.path("src/x/@y.go")))
+
+	tg.tempFile("src/x/-y.go", "package x\n")
+	tg.runFail("build", "x")
+	tg.grepStderr("invalid input file name \"-y.go\"", "did not reject -y.go")
+	tg.must(os.Remove(tg.path("src/x/-y.go")))
+
+	tg.runFail("build", "-gcflags=all=@x", "x")
+	tg.grepStderr("invalid command-line argument @x in command", "did not reject @x during exec")
+
+	tg.tempFile("src/@x/x.go", "package x\n")
+	tg.setenv("GOPATH", tg.path("."))
+	tg.runFail("build", "@x")
+	tg.grepStderr("invalid input directory name \"@x\"", "did not reject @x directory")
+
+	tg.tempFile("src/@x/y/y.go", "package y\n")
+	tg.setenv("GOPATH", tg.path("."))
+	tg.runFail("build", "@x/y")
+	tg.grepStderr("invalid import path \"@x/y\"", "did not reject @x/y import path")
+
+	tg.tempFile("src/-x/x.go", "package x\n")
+	tg.setenv("GOPATH", tg.path("."))
+	tg.runFail("build", "--", "-x")
+	tg.grepStderr("invalid input directory name \"-x\"", "did not reject -x directory")
+
+	tg.tempFile("src/-x/y/y.go", "package y\n")
+	tg.setenv("GOPATH", tg.path("."))
+	tg.runFail("build", "--", "-x/y")
+	tg.grepStderr("invalid import path \"-x/y\"", "did not reject -x/y import path")
+}
+
+func TestBadCgoDirectives(t *testing.T) {
+	if !canCgo {
+		t.Skip("no cgo")
+	}
+	tg := testgo(t)
+	defer tg.cleanup()
+
+	tg.tempFile("src/x/x.go", "package x\n")
+	tg.setenv("GOPATH", tg.path("."))
+
+	tg.tempFile("src/x/x.go", `package x
+
+		//go:cgo_ldflag "-fplugin=foo.so"
+
+		import "C"
+	`)
+	tg.runFail("build", "x")
+	tg.grepStderr("//go:cgo_ldflag .* only allowed in cgo-generated code", "did not reject //go:cgo_ldflag directive")
+
+	tg.must(os.Remove(tg.path("src/x/x.go")))
+	tg.runFail("build", "x")
+	tg.grepStderr("no Go files", "did not report missing source code")
+	tg.tempFile("src/x/_cgo_yy.go", `package x
+
+		//go:cgo_ldflag "-fplugin=foo.so"
+
+		import "C"
+	`)
+	tg.runFail("build", "x")
+	tg.grepStderr("no Go files", "did not report missing source code") // _* files are ignored...
+
+	tg.runFail("build", tg.path("src/x/_cgo_yy.go")) // ... but if forced, the comment is rejected
+	// Actually, today there is a separate issue that _ files named
+	// on the command-line are ignored. Once that is fixed,
+	// we want to see the cgo_ldflag error.
+	tg.grepStderr("//go:cgo_ldflag only allowed in cgo-generated code|no Go files", "did not reject //go:cgo_ldflag directive")
+	tg.must(os.Remove(tg.path("src/x/_cgo_yy.go")))
+
+	tg.tempFile("src/x/x.go", "package x\n")
+	tg.tempFile("src/x/y.go", `package x
+		// #cgo CFLAGS: -fplugin=foo.so
+		import "C"
+	`)
+	tg.runFail("build", "x")
+	tg.grepStderr("invalid flag in #cgo CFLAGS: -fplugin=foo.so", "did not reject -fplugin")
+
+	tg.tempFile("src/x/y.go", `package x
+		// #cgo CFLAGS: -Ibar -fplugin=foo.so
+		import "C"
+	`)
+	tg.runFail("build", "x")
+	tg.grepStderr("invalid flag in #cgo CFLAGS: -fplugin=foo.so", "did not reject -fplugin")
+
+	tg.tempFile("src/x/y.go", `package x
+		// #cgo pkg-config: -foo
+		import "C"
+	`)
+	tg.runFail("build", "x")
+	tg.grepStderr("invalid pkg-config package name: -foo", "did not reject pkg-config: -foo")
+
+	tg.tempFile("src/x/y.go", `package x
+		// #cgo pkg-config: @foo
+		import "C"
+	`)
+	tg.runFail("build", "x")
+	tg.grepStderr("invalid pkg-config package name: @foo", "did not reject pkg-config: -foo")
+
+	tg.tempFile("src/x/y.go", `package x
+		// #cgo CFLAGS: @foo
+		import "C"
+	`)
+	tg.runFail("build", "x")
+	tg.grepStderr("invalid flag in #cgo CFLAGS: @foo", "did not reject @foo flag")
+
+	tg.tempFile("src/x/y.go", `package x
+		// #cgo CFLAGS: -D
+		import "C"
+	`)
+	tg.runFail("build", "x")
+	tg.grepStderr("invalid flag in #cgo CFLAGS: -D without argument", "did not reject trailing -I flag")
+
+	// Note that -I @foo is allowed because we rewrite it into -I /path/to/src/@foo
+	// before the check is applied. There's no such rewrite for -D.
+
+	tg.tempFile("src/x/y.go", `package x
+		// #cgo CFLAGS: -D @foo
+		import "C"
+	`)
+	tg.runFail("build", "x")
+	tg.grepStderr("invalid flag in #cgo CFLAGS: -D @foo", "did not reject -D @foo flag")
+
+	tg.tempFile("src/x/y.go", `package x
+		// #cgo CFLAGS: -D@foo
+		import "C"
+	`)
+	tg.runFail("build", "x")
+	tg.grepStderr("invalid flag in #cgo CFLAGS: -D@foo", "did not reject -D@foo flag")
+
+	tg.setenv("CGO_CFLAGS", "-D@foo")
+	tg.tempFile("src/x/y.go", `package x
+		import "C"
+	`)
+	tg.run("build", "-n", "x")
+	tg.grepStderr("-D@foo", "did not find -D@foo in commands")
+}

src/cmd/go/internal/envcmd/env.go

@@ -113,7 +113,12 @@ func findEnv(env []cfg.EnvVar, name string) string {
 func ExtraEnvVars() []cfg.EnvVar {
 	var b work.Builder
 	b.Init()
-	cppflags, cflags, cxxflags, fflags, ldflags := b.CFlags(&load.Package{})
+	cppflags, cflags, cxxflags, fflags, ldflags, err := b.CFlags(&load.Package{})
+	if err != nil {
+		// Should not happen - b.CFlags was given an empty package.
+		fmt.Fprintf(os.Stderr, "go: invalid cflags: %v\n", err)
+		return nil
+	}
 	cmd := b.GccCmd(".", "")
 	return []cfg.EnvVar{
 		// Note: Update the switch in runEnv below when adding to this list.

src/cmd/go/internal/help/helpdoc.go

@@ -487,17 +487,26 @@ Environment variables for use with cgo:
 	CGO_CFLAGS
 		Flags that cgo will pass to the compiler when compiling
 		C code.
-	CGO_CPPFLAGS
+	CGO_CFLAGS_ALLOW
-		Flags that cgo will pass to the compiler when compiling
+		A regular expression specifying additional flags to allow
-		C or C++ code.
+		to appear in #cgo CFLAGS source code directives.
-	CGO_CXXFLAGS
+		Does not apply to the CGO_CFLAGS environment variable.
-		Flags that cgo will pass to the compiler when compiling
+	CGO_CFLAGS_DISALLOW
-		C++ code.
+		A regular expression specifying flags that must be disallowed
-	CGO_FFLAGS
+		from appearing in #cgo CFLAGS source code directives.
-		Flags that cgo will pass to the compiler when compiling
+		Does not apply to the CGO_CFLAGS environment variable.
-		Fortran code.
+	CGO_CPPFLAGS, CGO_CPPFLAGS_ALLOW, CGO_CPPFLAGS_DISALLOW
-	CGO_LDFLAGS
+		Like CGO_CFLAGS, CGO_CFLAGS_ALLOW, and CGO_CFLAGS_DISALLOW,
-		Flags that cgo will pass to the compiler when linking.
+		but for the C preprocessor.
+	CGO_CXXFLAGS, CGO_CXXFLAGS_ALLOW, CGO_CXXFLAGS_DISALLOW
+		Like CGO_CFLAGS, CGO_CFLAGS_ALLOW, and CGO_CFLAGS_DISALLOW,
+		but for the C++ compiler.
+	CGO_FFLAGS, CGO_FFLAGS_ALLOW, CGO_FFLAGS_DISALLOW
+		Like CGO_CFLAGS, CGO_CFLAGS_ALLOW, and CGO_CFLAGS_DISALLOW,
+		but for the Fortran compiler.
+	CGO_LDFLAGS, CGO_LDFLAGS_ALLOW, CGO_LDFLAGS_DISALLOW
+		Like CGO_CFLAGS, CGO_CFLAGS_ALLOW, and CGO_CFLAGS_DISALLOW,
+		but for the linker.
 	CXX
 		The command to use to compile C++ code.
 	PKG_CONFIG

src/cmd/go/internal/load/pkg.go

@@ -16,6 +16,7 @@ import (
 	"sort"
 	"strings"
 	"unicode"
+	"unicode/utf8"
 
 	"cmd/go/internal/base"
 	"cmd/go/internal/cfg"
@@ -54,6 +55,8 @@ type PackagePublic struct {
 	StaleReason string `json:",omitempty"` // why is Stale true?
 
 	// Source files
+	// If you add to this list you MUST add to p.AllFiles (below) too.
+	// Otherwise file name security lists will not apply to any new additions.
 	GoFiles        []string `json:",omitempty"` // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles)
 	CgoFiles       []string `json:",omitempty"` // .go sources files that import "C"
 	IgnoredGoFiles []string `json:",omitempty"` // .go sources ignored due to build constraints
@@ -85,12 +88,38 @@ type PackagePublic struct {
 	DepsErrors []*PackageError `json:",omitempty"` // errors loading dependencies
 
 	// Test information
+	// If you add to this list you MUST add to p.AllFiles (below) too.
+	// Otherwise file name security lists will not apply to any new additions.
 	TestGoFiles  []string `json:",omitempty"` // _test.go files in package
 	TestImports  []string `json:",omitempty"` // imports from TestGoFiles
 	XTestGoFiles []string `json:",omitempty"` // _test.go files outside package
 	XTestImports []string `json:",omitempty"` // imports from XTestGoFiles
 }
 
+// AllFiles returns the names of all the files considered for the package.
+// This is used for sanity and security checks, so we include all files,
+// even IgnoredGoFiles, because some subcommands consider them.
+// The go/build package filtered others out (like foo_wrongGOARCH.s)
+// and that's OK.
+func (p *Package) AllFiles() []string {
+	return str.StringList(
+		p.GoFiles,
+		p.CgoFiles,
+		p.IgnoredGoFiles,
+		p.CFiles,
+		p.CXXFiles,
+		p.MFiles,
+		p.HFiles,
+		p.FFiles,
+		p.SFiles,
+		p.SwigFiles,
+		p.SwigCXXFiles,
+		p.SysoFiles,
+		p.TestGoFiles,
+		p.XTestGoFiles,
+	)
+}
+
 type PackageInternal struct {
 	// Unexported fields are not part of the public API.
 	Build        *build.Package
@@ -1005,22 +1034,8 @@ func (p *Package) load(stk *ImportStack, bp *build.Package, err error) {
 	// To avoid problems on case-insensitive files, we reject any package
 	// where two different input files have equal names under a case-insensitive
 	// comparison.
-	f1, f2 := str.FoldDup(str.StringList(
+	inputs := p.AllFiles()
-		p.GoFiles,
+	f1, f2 := str.FoldDup(inputs)
-		p.CgoFiles,
-		p.IgnoredGoFiles,
-		p.CFiles,
-		p.CXXFiles,
-		p.MFiles,
-		p.HFiles,
-		p.FFiles,
-		p.SFiles,
-		p.SysoFiles,
-		p.SwigFiles,
-		p.SwigCXXFiles,
-		p.TestGoFiles,
-		p.XTestGoFiles,
-	))
 	if f1 != "" {
 		p.Error = &PackageError{
 			ImportStack: stk.Copy(),
@@ -1029,6 +1044,37 @@ func (p *Package) load(stk *ImportStack, bp *build.Package, err error) {
 		return
 	}
 
+	// If first letter of input file is ASCII, it must be alphanumeric.
+	// This avoids files turning into flags when invoking commands,
+	// and other problems we haven't thought of yet.
+	// Also, _cgo_ files must be generated by us, not supplied.
+	// They are allowed to have //go:cgo_ldflag directives.
+	// The directory scan ignores files beginning with _,
+	// so we shouldn't see any _cgo_ files anyway, but just be safe.
+	for _, file := range inputs {
+		if !SafeArg(file) || strings.HasPrefix(file, "_cgo_") {
+			p.Error = &PackageError{
+				ImportStack: stk.Copy(),
+				Err:         fmt.Sprintf("invalid input file name %q", file),
+			}
+			return
+		}
+	}
+	if name := pathpkg.Base(p.ImportPath); !SafeArg(name) {
+		p.Error = &PackageError{
+			ImportStack: stk.Copy(),
+			Err:         fmt.Sprintf("invalid input directory name %q", name),
+		}
+		return
+	}
+	if !SafeArg(p.ImportPath) {
+		p.Error = &PackageError{
+			ImportStack: stk.Copy(),
+			Err:         fmt.Sprintf("invalid import path %q", p.ImportPath),
+		}
+		return
+	}
+
 	// Build list of imported packages and full dependency list.
 	imports := make([]*Package, 0, len(p.Imports))
 	for i, path := range importPaths {
@@ -1152,6 +1198,22 @@ func (p *Package) load(stk *ImportStack, bp *build.Package, err error) {
 	}
 }
 
+// SafeArg reports whether arg is a "safe" command-line argument,
+// meaning that when it appears in a command-line, it probably
+// doesn't have some special meaning other than its own name.
+// Obviously args beginning with - are not safe (they look like flags).
+// Less obviously, args beginning with @ are not safe (they look like
+// GNU binutils flagfile specifiers, sometimes called "response files").
+// To be conservative, we reject almost any arg beginning with non-alphanumeric ASCII.
+// We accept leading . _ and / as likely in file system paths.
+func SafeArg(name string) bool {
+	if name == "" {
+		return false
+	}
+	c := name[0]
+	return '0' <= c && c <= '9' || 'A' <= c && c <= 'Z' || 'a' <= c && c <= 'z' || c == '.' || c == '_' || c == '/' || c >= utf8.RuneSelf
+}
+
 // LinkerDeps returns the list of linker-induced dependencies for main package p.
 func LinkerDeps(p *Package) []string {
 	// Everything links runtime.

src/cmd/go/internal/work/exec.go

@@ -197,7 +197,7 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID {
 	fmt.Fprintf(h, "omitdebug %v standard %v local %v prefix %q\n", p.Internal.OmitDebug, p.Standard, p.Internal.Local, p.Internal.LocalPrefix)
 	if len(p.CgoFiles)+len(p.SwigFiles) > 0 {
 		fmt.Fprintf(h, "cgo %q\n", b.toolID("cgo"))
-		cppflags, cflags, cxxflags, fflags, _ := b.CFlags(p)
+		cppflags, cflags, cxxflags, fflags, _, _ := b.CFlags(p)
 		fmt.Fprintf(h, "CC=%q %q %q\n", b.ccExe(), cppflags, cflags)
 		if len(p.CXXFiles)+len(p.SwigFiles) > 0 {
 			fmt.Fprintf(h, "CXX=%q %q\n", b.cxxExe(), cxxflags)
@@ -521,7 +521,14 @@ func (b *Builder) build(a *Action) (err error) {
 	}
 
 	// Prepare Go import config.
+	// We start it off with a comment so it can't be empty, so icfg.Bytes() below is never nil.
+	// It should never be empty anyway, but there have been bugs in the past that resulted
+	// in empty configs, which then unfortunately turn into "no config passed to compiler",
+	// and the compiler falls back to looking in pkg itself, which mostly works,
+	// except when it doesn't.
 	var icfg bytes.Buffer
+	fmt.Fprintf(&icfg, "# import config\n")
+
 	for i, raw := range a.Package.Internal.RawImports {
 		final := a.Package.Imports[i]
 		if final != raw {
@@ -928,28 +935,38 @@ func splitPkgConfigOutput(out []byte) []string {
 // Calls pkg-config if needed and returns the cflags/ldflags needed to build the package.
 func (b *Builder) getPkgConfigFlags(p *load.Package) (cflags, ldflags []string, err error) {
 	if pkgs := p.CgoPkgConfig; len(pkgs) > 0 {
+		for _, pkg := range pkgs {
+			if !load.SafeArg(pkg) {
+				return nil, nil, fmt.Errorf("invalid pkg-config package name: %s", pkg)
+			}
+		}
 		var out []byte
-		out, err = b.runOut(p.Dir, p.ImportPath, nil, b.PkgconfigCmd(), "--cflags", pkgs)
+		out, err = b.runOut(p.Dir, p.ImportPath, nil, b.PkgconfigCmd(), "--cflags", "--", pkgs)
 		if err != nil {
 			b.showOutput(nil, p.Dir, b.PkgconfigCmd()+" --cflags "+strings.Join(pkgs, " "), string(out))
 			b.Print(err.Error() + "\n")
-			err = errPrintedOutput
+			return nil, nil, errPrintedOutput
-			return
 		}
 		if len(out) > 0 {
 			cflags = splitPkgConfigOutput(out)
+			if err := checkCompilerFlags("CFLAGS", "pkg-config --cflags", cflags); err != nil {
+				return nil, nil, err
+			}
 		}
-		out, err = b.runOut(p.Dir, p.ImportPath, nil, b.PkgconfigCmd(), "--libs", pkgs)
+		out, err = b.runOut(p.Dir, p.ImportPath, nil, b.PkgconfigCmd(), "--libs", "--", pkgs)
 		if err != nil {
 			b.showOutput(nil, p.Dir, b.PkgconfigCmd()+" --libs "+strings.Join(pkgs, " "), string(out))
 			b.Print(err.Error() + "\n")
-			err = errPrintedOutput
+			return nil, nil, errPrintedOutput
-			return
 		}
 		if len(out) > 0 {
 			ldflags = strings.Fields(string(out))
+			if err := checkLinkerFlags("CFLAGS", "pkg-config --cflags", ldflags); err != nil {
+				return nil, nil, err
+			}
 		}
 	}
+
 	return
 }
 
@@ -1447,6 +1464,17 @@ func (b *Builder) processOutput(out []byte) string {
 // It returns the command output and any errors that occurred.
 func (b *Builder) runOut(dir string, desc string, env []string, cmdargs ...interface{}) ([]byte, error) {
 	cmdline := str.StringList(cmdargs...)
+
+	for _, arg := range cmdline {
+		// GNU binutils commands, including gcc and gccgo, interpret an argument
+		// @foo anywhere in the command line (even following --) as meaning
+		// "read and insert arguments from the file named foo."
+		// Don't say anything that might be misinterpreted that way.
+		if strings.HasPrefix(arg, "@") {
+			return nil, fmt.Errorf("invalid command-line argument %s in command: %s", arg, joinUnambiguously(cmdline))
+		}
+	}
+
 	if cfg.BuildN || cfg.BuildX {
 		var envcmdline string
 		for _, e := range env {
@@ -1888,22 +1916,44 @@ func envList(key, def string) []string {
 }
 
 // CFlags returns the flags to use when invoking the C, C++ or Fortran compilers, or cgo.
-func (b *Builder) CFlags(p *load.Package) (cppflags, cflags, cxxflags, fflags, ldflags []string) {
+func (b *Builder) CFlags(p *load.Package) (cppflags, cflags, cxxflags, fflags, ldflags []string, err error) {
 	defaults := "-g -O2"
 
-	cppflags = str.StringList(envList("CGO_CPPFLAGS", ""), p.CgoCPPFLAGS)
+	if cppflags, err = buildFlags("CPPFLAGS", "", p.CgoCPPFLAGS, checkCompilerFlags); err != nil {
-	cflags = str.StringList(envList("CGO_CFLAGS", defaults), p.CgoCFLAGS)
+		return
-	cxxflags = str.StringList(envList("CGO_CXXFLAGS", defaults), p.CgoCXXFLAGS)
+	}
-	fflags = str.StringList(envList("CGO_FFLAGS", defaults), p.CgoFFLAGS)
+	if cflags, err = buildFlags("CFLAGS", defaults, p.CgoCFLAGS, checkCompilerFlags); err != nil {
-	ldflags = str.StringList(envList("CGO_LDFLAGS", defaults), p.CgoLDFLAGS)
+		return
+	}
+	if cxxflags, err = buildFlags("CXXFLAGS", defaults, p.CgoCXXFLAGS, checkCompilerFlags); err != nil {
+		return
+	}
+	if fflags, err = buildFlags("FFLAGS", defaults, p.CgoFFLAGS, checkCompilerFlags); err != nil {
+		return
+	}
+	if ldflags, err = buildFlags("LDFLAGS", defaults, p.CgoLDFLAGS, checkLinkerFlags); err != nil {
+		return
+	}
+
 	return
 }
 
+func buildFlags(name, defaults string, fromPackage []string, check func(string, string, []string) error) ([]string, error) {
+	if err := check(name, "#cgo "+name, fromPackage); err != nil {
+		return nil, err
+	}
+	return str.StringList(envList("CGO_"+name, defaults), fromPackage), nil
+}
+
 var cgoRe = regexp.MustCompile(`[/\\:]`)
 
 func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgofiles, gccfiles, gxxfiles, mfiles, ffiles []string) (outGo, outObj []string, err error) {
 	p := a.Package
-	cgoCPPFLAGS, cgoCFLAGS, cgoCXXFLAGS, cgoFFLAGS, cgoLDFLAGS := b.CFlags(p)
+	cgoCPPFLAGS, cgoCFLAGS, cgoCXXFLAGS, cgoFFLAGS, cgoLDFLAGS, err := b.CFlags(p)
+	if err != nil {
+		return nil, nil, err
+	}
+
 	cgoCPPFLAGS = append(cgoCPPFLAGS, pcCFLAGS...)
 	cgoLDFLAGS = append(cgoLDFLAGS, pcLDFLAGS...)
 	// If we are compiling Objective-C code, then we need to link against libobjc
@@ -1953,6 +2003,12 @@ func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgo
 	}
 
 	// Update $CGO_LDFLAGS with p.CgoLDFLAGS.
+	// These flags are recorded in the generated _cgo_gotypes.go file
+	// using //go:cgo_ldflag directives, the compiler records them in the
+	// object file for the package, and then the Go linker passes them
+	// along to the host linker. At this point in the code, cgoLDFLAGS
+	// consists of the original $CGO_LDFLAGS (unchecked) and all the
+	// flags put together from source code (checked).
 	var cgoenv []string
 	if len(cgoLDFLAGS) > 0 {
 		flags := make([]string, len(cgoLDFLAGS))
@@ -2245,7 +2301,11 @@ func (b *Builder) swigIntSize(objdir string) (intsize string, err error) {
 
 // Run SWIG on one SWIG input file.
 func (b *Builder) swigOne(a *Action, p *load.Package, file, objdir string, pcCFLAGS []string, cxx bool, intgosize string) (outGo, outC string, err error) {
-	cgoCPPFLAGS, cgoCFLAGS, cgoCXXFLAGS, _, _ := b.CFlags(p)
+	cgoCPPFLAGS, cgoCFLAGS, cgoCXXFLAGS, _, _, err := b.CFlags(p)
+	if err != nil {
+		return "", "", err
+	}
+
 	var cflags []string
 	if cxx {
 		cflags = str.StringList(cgoCPPFLAGS, pcCFLAGS, cgoCXXFLAGS)

src/cmd/go/internal/work/security.go

@@ -0,0 +1,160 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Checking of compiler and linker flags.
+// We must avoid flags like -fplugin=, which can allow
+// arbitrary code execution during the build.
+// Do not make changes here without carefully
+// considering the implications.
+// (That's why the code is isolated in a file named security.go.)
+//
+// Note that -Wl,foo means split foo on commas and pass to
+// the linker, so that -Wl,-foo,bar means pass -foo bar to
+// the linker. Similarly -Wa,foo for the assembler and so on.
+// If any of these are permitted, the wildcard portion must
+// disallow commas.
+//
+// Note also that GNU binutils accept any argument @foo
+// as meaning "read more flags from the file foo", so we must
+// guard against any command-line argument beginning with @,
+// even things like "-I @foo".
+// We use load.SafeArg (which is even more conservative)
+// to reject these.
+//
+// Even worse, gcc -I@foo (one arg) turns into cc1 -I @foo (two args),
+// so although gcc doesn't expand the @foo, cc1 will.
+// So out of paranoia, we reject @ at the beginning of every
+// flag argument that might be split into its own argument.
+
+package work
+
+import (
+	"cmd/go/internal/load"
+	"fmt"
+	"os"
+	"regexp"
+)
+
+var re = regexp.MustCompile
+
+var validCompilerFlags = []*regexp.Regexp{
+	re(`-D([A-Za-z_].*)`),
+	re(`-I([^@\-].*)`),
+	re(`-O`),
+	re(`-O([^@\-].*)`),
+	re(`-W`),
+	re(`-W([^@,]+)`), // -Wall but not -Wa,-foo.
+	re(`-f(no-)?objc-arc`),
+	re(`-f(no-)?omit-frame-pointer`),
+	re(`-f(no-)?(pic|PIC|pie|PIE)`),
+	re(`-f(no-)?split-stack`),
+	re(`-f(no-)?stack-(.+)`),
+	re(`-f(no-)?strict-aliasing`),
+	re(`-fsanitize=(.+)`),
+	re(`-g([^@\-].*)?`),
+	re(`-m(arch|cpu|fpu|tune)=([^@\-].*)`),
+	re(`-m(no-)?stack-(.+)`),
+	re(`-mmacosx-(.+)`),
+	re(`-mnop-fun-dllimport`),
+	re(`-pthread`),
+	re(`-std=([^@\-].*)`),
+	re(`-x([^@\-].*)`),
+}
+
+var validCompilerFlagsWithNextArg = []string{
+	"-D",
+	"-I",
+	"-framework",
+	"-x",
+}
+
+var validLinkerFlags = []*regexp.Regexp{
+	re(`-F([^@\-].*)`),
+	re(`-l([^@\-].*)`),
+	re(`-L([^@\-].*)`),
+	re(`-f(no-)?(pic|PIC|pie|PIE)`),
+	re(`-fsanitize=([^@\-].*)`),
+	re(`-g([^@\-].*)?`),
+	re(`-m(arch|cpu|fpu|tune)=([^@\-].*)`),
+	re(`-(pic|PIC|pie|PIE)`),
+	re(`-pthread`),
+
+	// Note that any wildcards in -Wl need to exclude comma,
+	// since -Wl splits its argument at commas and passes
+	// them all to the linker uninterpreted. Allowing comma
+	// in a wildcard would allow tunnelling arbitrary additional
+	// linker arguments through one of these.
+	re(`-Wl,-rpath,([^,@\-][^,]+)`),
+	re(`-Wl,--(no-)?warn-([^,]+)`),
+
+	re(`[a-zA-Z0-9_].*\.(o|obj|dll|dylib|so)`), // direct linker inputs: x.o or libfoo.so (but not -foo.o or @foo.o)
+}
+
+var validLinkerFlagsWithNextArg = []string{
+	"-F",
+	"-l",
+	"-L",
+	"-framework",
+}
+
+func checkCompilerFlags(name, source string, list []string) error {
+	return checkFlags(name, source, list, validCompilerFlags, validCompilerFlagsWithNextArg)
+}
+
+func checkLinkerFlags(name, source string, list []string) error {
+	return checkFlags(name, source, list, validLinkerFlags, validLinkerFlagsWithNextArg)
+}
+
+func checkFlags(name, source string, list []string, valid []*regexp.Regexp, validNext []string) error {
+	// Let users override rules with $CGO_CFLAGS_ALLOW, $CGO_CFLAGS_DISALLOW, etc.
+	var (
+		allow    *regexp.Regexp
+		disallow *regexp.Regexp
+	)
+	if env := os.Getenv("CGO_" + name + "_ALLOW"); env != "" {
+		r, err := regexp.Compile(env)
+		if err != nil {
+			return fmt.Errorf("parsing $CGO_%s_ALLOW: %v", name, err)
+		}
+		allow = r
+	}
+	if env := os.Getenv("CGO_" + name + "_DISALLOW"); env != "" {
+		r, err := regexp.Compile(env)
+		if err != nil {
+			return fmt.Errorf("parsing $CGO_%s_DISALLOW: %v", name, err)
+		}
+		disallow = r
+	}
+
+Args:
+	for i := 0; i < len(list); i++ {
+		arg := list[i]
+		if disallow != nil && disallow.FindString(arg) == arg {
+			goto Bad
+		}
+		if allow != nil && allow.FindString(arg) == arg {
+			continue Args
+		}
+		for _, re := range valid {
+			if re.FindString(arg) == arg { // must be complete match
+				continue Args
+			}
+		}
+		for _, x := range validNext {
+			if arg == x {
+				if i+1 < len(list) && load.SafeArg(list[i+1]) {
+					i++
+					continue Args
+				}
+				if i+1 < len(list) {
+					return fmt.Errorf("invalid flag in %s: %s %s", source, arg, list[i+1])
+				}
+				return fmt.Errorf("invalid flag in %s: %s without argument", source, arg)
+			}
+		}
+	Bad:
+		return fmt.Errorf("invalid flag in %s: %s", source, arg)
+	}
+	return nil
+}

src/cmd/go/internal/work/security_test.go

@@ -0,0 +1,240 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package work
+
+import (
+	"os"
+	"testing"
+)
+
+var goodCompilerFlags = [][]string{
+	{"-DFOO"},
+	{"-Dfoo=bar"},
+	{"-I/"},
+	{"-I/etc/passwd"},
+	{"-I."},
+	{"-O"},
+	{"-O2"},
+	{"-Osmall"},
+	{"-W"},
+	{"-Wall"},
+	{"-fobjc-arc"},
+	{"-fno-objc-arc"},
+	{"-fomit-frame-pointer"},
+	{"-fno-omit-frame-pointer"},
+	{"-fpic"},
+	{"-fno-pic"},
+	{"-fPIC"},
+	{"-fno-PIC"},
+	{"-fpie"},
+	{"-fno-pie"},
+	{"-fPIE"},
+	{"-fno-PIE"},
+	{"-fsplit-stack"},
+	{"-fno-split-stack"},
+	{"-fstack-xxx"},
+	{"-fno-stack-xxx"},
+	{"-fsanitize=hands"},
+	{"-g"},
+	{"-ggdb"},
+	{"-march=souza"},
+	{"-mcpu=123"},
+	{"-mfpu=123"},
+	{"-mtune=happybirthday"},
+	{"-mstack-overflow"},
+	{"-mno-stack-overflow"},
+	{"-mmacosx-version"},
+	{"-mnop-fun-dllimport"},
+	{"-pthread"},
+	{"-std=c99"},
+	{"-xc"},
+	{"-D", "FOO"},
+	{"-D", "foo=bar"},
+	{"-I", "."},
+	{"-I", "/etc/passwd"},
+	{"-I", "世界"},
+	{"-framework", "Chocolate"},
+	{"-x", "c"},
+}
+
+var badCompilerFlags = [][]string{
+	{"-D@X"},
+	{"-D-X"},
+	{"-I@dir"},
+	{"-I-dir"},
+	{"-O@1"},
+	{"-Wa,-foo"},
+	{"-W@foo"},
+	{"-g@gdb"},
+	{"-g-gdb"},
+	{"-march=@dawn"},
+	{"-march=-dawn"},
+	{"-std=@c99"},
+	{"-std=-c99"},
+	{"-x@c"},
+	{"-x-c"},
+	{"-D", "@foo"},
+	{"-D", "-foo"},
+	{"-I", "@foo"},
+	{"-I", "-foo"},
+	{"-framework", "-Caffeine"},
+	{"-framework", "@Home"},
+	{"-x", "--c"},
+	{"-x", "@obj"},
+}
+
+func TestCheckCompilerFlags(t *testing.T) {
+	for _, f := range goodCompilerFlags {
+		if err := checkCompilerFlags("test", "test", f); err != nil {
+			t.Errorf("unexpected error for %q: %v", f, err)
+		}
+	}
+	for _, f := range badCompilerFlags {
+		if err := checkCompilerFlags("test", "test", f); err == nil {
+			t.Errorf("missing error for %q", f)
+		}
+	}
+}
+
+var goodLinkerFlags = [][]string{
+	{"-Fbar"},
+	{"-lbar"},
+	{"-Lbar"},
+	{"-fpic"},
+	{"-fno-pic"},
+	{"-fPIC"},
+	{"-fno-PIC"},
+	{"-fpie"},
+	{"-fno-pie"},
+	{"-fPIE"},
+	{"-fno-PIE"},
+	{"-fsanitize=hands"},
+	{"-g"},
+	{"-ggdb"},
+	{"-march=souza"},
+	{"-mcpu=123"},
+	{"-mfpu=123"},
+	{"-mtune=happybirthday"},
+	{"-pic"},
+	{"-pthread"},
+	{"-Wl,-rpath,foo"},
+	{"-Wl,-rpath,$ORIGIN/foo"},
+	{"-Wl,--warn-error"},
+	{"-Wl,--no-warn-error"},
+	{"foo.so"},
+	{"_世界.dll"},
+	{"libcgosotest.dylib"},
+	{"-F", "framework"},
+	{"-l", "."},
+	{"-l", "/etc/passwd"},
+	{"-l", "世界"},
+	{"-L", "framework"},
+	{"-framework", "Chocolate"},
+}
+
+var badLinkerFlags = [][]string{
+	{"-DFOO"},
+	{"-Dfoo=bar"},
+	{"-O"},
+	{"-O2"},
+	{"-Osmall"},
+	{"-W"},
+	{"-Wall"},
+	{"-fobjc-arc"},
+	{"-fno-objc-arc"},
+	{"-fomit-frame-pointer"},
+	{"-fno-omit-frame-pointer"},
+	{"-fsplit-stack"},
+	{"-fno-split-stack"},
+	{"-fstack-xxx"},
+	{"-fno-stack-xxx"},
+	{"-mstack-overflow"},
+	{"-mno-stack-overflow"},
+	{"-mmacosx-version"},
+	{"-mnop-fun-dllimport"},
+	{"-std=c99"},
+	{"-xc"},
+	{"-D", "FOO"},
+	{"-D", "foo=bar"},
+	{"-I", "FOO"},
+	{"-L", "@foo"},
+	{"-L", "-foo"},
+	{"-x", "c"},
+	{"-D@X"},
+	{"-D-X"},
+	{"-I@dir"},
+	{"-I-dir"},
+	{"-O@1"},
+	{"-Wa,-foo"},
+	{"-W@foo"},
+	{"-g@gdb"},
+	{"-g-gdb"},
+	{"-march=@dawn"},
+	{"-march=-dawn"},
+	{"-std=@c99"},
+	{"-std=-c99"},
+	{"-x@c"},
+	{"-x-c"},
+	{"-D", "@foo"},
+	{"-D", "-foo"},
+	{"-I", "@foo"},
+	{"-I", "-foo"},
+	{"-l", "@foo"},
+	{"-l", "-foo"},
+	{"-framework", "-Caffeine"},
+	{"-framework", "@Home"},
+	{"-x", "--c"},
+	{"-x", "@obj"},
+	{"-Wl,-rpath,@foo"},
+}
+
+func TestCheckLinkerFlags(t *testing.T) {
+	for _, f := range goodLinkerFlags {
+		if err := checkLinkerFlags("test", "test", f); err != nil {
+			t.Errorf("unexpected error for %q: %v", f, err)
+		}
+	}
+	for _, f := range badLinkerFlags {
+		if err := checkLinkerFlags("test", "test", f); err == nil {
+			t.Errorf("missing error for %q", f)
+		}
+	}
+}
+
+func TestCheckFlagAllowDisallow(t *testing.T) {
+	if err := checkCompilerFlags("TEST", "test", []string{"-disallow"}); err == nil {
+		t.Fatalf("missing error for -disallow")
+	}
+	os.Setenv("CGO_TEST_ALLOW", "-disallo")
+	if err := checkCompilerFlags("TEST", "test", []string{"-disallow"}); err == nil {
+		t.Fatalf("missing error for -disallow with CGO_TEST_ALLOW=-disallo")
+	}
+	os.Setenv("CGO_TEST_ALLOW", "-disallow")
+	if err := checkCompilerFlags("TEST", "test", []string{"-disallow"}); err != nil {
+		t.Fatalf("unexpected error for -disallow with CGO_TEST_ALLOW=-disallow: %v", err)
+	}
+	os.Unsetenv("CGO_TEST_ALLOW")
+
+	if err := checkCompilerFlags("TEST", "test", []string{"-Wall"}); err != nil {
+		t.Fatalf("unexpected error for -Wall: %v", err)
+	}
+	os.Setenv("CGO_TEST_DISALLOW", "-Wall")
+	if err := checkCompilerFlags("TEST", "test", []string{"-Wall"}); err == nil {
+		t.Fatalf("missing error for -Wall with CGO_TEST_DISALLOW=-Wall")
+	}
+	os.Setenv("CGO_TEST_ALLOW", "-Wall") // disallow wins
+	if err := checkCompilerFlags("TEST", "test", []string{"-Wall"}); err == nil {
+		t.Fatalf("missing error for -Wall with CGO_TEST_DISALLOW=-Wall and CGO_TEST_ALLOW=-Wall")
+	}
+
+	os.Setenv("CGO_TEST_ALLOW", "-fplugin.*")
+	os.Setenv("CGO_TEST_DISALLOW", "-fplugin=lint.so")
+	if err := checkCompilerFlags("TEST", "test", []string{"-fplugin=faster.so"}); err != nil {
+		t.Fatalf("unexpected error for -fplugin=faster.so: %v", err)
+	}
+	if err := checkCompilerFlags("TEST", "test", []string{"-fplugin=lint.so"}); err == nil {
+		t.Fatalf("missing error for -fplugin=lint.so: %v", err)
+	}
+}