[release-branch.go1.8] cmd/go: accept only limited compiler and linke…

…r 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 #23674, CVE-2018-6574.

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

misc/cgo/errors/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

@@ -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/go.go

@@ -248,6 +248,7 @@ var nblank *Node
 var typecheckok bool
 
 var compiling_runtime bool
+var compiling_std bool
 
 var compiling_wrappers int
 

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

@@ -154,6 +154,7 @@ func Main() {
 	}
 
 	flag.BoolVar(&compiling_runtime, "+", false, "compiling runtime")
+	flag.BoolVar(&compiling_std, "std", false, "compiling standard library")
 	obj.Flagcount("%", "debug non-static initializers", &Debug['%'])
 	obj.Flagcount("B", "disable bounds checking", &Debug['B'])
 	flag.StringVar(&localimport, "D", "", "set relative `path` for local imports")

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

@@ -7,6 +7,7 @@ package gc
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"unicode/utf8"
@@ -1057,6 +1058,11 @@ func (p *noder) pragma(pos, line int, text string) syntax.Pragma {
 
 	case strings.HasPrefix(text, "go:cgo_"):
 		lineno = p.baseline + int32(line) - 1 // pragcgo may call yyerror
+		// 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() && !compiling_std {
+			p.error(syntax.Error{Pos: pos, Line: line, Msg: fmt.Sprintf("//%s only allowed in cgo-generated code", text)})
+		}
 		pragcgobuf += pragcgo(text)
 		fallthrough // because of //go:cgo_unsafe_args
 	default:
@@ -1071,6 +1077,20 @@ func (p *noder) pragma(pos, line int, text string) syntax.Pragma {
 	return 0
 }
 
