aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKatie Hockman <katie@golang.org>2020-11-12 15:49:20 -0500
committerKatie Hockman <katie@golang.org>2020-11-12 15:49:20 -0500
commitd8c8fd6c305cf34a569e50a8d1f69aa123f9b32f (patch)
treec11cd9a7baa355241e0948ec3254fbaf2e05bf3d
parent1f2e58d89b754c28ba7448ead7d8d9ae8957ab49 (diff)
parentc53315d6cf1b4bfea6ff356b4a1524778c683bb9 (diff)
downloadgo-d8c8fd6c305cf34a569e50a8d1f69aa123f9b32f.tar.gz
go-d8c8fd6c305cf34a569e50a8d1f69aa123f9b32f.zip
[release-branch.go1.15] all: merge release-branch.go1.15-security into release-branch.go1.15
Change-Id: I5690e7f4f7f04b9df1881fa60f3d3c6841cefe40
-rw-r--r--VERSION2
-rw-r--r--misc/cgo/errors/badsym_test.go216
-rw-r--r--src/cmd/cgo/out.go23
-rw-r--r--src/cmd/go/internal/work/exec.go60
-rw-r--r--src/cmd/go/internal/work/security.go8
-rw-r--r--src/cmd/go/internal/work/security_test.go5
-rw-r--r--src/math/big/nat.go2
7 files changed, 310 insertions, 6 deletions
diff --git a/VERSION b/VERSION
index b0378bdff8d..d4bbebb9fca 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-go1.15.4 \ No newline at end of file
+go1.15.5 \ No newline at end of file
diff --git a/misc/cgo/errors/badsym_test.go b/misc/cgo/errors/badsym_test.go
new file mode 100644
index 00000000000..b2701bf922e
--- /dev/null
+++ b/misc/cgo/errors/badsym_test.go
@@ -0,0 +1,216 @@
+// Copyright 2020 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 errorstest
+
+import (
+ "bytes"
+ "io/ioutil"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "strings"
+ "testing"
+ "unicode"
+)
+
+// A manually modified object file could pass unexpected characters
+// into the files generated by cgo.
+
+const magicInput = "abcdefghijklmnopqrstuvwxyz0123"
+const magicReplace = "\n//go:cgo_ldflag \"-badflag\"\n//"
+
+const cSymbol = "BadSymbol" + magicInput + "Name"
+const cDefSource = "int " + cSymbol + " = 1;"
+const cRefSource = "extern int " + cSymbol + "; int F() { return " + cSymbol + "; }"
+
+// goSource is the source code for the trivial Go file we use.
+// We will replace TMPDIR with the temporary directory name.
+const goSource = `
+package main
+
+// #cgo LDFLAGS: TMPDIR/cbad.o TMPDIR/cbad.so
+// extern int F();
+import "C"
+
+func main() {
+ println(C.F())
+}
+`
+
+func TestBadSymbol(t *testing.T) {
+ dir := t.TempDir()
+
+ mkdir := func(base string) string {
+ ret := filepath.Join(dir, base)
+ if err := os.Mkdir(ret, 0755); err != nil {
+ t.Fatal(err)
+ }
+ return ret
+ }
+
+ cdir := mkdir("c")
+ godir := mkdir("go")
+
+ makeFile := func(mdir, base, source string) string {
+ ret := filepath.Join(mdir, base)
+ if err := ioutil.WriteFile(ret, []byte(source), 0644); err != nil {
+ t.Fatal(err)
+ }
+ return ret
+ }
+
+ cDefFile := makeFile(cdir, "cdef.c", cDefSource)
+ cRefFile := makeFile(cdir, "cref.c", cRefSource)
+
+ ccCmd := cCompilerCmd(t)
+
+ cCompile := func(arg, base, src string) string {
+ out := filepath.Join(cdir, base)
+ run := append(ccCmd, arg, "-o", out, src)
+ output, err := exec.Command(run[0], run[1:]...).CombinedOutput()
+ if err != nil {
+ t.Log(run)
+ t.Logf("%s", output)
+ t.Fatal(err)
+ }
+ if err := os.Remove(src); err != nil {
+ t.Fatal(err)
+ }
+ return out
+ }
+
+ // Build a shared library that defines a symbol whose name
+ // contains magicInput.
+
+ cShared := cCompile("-shared", "c.so", cDefFile)
+
+ // Build an object file that refers to the symbol whose name
+ // contains magicInput.
+
+ cObj := cCompile("-c", "c.o", cRefFile)
+
+ // Rewrite the shared library and the object file, replacing
+ // magicInput with magicReplace. This will have the effect of
+ // introducing a symbol whose name looks like a cgo command.
+ // The cgo tool will use that name when it generates the
+ // _cgo_import.go file, thus smuggling a magic //go:cgo_ldflag
+ // pragma into a Go file. We used to not check the pragmas in
+ // _cgo_import.go.
+
+ rewrite := func(from, to string) {
+ obj, err := ioutil.ReadFile(from)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ if bytes.Count(obj, []byte(magicInput)) == 0 {
+ t.Fatalf("%s: did not find magic string", from)
+ }
+
+ if len(magicInput) != len(magicReplace) {
+ t.Fatalf("internal test error: different magic lengths: %d != %d", len(magicInput), len(magicReplace))
+ }
+
+ obj = bytes.ReplaceAll(obj, []byte(magicInput), []byte(magicReplace))
+
+ if err := ioutil.WriteFile(to, obj, 0644); err != nil {
+ t.Fatal(err)
+ }
+ }
+
+ cBadShared := filepath.Join(godir, "cbad.so")
+ rewrite(cShared, cBadShared)
+
+ cBadObj := filepath.Join(godir, "cbad.o")
+ rewrite(cObj, cBadObj)
+
+ goSourceBadObject := strings.ReplaceAll(goSource, "TMPDIR", godir)
+ makeFile(godir, "go.go", goSourceBadObject)
+
+ makeFile(godir, "go.mod", "module badsym")
+
+ // Try to build our little package.
+ cmd := exec.Command("go", "build", "-ldflags=-v")
+ cmd.Dir = godir
+ output, err := cmd.CombinedOutput()
+
+ // The build should fail, but we want it to fail because we
+ // detected the error, not because we passed a bad flag to the
+ // C linker.
+
+ if err == nil {
+ t.Errorf("go build succeeded unexpectedly")
+ }
+
+ t.Logf("%s", output)
+
+ for _, line := range bytes.Split(output, []byte("\n")) {
+ if bytes.Contains(line, []byte("dynamic symbol")) && bytes.Contains(line, []byte("contains unsupported character")) {
+ // This is the error from cgo.
+ continue
+ }
+
+ // We passed -ldflags=-v to see the external linker invocation,
+ // which should not include -badflag.
+ if bytes.Contains(line, []byte("-badflag")) {
+ t.Error("output should not mention -badflag")
+ }
+
+ // Also check for compiler errors, just in case.
+ // GCC says "unrecognized command line option".
+ // clang says "unknown argument".
+ if bytes.Contains(line, []byte("unrecognized")) || bytes.Contains(output, []byte("unknown")) {
+ t.Error("problem should have been caught before invoking C linker")
+ }
+ }
+}
+
+func cCompilerCmd(t *testing.T) []string {
+ cc := []string{goEnv(t, "CC")}
+
+ out := goEnv(t, "GOGCCFLAGS")
+ quote := '\000'
+ start := 0
+ lastSpace := true
+ backslash := false
+ s := string(out)
+ for i, c := range s {
+ if quote == '\000' && unicode.IsSpace(c) {
+ if !lastSpace {
+ cc = append(cc, s[start:i])
+ lastSpace = true
+ }
+ } else {
+ if lastSpace {
+ start = i
+ lastSpace = false
+ }
+ if quote == '\000' && !backslash && (c == '"' || c == '\'') {
+ quote = c
+ backslash = false
+ } else if !backslash && quote == c {
+ quote = '\000'
+ } else if (quote == '\000' || quote == '"') && !backslash && c == '\\' {
+ backslash = true
+ } else {
+ backslash = false
+ }
+ }
+ }
+ if !lastSpace {
+ cc = append(cc, s[start:])
+ }
+ return cc
+}
+
+func goEnv(t *testing.T, key string) string {
+ out, err := exec.Command("go", "env", key).CombinedOutput()
+ if err != nil {
+ t.Logf("go env %s\n", key)
+ t.Logf("%s", out)
+ t.Fatal(err)
+ }
+ return strings.TrimSpace(string(out))
+}
diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go
index cbdb4e0f5b2..dcd69edb44b 100644
--- a/src/cmd/cgo/out.go
+++ b/src/cmd/cgo/out.go
@@ -336,6 +336,8 @@ func dynimport(obj string) {
if s.Version != "" {
targ += "#" + s.Version
}
+ checkImportSymName(s.Name)
+ checkImportSymName(targ)
fmt.Fprintf(stdout, "//go:cgo_import_dynamic %s %s %q\n", s.Name, targ, s.Library)
}
lib, _ := f.ImportedLibraries()
@@ -351,6 +353,7 @@ func dynimport(obj string) {
if len(s) > 0 && s[0] == '_' {
s = s[1:]
}
+ checkImportSymName(s)
fmt.Fprintf(stdout, "//go:cgo_import_dynamic %s %s %q\n", s, s, "")
}
lib, _ := f.ImportedLibraries()
@@ -365,6 +368,8 @@ func dynimport(obj string) {
for _, s := range sym {
ss := strings.Split(s, ":")
name := strings.Split(ss[0], "@")[0]
+ checkImportSymName(name)
+ checkImportSymName(ss[0])
fmt.Fprintf(stdout, "//go:cgo_import_dynamic %s %s %q\n", name, ss[0], strings.ToLower(ss[1]))
}
return
@@ -382,6 +387,7 @@ func dynimport(obj string) {
// Go symbols.
continue
}
+ checkImportSymName(s.Name)
fmt.Fprintf(stdout, "//go:cgo_import_dynamic %s %s %q\n", s.Name, s.Name, s.Library)
}
lib, err := f.ImportedLibraries()
@@ -397,6 +403,23 @@ func dynimport(obj string) {
fatalf("cannot parse %s as ELF, Mach-O, PE or XCOFF", obj)
}
+// checkImportSymName checks a symbol name we are going to emit as part
+// of a //go:cgo_import_dynamic pragma. These names come from object
+// files, so they may be corrupt. We are going to emit them unquoted,
+// so while they don't need to be valid symbol names (and in some cases,
+// involving symbol versions, they won't be) they must contain only
+// graphic characters and must not contain Go comments.
+func checkImportSymName(s string) {
+ for _, c := range s {
+ if !unicode.IsGraphic(c) || unicode.IsSpace(c) {
+ fatalf("dynamic symbol %q contains unsupported character", s)
+ }
+ }
+ if strings.Index(s, "//") >= 0 || strings.Index(s, "/*") >= 0 {
+ fatalf("dynamic symbol %q contains Go comment")
+ }
+}
+
// Construct a gcc struct matching the gc argument frame.
// Assumes that in gcc, char is 1 byte, short 2 bytes, int 4 bytes, long long 8 bytes.
// These assumptions are checked by the gccProlog.
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 071c9d2db98..13d4c8cbb45 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -2711,6 +2711,66 @@ func (b *Builder) cgo(a *Action, cgoExe, objdir string, pcCFLAGS, pcLDFLAGS, cgo
noCompiler()
}
+ // Double check the //go:cgo_ldflag comments in the generated files.
+ // The compiler only permits such comments in files whose base name
+ // starts with "_cgo_". Make sure that the comments in those files
+ // are safe. This is a backstop against people somehow smuggling
+ // such a comment into a file generated by cgo.
+ if cfg.BuildToolchainName == "gc" && !cfg.BuildN {
+ var flags []string
+ for _, f := range outGo {
+ if !strings.HasPrefix(filepath.Base(f), "_cgo_") {
+ continue
+ }
+
+ src, err := ioutil.ReadFile(f)
+ if err != nil {
+ return nil, nil, err
+ }
+
+ const cgoLdflag = "//go:cgo_ldflag"
+ idx := bytes.Index(src, []byte(cgoLdflag))
+ for idx >= 0 {
+ // We are looking at //go:cgo_ldflag.
+ // Find start of line.
+ start := bytes.LastIndex(src[:idx], []byte("\n"))
+ if start == -1 {
+ start = 0
+ }
+
+ // Find end of line.
+ end := bytes.Index(src[idx:], []byte("\n"))
+ if end == -1 {
+ end = len(src)
+ } else {
+ end += idx
+ }
+
+ // Check for first line comment in line.
+ // We don't worry about /* */ comments,
+ // which normally won't appear in files
+ // generated by cgo.
+ commentStart := bytes.Index(src[start:], []byte("//"))
+ commentStart += start
+ // If that line comment is //go:cgo_ldflag,
+ // it's a match.
+ if bytes.HasPrefix(src[commentStart:], []byte(cgoLdflag)) {
+ // Pull out the flag, and unquote it.
+ // This is what the compiler does.
+ flag := string(src[idx+len(cgoLdflag) : end])
+ flag = strings.TrimSpace(flag)
+ flag = strings.Trim(flag, `"`)
+ flags = append(flags, flag)
+ }
+ src = src[end:]
+ idx = bytes.Index(src, []byte(cgoLdflag))
+ }
+ }
+ if err := checkLinkerFlags("LDFLAGS", "go:cgo_ldflag", flags); err != nil {
+ return nil, nil, err
+ }
+ }
+
return outGo, outObj, nil
}
diff --git a/src/cmd/go/internal/work/security.go b/src/cmd/go/internal/work/security.go
index 3ee68ac1b41..0d9628241fd 100644
--- a/src/cmd/go/internal/work/security.go
+++ b/src/cmd/go/internal/work/security.go
@@ -42,8 +42,8 @@ import (
var re = lazyregexp.New
var validCompilerFlags = []*lazyregexp.Regexp{
- re(`-D([A-Za-z_].*)`),
- re(`-U([A-Za-z_]*)`),
+ re(`-D([A-Za-z_][A-Za-z0-9_]*)(=[^@\-]*)?`),
+ re(`-U([A-Za-z_][A-Za-z0-9_]*)`),
re(`-F([^@\-].*)`),
re(`-I([^@\-].*)`),
re(`-O`),
@@ -51,8 +51,8 @@ var validCompilerFlags = []*lazyregexp.Regexp{
re(`-W`),
re(`-W([^@,]+)`), // -Wall but not -Wa,-foo.
re(`-Wa,-mbig-obj`),
- re(`-Wp,-D([A-Za-z_].*)`),
- re(`-Wp,-U([A-Za-z_]*)`),
+ re(`-Wp,-D([A-Za-z_][A-Za-z0-9_]*)(=[^@,\-]*)?`),
+ re(`-Wp,-U([A-Za-z_][A-Za-z0-9_]*)`),
re(`-ansi`),
re(`-f(no-)?asynchronous-unwind-tables`),
re(`-f(no-)?blocks`),
diff --git a/src/cmd/go/internal/work/security_test.go b/src/cmd/go/internal/work/security_test.go
index 11e74f29c6a..aec9789185e 100644
--- a/src/cmd/go/internal/work/security_test.go
+++ b/src/cmd/go/internal/work/security_test.go
@@ -13,6 +13,7 @@ var goodCompilerFlags = [][]string{
{"-DFOO"},
{"-Dfoo=bar"},
{"-Ufoo"},
+ {"-Ufoo1"},
{"-F/Qt"},
{"-I/"},
{"-I/etc/passwd"},
@@ -24,6 +25,8 @@ var goodCompilerFlags = [][]string{
{"-Wall"},
{"-Wp,-Dfoo=bar"},
{"-Wp,-Ufoo"},
+ {"-Wp,-Dfoo1"},
+ {"-Wp,-Ufoo1"},
{"-fobjc-arc"},
{"-fno-objc-arc"},
{"-fomit-frame-pointer"},
@@ -78,6 +81,8 @@ var badCompilerFlags = [][]string{
{"-O@1"},
{"-Wa,-foo"},
{"-W@foo"},
+ {"-Wp,-DX,-D@X"},
+ {"-Wp,-UX,-U@X"},
{"-g@gdb"},
{"-g-gdb"},
{"-march=@dawn"},
diff --git a/src/math/big/nat.go b/src/math/big/nat.go
index 6a3989bf9d8..8c43de69d33 100644
--- a/src/math/big/nat.go
+++ b/src/math/big/nat.go
@@ -928,7 +928,7 @@ func (z nat) divRecursiveStep(u, v nat, depth int, tmp *nat, temps []*nat) {
// Now u < (v<<B), compute lower bits in the same way.
// Choose shift = B-1 again.
- s := B
+ s := B - 1
qhat := *temps[depth]
qhat.clear()
qhat.divRecursiveStep(u[s:].norm(), v[s:], depth+1, tmp, temps)