[release-branch.go1.10] all: merge master into release-branch.go1.10, for go1.10rc2
This adds the go get security fix.
* 1dcb5836 cmd/go: accept only limited compiler and linker flags in #cgo directives
Change-Id: Ib2caf2039d2cefabe3afa0bb4dcc4c0dc8d664ff
diff --git a/misc/cgo/errors/src/err1.go b/misc/cgo/errors/src/err1.go
index 61bbcd2..2c232cf 100644
--- a/misc/cgo/errors/src/err1.go
+++ b/misc/cgo/errors/src/err1.go
@@ -5,7 +5,7 @@
package main
/*
-#cgo LDFLAGS: -c
+#cgo LDFLAGS: -L/nonexist
void test() {
xxx; // ERROR HERE
diff --git a/src/cmd/cgo/doc.go b/src/cmd/cgo/doc.go
index 8e4cd88..c16b63a 100644
--- a/src/cmd/cgo/doc.go
+++ b/src/cmd/cgo/doc.go
@@ -45,8 +45,8 @@
// #include <png.h>
import "C"
-Alternatively, CPPFLAGS and LDFLAGS may be obtained via the pkg-config
-tool using a '#cgo pkg-config:' directive followed by the package names.
+Alternatively, CPPFLAGS and LDFLAGS may be obtained via the pkg-config tool
+using a '#cgo pkg-config:' directive followed by the package names.
For example:
// #cgo pkg-config: png cairo
@@ -55,11 +55,21 @@
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
diff --git a/src/cmd/compile/internal/gc/noder.go b/src/cmd/compile/internal/gc/noder.go
index dcd5f20..fff04bc 100644
--- a/src/cmd/compile/internal/gc/noder.go
+++ b/src/cmd/compile/internal/gc/noder.go
@@ -7,6 +7,7 @@
import (
"fmt"
"os"
+ "path/filepath"
"runtime"
"strconv"
"strings"
@@ -1346,6 +1347,11 @@
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 @@
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 {
diff --git a/src/cmd/dist/build.go b/src/cmd/dist/build.go
index e80d466..49ed800 100644
--- a/src/cmd/dist/build.go
+++ b/src/cmd/dist/build.go
@@ -784,7 +784,7 @@
} 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)...)
}
diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
index 91657eb..7557647 100644
--- a/src/cmd/go/alldocs.go
+++ b/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
-// 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
diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index 92600b6..3972d79 100644
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -2789,7 +2789,7 @@
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 @@
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")
+}
diff --git a/src/cmd/go/internal/envcmd/env.go b/src/cmd/go/internal/envcmd/env.go
index fa19beb..603f7b5 100644
--- a/src/cmd/go/internal/envcmd/env.go
+++ b/src/cmd/go/internal/envcmd/env.go
@@ -113,7 +113,12 @@
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.
diff --git a/src/cmd/go/internal/help/helpdoc.go b/src/cmd/go/internal/help/helpdoc.go
index 4ebf206..c39af79 100644
--- a/src/cmd/go/internal/help/helpdoc.go
+++ b/src/cmd/go/internal/help/helpdoc.go
@@ -487,17 +487,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
diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go
index eaff717..bbd75bc 100644
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -16,6 +16,7 @@
"sort"
"strings"
"unicode"
+ "unicode/utf8"
"cmd/go/internal/base"
"cmd/go/internal/cfg"
@@ -54,6 +55,8 @@
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 @@
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 @@
// 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(
- p.GoFiles,
- 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,
- ))
+ inputs := p.AllFiles()
+ f1, f2 := str.FoldDup(inputs)
if f1 != "" {
p.Error = &PackageError{
ImportStack: stk.Copy(),
@@ -1029,6 +1044,37 @@
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 @@
}
}
+// 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.
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 58bab5c..a5ab75f 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -197,7 +197,7 @@
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 @@
}
// 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 @@
// 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
+ 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(nil, 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
}
@@ -1447,6 +1464,17 @@
// 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 @@
}
// 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)
- cflags = str.StringList(envList("CGO_CFLAGS", defaults), p.CgoCFLAGS)
- cxxflags = str.StringList(envList("CGO_CXXFLAGS", defaults), p.CgoCXXFLAGS)
- fflags = str.StringList(envList("CGO_FFLAGS", defaults), p.CgoFFLAGS)
- ldflags = str.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 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 @@
}
// 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 @@
// 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)
diff --git a/src/cmd/go/internal/work/security.go b/src/cmd/go/internal/work/security.go
new file mode 100644
index 0000000..fee5bee
--- /dev/null
+++ b/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
+}
diff --git a/src/cmd/go/internal/work/security_test.go b/src/cmd/go/internal/work/security_test.go
new file mode 100644
index 0000000..739ab5a
--- /dev/null
+++ b/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)
+ }
+}