+// isCgoGeneratedFile reports whether lineno 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() bool {
+	stk := Ctxt.LineHist.At(int(lineno))
+	if stk == nil {
+		return false
+	}
+	return strings.HasPrefix(filepath.Base(filepath.Clean(stk.File)), "_cgo_")
+}
+
 func mkname(sym *Sym) *Node {
 	n := oldname(sym)
 	if n.Name != nil && n.Name.Pack != nil {

src/cmd/dist/build.go

@@ -701,7 +701,7 @@ func install(dir string) {
 	} 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

@@ -1103,17 +1103,26 @@
 // 	CGO_CFLAGS
 // 		Flags that cgo will pass to the compiler when compiling
 // 		C code.
-// 	CGO_CPPFLAGS
-// 		Flags that cgo will pass to the compiler when compiling
-// 		C or C++ code.
-// 	CGO_CXXFLAGS
-// 		Flags that cgo will pass to the compiler when compiling
-// 		C++ code.
-// 	CGO_FFLAGS
-// 		Flags that cgo will pass to the compiler when compiling
-// 		Fortran code.
-// 	CGO_LDFLAGS
-// 		Flags that cgo will pass to the compiler when linking.
+// 	CGO_CFLAGS_ALLOW
+// 		A regular expression specifying additional flags to allow
+// 		to appear in #cgo CFLAGS source code directives.
+// 		Does not apply to the CGO_CFLAGS environment variable.
+// 	CGO_CFLAGS_DISALLOW
+// 		A regular expression specifying flags that must be disallowed
+// 		from appearing in #cgo CFLAGS source code directives.
+// 		Does not apply to the CGO_CFLAGS environment variable.
+// 	CGO_CPPFLAGS, CGO_CPPFLAGS_ALLOW, CGO_CPPFLAGS_DISALLOW
+// 		Like CGO_CFLAGS, CGO_CFLAGS_ALLOW, and CGO_CFLAGS_DISALLOW,
+// 		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/build.go

@@ -1670,26 +1670,35 @@ 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 *Package) (cflags, ldflags []string, err error) {
 	if pkgs := p.CgoPkgConfig; len(pkgs) > 0 {
+		for _, pkg := range pkgs {
+			if !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(p.Dir, b.pkgconfigCmd()+" --cflags "+strings.Join(pkgs, " "), string(out))
 			b.print(err.Error() + "\n")
-			err = errPrintedOutput
-			return
+			return nil, nil, errPrintedOutput
 		}
 		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(p.Dir, b.pkgconfigCmd()+" --libs "+strings.Join(pkgs, " "), string(out))
 			b.print(err.Error() + "\n")
-			err = errPrintedOutput
-			return
+			return nil, nil, errPrintedOutput
 		}
 		if len(out) > 0 {
 			ldflags = strings.Fields(string(out))
+			if err := checkLinkerFlags("CFLAGS", "pkg-config --cflags", ldflags); err != nil {
+				return nil, nil, err
+			}
 		}
 	}
 	return
@@ -2117,6 +2126,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 := 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 buildN || buildX {
 		var envcmdline string
 		for i := range env {
@@ -2357,6 +2377,9 @@ func (gcToolchain) gc(b *builder, p *Package, archive, obj string, asmhdr bool,
 		// additional reflect type data.
 		gcargs = append(gcargs, "-+")
 	}
+	if p.Standard {
+		gcargs = append(gcargs, "-std")
+	}
 
 	// If we're giving the compiler the entire package (no C etc files), tell it that,
 	// so that it can give good error messages about forward declarations.
@@ -3255,23 +3278,45 @@ func envList(key, def string) []string {
 	return strings.Fields(v)
 }
 
-// Return the flags to use when invoking the C, C++ or Fortran compilers, or cgo.
-func (b *builder) cflags(p *Package) (cppflags, cflags, cxxflags, fflags, ldflags []string) {
+// CFlags returns the flags to use when invoking the C, C++ or Fortran compilers, or cgo.
+func (b *builder) cflags(p *Package) (cppflags, cflags, cxxflags, fflags, ldflags []string, err error) {
 	defaults := "-g -O2"
 
-	cppflags = stringList(envList("CGO_CPPFLAGS", ""), p.CgoCPPFLAGS)
-	cflags = stringList(envList("CGO_CFLAGS", defaults), p.CgoCFLAGS)
-	cxxflags = stringList(envList("CGO_CXXFLAGS", defaults), p.CgoCXXFLAGS)
-	fflags = stringList(envList("CGO_FFLAGS", defaults), p.CgoFFLAGS)
-	ldflags = stringList(envList("CGO_LDFLAGS", defaults), p.CgoLDFLAGS)
+	if cppflags, err = buildFlags("CPPFLAGS", "", p.CgoCPPFLAGS, checkCompilerFlags); err != nil {
+		return
+	}
+	if cflags, err = buildFlags("CFLAGS", defaults, p.CgoCFLAGS, checkCompilerFlags); err != nil {
+		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 stringList(envList("CGO_"+name, defaults), fromPackage), nil
+}
+
 var cgoRe = regexp.MustCompile(`[/\\:]`)
 
 func (b *builder) cgo(a *action, cgoExe, obj string, pcCFLAGS, pcLDFLAGS, cgofiles, objdirCgofiles, gccfiles, gxxfiles, mfiles, ffiles []string) (outGo, outObj []string, err error) {
 	p := a.p
-	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
@@ -3335,6 +3380,12 @@ func (b *builder) cgo(a *action, cgoExe, obj string, pcCFLAGS, pcLDFLAGS, cgofil
 	}
 
 	// 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))
@@ -3684,7 +3735,11 @@ func (b *builder) swigIntSize(obj string) (intsize string, err error) {
 
 // Run SWIG on one SWIG input file.
 func (b *builder) swigOne(p *Package, file, obj 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 = stringList(cgoCPPFLAGS, pcCFLAGS, cgoCXXFLAGS)

src/cmd/go/env.go

@@ -90,7 +90,12 @@ func findEnv(env []envVar, name string) string {
 func extraEnvVars() []envVar {
 	var b builder
 	b.init()
-	cppflags, cflags, cxxflags, fflags, ldflags := b.cflags(&Package{})
+	cppflags, cflags, cxxflags, fflags, ldflags, err := b.cflags(&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
+	}
 	return []envVar{
 		{"PKG_CONFIG", b.pkgconfigCmd()},
 		{"CGO_CFLAGS", strings.Join(cflags, " ")},