]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.15-security] all: introduce and use internal/execabs
authorRoland Shoemaker <roland@golang.org>
Fri, 15 Jan 2021 20:14:06 +0000 (12:14 -0800)
committerRoland Shoemaker <bracewell@google.com>
Sat, 16 Jan 2021 00:29:16 +0000 (00:29 +0000)
Introduces a wrapper around os/exec, internal/execabs, for use in
all commands. This wrapper prevents exec.LookPath and exec.Command from
running executables in the current directory.

All imports of os/exec in non-test files in cmd/ are replaced with
imports of internal/execabs.

This issue was reported by RyotaK.

Fixes CVE-2021-3115

Change-Id: I0423451a6e27ec1e1d6f3fe929ab1ef69145c08f
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/955304
Reviewed-by: Russ Cox <rsc@google.com>
Reviewed-by: Katie Hockman <katiehockman@google.com>
(cherry picked from commit 44f09a6990ccf4db601cbf8208c89ac4e888f884)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/955308

38 files changed:
src/cmd/api/goapi.go
src/cmd/api/run.go
src/cmd/cgo/out.go
src/cmd/cgo/util.go
src/cmd/compile/internal/ssa/html.go
src/cmd/cover/func.go
src/cmd/cover/testdata/toolexec.go
src/cmd/dist/buildtool.go
src/cmd/doc/dirs.go
src/cmd/fix/typecheck.go
src/cmd/go/internal/base/base.go
src/cmd/go/internal/bug/bug.go
src/cmd/go/internal/generate/generate.go
src/cmd/go/internal/modfetch/codehost/codehost.go
src/cmd/go/internal/modfetch/codehost/git.go
src/cmd/go/internal/test/genflags.go
src/cmd/go/internal/test/test.go
src/cmd/go/internal/tool/tool.go
src/cmd/go/internal/vet/vetflag.go
src/cmd/go/internal/work/build.go
src/cmd/go/internal/work/buildid.go
src/cmd/go/internal/work/exec.go
src/cmd/go/internal/work/gccgo.go
src/cmd/go/testdata/addmod.go
src/cmd/internal/browser/browser.go
src/cmd/internal/diff/diff.go
src/cmd/internal/dwarf/dwarf.go
src/cmd/link/internal/ld/execarchive.go
src/cmd/link/internal/ld/lib.go
src/cmd/test2json/main.go
src/cmd/trace/pprof.go
src/go/build/build.go
src/go/build/deps_test.go
src/go/internal/gccgoimporter/gccgoinstallation.go
src/go/internal/srcimporter/srcimporter.go
src/internal/execabs/execabs.go [new file with mode: 0644]
src/internal/execabs/execabs_test.go [new file with mode: 0644]
src/internal/goroot/gc.go

index 01b17b8839209d3ab242364ae25d78e6011f53af..6cddfc100bb6e29597a368787e1391eec49502da 100644 (file)
@@ -16,11 +16,11 @@ import (
        "go/parser"
        "go/token"
        "go/types"
+       exec "internal/execabs"
        "io"
        "io/ioutil"
        "log"
        "os"
-       "os/exec"
        "path/filepath"
        "regexp"
        "runtime"
index a36f1179c16e9839fa7f1bd417b788a27d9df6db..ecb1d0f81aa46695d47634b94eb6571368e0ffef 100644 (file)
@@ -10,9 +10,9 @@ package main
 
 import (
        "fmt"
+       exec "internal/execabs"
        "log"
        "os"
-       "os/exec"
        "path/filepath"
        "runtime"
        "strings"
index dcd69edb44b4034ff8e81484b7068ea6cd5bbda8..b9043efbf7807fc5ce07644017505f87cd4dea27 100644 (file)
@@ -13,11 +13,11 @@ import (
        "go/ast"
        "go/printer"
        "go/token"
+       exec "internal/execabs"
        "internal/xcoff"
        "io"
        "io/ioutil"
        "os"
-       "os/exec"
        "path/filepath"
        "regexp"
        "sort"
index 779f7be2259b3750166b6edce318adb779e7a3a6..00d931b98a0c1e8416d5bb14334946dbd9a56304 100644 (file)
@@ -8,9 +8,9 @@ import (
        "bytes"
        "fmt"
        "go/token"
+       exec "internal/execabs"
        "io/ioutil"
        "os"
-       "os/exec"
 )
 
 // run runs the command argv, feeding in stdin on standard input.
index ba37a804123919009630c8808464f2acaad1db02..99a4157937530ab2ad8aa72efecc9c265862d2a2 100644 (file)
@@ -9,9 +9,9 @@ import (
        "cmd/internal/src"
        "fmt"
        "html"
+       exec "internal/execabs"
        "io"
        "os"
-       "os/exec"
        "path/filepath"
        "strconv"
        "strings"
index 988c4caebffe13dc11e6bffb304016abc5673723..ce7c771ac961971af670b00c1c79ea1c8b22153c 100644 (file)
@@ -15,9 +15,9 @@ import (
        "go/ast"
        "go/parser"
        "go/token"
+       exec "internal/execabs"
        "io"
        "os"
-       "os/exec"
        "path"
        "path/filepath"
        "runtime"
index 1769efedbeb678674045a9d6b9bbc08fb4e5ed30..386de79038a355c2de01fead816a1410479490f8 100644 (file)
@@ -16,7 +16,7 @@ package main
 
 import (
        "os"
-       "os/exec"
+       exec "internal/execabs"
        "strings"
 )
 
index 9502dac4eb2bda96a13bc2b7fbf07b03c1cfc6d2..710f4bde6f0e6879301e3e5568834a5aa9864a02 100644 (file)
@@ -302,8 +302,10 @@ func bootstrapFixImports(srcFile string) string {
                        continue
                }
                if strings.HasPrefix(line, `import "`) || strings.HasPrefix(line, `import . "`) ||
-                       inBlock && (strings.HasPrefix(line, "\t\"") || strings.HasPrefix(line, "\t. \"")) {
+                       inBlock && (strings.HasPrefix(line, "\t\"") || strings.HasPrefix(line, "\t. \"") || strings.HasPrefix(line, "\texec \"")) {
                        line = strings.Replace(line, `"cmd/`, `"bootstrap/cmd/`, -1)
+                       // During bootstrap, must use plain os/exec.
+                       line = strings.Replace(line, `exec "internal/execabs"`, `"os/exec"`, -1)
                        for _, dir := range bootstrapDirs {
                                if strings.HasPrefix(dir, "cmd/") {
                                        continue
index 38cbe7fa021973943c459461684f14b58220b7be..661624cfe4c168c793d582203de163f3169634e6 100644 (file)
@@ -7,9 +7,9 @@ package main
 import (
        "bytes"
        "fmt"
+       exec "internal/execabs"
        "log"
        "os"
-       "os/exec"
        "path/filepath"
        "regexp"
        "strings"
index 66e0cdcec056dfccf0d376c657345ed6626010be..8390e4446c030dbfe1d52d2a47832684f0b6c4be 100644 (file)
@@ -9,9 +9,9 @@ import (
        "go/ast"
        "go/parser"
        "go/token"
+       exec "internal/execabs"
        "io/ioutil"
        "os"
-       "os/exec"
        "path/filepath"
        "reflect"
        "runtime"
index ab2f1bb4e2c27d774e552a3cbe47cbf5f3a23194..b63303afd1827d524c0f7bd808af61384e359e14 100644 (file)
@@ -9,9 +9,9 @@ package base
 import (
        "flag"
        "fmt"
+       exec "internal/execabs"
        "log"
        "os"
-       "os/exec"
        "strings"
        "sync"
 
index fe71281ef054ebebf4a3f51c0f267acbdc4aa999..9434bc2b1c11b23967b1b3642b367eb02fda4ba7 100644 (file)
@@ -8,11 +8,11 @@ package bug
 import (
        "bytes"
        "fmt"
+       exec "internal/execabs"
        "io"
        "io/ioutil"
        urlpkg "net/url"
        "os"
-       "os/exec"
        "path/filepath"
        "regexp"
        "runtime"
index 093b19817b5197fead3edcfeea2ffecde7508d87..3abb4ca537da1cfd00a45b3448ca4241e220f98d 100644 (file)
@@ -11,11 +11,11 @@ import (
        "fmt"
        "go/parser"
        "go/token"
+       exec "internal/execabs"
        "io"
        "io/ioutil"
        "log"
        "os"
-       "os/exec"
        "path/filepath"
        "regexp"
        "strconv"
index d85eddf767be100f3a4d7da79140466464d88fe8..058052e32903a3659c30dd88e550cd35c6640686 100644 (file)
@@ -10,10 +10,10 @@ import (
        "bytes"
        "crypto/sha256"
        "fmt"
+       exec "internal/execabs"
        "io"
        "io/ioutil"
        "os"
-       "os/exec"
        "path/filepath"
        "strings"
        "sync"
index 31921324a7d10e3fdddc32ec5fcaacf379490d7f..06b2fa4fb56ee9d5973232591fe7d13040c3dc47 100644 (file)
@@ -8,11 +8,11 @@ import (
        "bytes"
        "errors"
        "fmt"
+       exec "internal/execabs"
        "io"
        "io/ioutil"
        "net/url"
        "os"
-       "os/exec"
        "path/filepath"
        "sort"
        "strconv"
index 512fa1671ef7db9985e5591c88c0a47355232406..1331287fa369bbc35c396aaffed3ef83c977f423 100644 (file)
@@ -9,9 +9,9 @@ package main
 import (
        "bytes"
        "flag"
+       exec "internal/execabs"
        "log"
        "os"
-       "os/exec"
        "strings"
        "testing"
        "text/template"
index 77bfc11fe9a45dd64fd581216975debe8ca248a2..8cb902748efa6606a8679e406e2c0286ec2ee9fc 100644 (file)
@@ -10,10 +10,10 @@ import (
        "errors"
        "fmt"
        "go/build"
+       exec "internal/execabs"
        "io"
        "io/ioutil"
        "os"
-       "os/exec"
        "path"
        "path/filepath"
        "regexp"
index 930eecb63f1c4955c842e6e7d10a936e77783254..f06c9038a063eaf0135552a9de1428b8941aedc9 100644 (file)
@@ -7,8 +7,8 @@ package tool
 
 import (
        "fmt"
+       exec "internal/execabs"
        "os"
-       "os/exec"
        "sort"
        "strings"
 
index ef995ef83539a1f2e7017fd12b3bbd6384abcb81..5bf5cf44467f144f27df7a267203c9745e1645e5 100644 (file)
@@ -10,9 +10,9 @@ import (
        "errors"
        "flag"
        "fmt"
+       exec "internal/execabs"
        "log"
        "os"
-       "os/exec"
        "path/filepath"
        "strings"
 
index 7146c9ce00e8e282f572c66196d8a3db25333003..a2cbea870dc2231b65090eab3b915518920297ca 100644 (file)
@@ -8,8 +8,8 @@ import (
        "errors"
        "fmt"
        "go/build"
+       exec "internal/execabs"
        "os"
-       "os/exec"
        "path/filepath"
        "runtime"
        "strings"
index 6613b6fe3f2fa500b2306eec81344efe17429d76..2a79d9d76773f8a42ed4fbe00749c4211781192f 100644 (file)
@@ -7,9 +7,9 @@ package work
 import (
        "bytes"
        "fmt"
+       exec "internal/execabs"
        "io/ioutil"
        "os"
-       "os/exec"
        "strings"
 
        "cmd/go/internal/base"
index 3c39734590191ae45ab1c8627b035cd81373b9bb..072162a26cc2950f250413442b96374a49b421e2 100644 (file)
@@ -11,13 +11,13 @@ import (
        "encoding/json"
        "errors"
        "fmt"
+       exec "internal/execabs"
        "internal/lazyregexp"
        "io"
        "io/ioutil"
        "log"
        "math/rand"
        "os"
-       "os/exec"
        "path/filepath"
        "regexp"
        "runtime"
index 4c1f36dbd6a6807fcb2bed818470891dd5aa5d69..2f5d5d628374af53ec3cf70ffbea09906ee2be30 100644 (file)
@@ -6,9 +6,9 @@ package work
 
 import (
        "fmt"
+       exec "internal/execabs"
        "io/ioutil"
        "os"
-       "os/exec"
        "path/filepath"
        "strings"
 
index d9c3aab9c49630306a61607398f21b451283b39c..9c74cf500b6e1954c0705ff751251f32a3e54786 100644 (file)
@@ -25,7 +25,7 @@ import (
        "io/ioutil"
        "log"
        "os"
-       "os/exec"
+       exec "internal/execabs"
        "path/filepath"
        "strings"
 
index 6867c85d2320406bb4c80b8960f5b958c6a2f196..577d31789f19afcb768e847cd69d2714e4d48b06 100644 (file)
@@ -6,8 +6,8 @@
 package browser
 
 import (
+       exec "internal/execabs"
        "os"
-       "os/exec"
        "runtime"
        "time"
 )
index e9d2c237800a9c6c658eb41db1b58dcff102fe18..c0ca2f310630cd7266a770d6b42748ae034cb050 100644 (file)
@@ -7,9 +7,9 @@
 package diff
 
 import (
+       exec "internal/execabs"
        "io/ioutil"
        "os"
-       "os/exec"
        "runtime"
 )
 
index a17b574cdd8d97dd47c6d06e9aaebe3ff6102986..1269ebc2630d9f0d02cfe8dee073d8affa855268 100644 (file)
@@ -12,7 +12,7 @@ import (
        "cmd/internal/objabi"
        "errors"
        "fmt"
-       "os/exec"
+       exec "internal/execabs"
        "sort"
        "strconv"
        "strings"
index fe5cc4086573b4b7754ec604f6e64c09e6a7b3e2..4687c624de464611e7de9036ae187c3e94be6023 100644 (file)
@@ -7,8 +7,8 @@
 package ld
 
 import (
+       exec "internal/execabs"
        "os"
-       "os/exec"
        "path/filepath"
        "syscall"
 )
index 0366bc7a6f456aa13b49481376091935ab55301a..fc897599970020f1ee79d19246f6ad98604e366e 100644 (file)
@@ -50,11 +50,11 @@ import (
        "encoding/binary"
        "encoding/hex"
        "fmt"
+       exec "internal/execabs"
        "io"
        "io/ioutil"
        "log"
        "os"
-       "os/exec"
        "path/filepath"
        "runtime"
        "sort"
index 57a874193e311cb19797dee89f16055fde81edeb..14626974989834e4118ac9856b04bad6db1dd3d4 100644 (file)
@@ -82,9 +82,9 @@ package main
 import (
        "flag"
        "fmt"
+       exec "internal/execabs"
        "io"
        "os"
-       "os/exec"
 
        "cmd/internal/test2json"
 )
index a31d71b013c5da41f935e902376c150444c6cbc4..e6ea1a4ab61531809db065c87800918da92f0bbe 100644 (file)
@@ -9,12 +9,12 @@ package main
 import (
        "bufio"
        "fmt"
+       exec "internal/execabs"
        "internal/trace"
        "io"
        "io/ioutil"
        "net/http"
        "os"
-       "os/exec"
        "path/filepath"
        "runtime"
        "sort"
index 4a5da308a0c584ebe511ec7b5a1c1fc4661dcaee..9050d070f8a1b22e0a75efe5fabbc6d77c09a437 100644 (file)
@@ -12,12 +12,12 @@ import (
        "go/doc"
        "go/parser"
        "go/token"
+       exec "internal/execabs"
        "internal/goroot"
        "internal/goversion"
        "io"
        "io/ioutil"
        "os"
-       "os/exec"
        pathpkg "path"
        "path/filepath"
        "runtime"
index fa8ecf10f4281542d40a5b5bca6fdb010b64208c..875acebf9c5c7ee1a5a503a1d86306d1410c45bc 100644 (file)
@@ -161,7 +161,7 @@ var depsRules = `
        reflect !< OS;
 
        OS
-       < golang.org/x/sys/cpu, internal/goroot;
+       < golang.org/x/sys/cpu;
 
        # FMT is OS (which includes string routines) plus reflect and fmt.
        # It does not include package log, which should be avoided in core packages.
@@ -177,6 +177,12 @@ var depsRules = `
 
        log !< FMT;
 
+       OS, FMT
+       < internal/execabs;
+
+       OS, internal/execabs
+       < internal/goroot;
+
        # Misc packages needing only FMT.
        FMT
        < flag,
index 8fc7ce3232190c93bb74f445bf55be1929e4c786..e90a3cc0b0a343e601bc2fe6389e43a160c713b9 100644 (file)
@@ -7,8 +7,8 @@ package gccgoimporter
 import (
        "bufio"
        "go/types"
+       exec "internal/execabs"
        "os"
-       "os/exec"
        "path/filepath"
        "strings"
 )
index 90bb3a9bc11554e60163d9b59f4989c8d757a554..37d588335421e3a0f7da229befe049f7c67cc524 100644 (file)
@@ -13,10 +13,10 @@ import (
        "go/parser"
        "go/token"
        "go/types"
+       exec "internal/execabs"
        "io"
        "io/ioutil"
        "os"
-       "os/exec"
        "path/filepath"
        "strings"
        "sync"
diff --git a/src/internal/execabs/execabs.go b/src/internal/execabs/execabs.go
new file mode 100644 (file)
index 0000000..547c3a5
--- /dev/null
@@ -0,0 +1,70 @@
+// 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 execabs is a drop-in replacement for os/exec
+// that requires PATH lookups to find absolute paths.
+// That is, execabs.Command("cmd") runs the same PATH lookup
+// as exec.Command("cmd"), but if the result is a path
+// which is relative, the Run and Start methods will report
+// an error instead of running the executable.
+package execabs
+
+import (
+       "context"
+       "fmt"
+       "os/exec"
+       "path/filepath"
+       "reflect"
+       "unsafe"
+)
+
+var ErrNotFound = exec.ErrNotFound
+
+type (
+       Cmd       = exec.Cmd
+       Error     = exec.Error
+       ExitError = exec.ExitError
+)
+
+func relError(file, path string) error {
+       return fmt.Errorf("%s resolves to executable in current directory (.%c%s)", file, filepath.Separator, path)
+}
+
+func LookPath(file string) (string, error) {
+       path, err := exec.LookPath(file)
+       if err != nil {
+               return "", err
+       }
+       if filepath.Base(file) == file && !filepath.IsAbs(path) {
+               return "", relError(file, path)
+       }
+       return path, nil
+}
+
+func fixCmd(name string, cmd *exec.Cmd) {
+       if filepath.Base(name) == name && !filepath.IsAbs(cmd.Path) {
+               // exec.Command was called with a bare binary name and
+               // exec.LookPath returned a path which is not absolute.
+               // Set cmd.lookPathErr and clear cmd.Path so that it
+               // cannot be run.
+               lookPathErr := (*error)(unsafe.Pointer(reflect.ValueOf(cmd).Elem().FieldByName("lookPathErr").Addr().Pointer()))
+               if *lookPathErr == nil {
+                       *lookPathErr = relError(name, cmd.Path)
+               }
+               cmd.Path = ""
+       }
+}
+
+func CommandContext(ctx context.Context, name string, arg ...string) *exec.Cmd {
+       cmd := exec.CommandContext(ctx, name, arg...)
+       fixCmd(name, cmd)
+       return cmd
+
+}
+
+func Command(name string, arg ...string) *exec.Cmd {
+       cmd := exec.Command(name, arg...)
+       fixCmd(name, cmd)
+       return cmd
+}
diff --git a/src/internal/execabs/execabs_test.go b/src/internal/execabs/execabs_test.go
new file mode 100644 (file)
index 0000000..a0b88dd
--- /dev/null
@@ -0,0 +1,107 @@
+// 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 execabs
+
+import (
+       "context"
+       "fmt"
+       "io/ioutil"
+       "os"
+       "os/exec"
+       "path/filepath"
+       "runtime"
+       "testing"
+)
+
+func TestFixCmd(t *testing.T) {
+       cmd := &exec.Cmd{Path: "hello"}
+       fixCmd("hello", cmd)
+       if cmd.Path != "" {
+               t.Errorf("fixCmd didn't clear cmd.Path")
+       }
+       expectedErr := fmt.Sprintf("hello resolves to executable in current directory (.%chello)", filepath.Separator)
+       if err := cmd.Run(); err == nil {
+               t.Fatal("Command.Run didn't fail")
+       } else if err.Error() != expectedErr {
+               t.Fatalf("Command.Run returned unexpected error: want %q, got %q", expectedErr, err.Error())
+       }
+}
+
+func TestCommand(t *testing.T) {
+       for _, cmd := range []func(string) *Cmd{
+               func(s string) *Cmd { return Command(s) },
+               func(s string) *Cmd { return CommandContext(context.Background(), s) },
+       } {
+               tmpDir, err := ioutil.TempDir("", "execabs-test")
+               if err != nil {
+                       t.Fatalf("ioutil.TempDir failed: %s", err)
+               }
+               defer os.RemoveAll(tmpDir)
+               executable := "execabs-test"
+               if runtime.GOOS == "windows" {
+                       executable += ".exe"
+               }
+               if err = ioutil.WriteFile(filepath.Join(tmpDir, executable), []byte{1, 2, 3}, 0111); err != nil {
+                       t.Fatalf("ioutil.WriteFile failed: %s", err)
+               }
+               cwd, err := os.Getwd()
+               if err != nil {
+                       t.Fatalf("os.Getwd failed: %s", err)
+               }
+               defer os.Chdir(cwd)
+               if err = os.Chdir(tmpDir); err != nil {
+                       t.Fatalf("os.Chdir failed: %s", err)
+               }
+               if runtime.GOOS != "windows" {
+                       // add "." to PATH so that exec.LookPath looks in the current directory on
+                       // non-windows platforms as well
+                       origPath := os.Getenv("PATH")
+                       defer os.Setenv("PATH", origPath)
+                       os.Setenv("PATH", fmt.Sprintf(".:%s", origPath))
+               }
+               expectedErr := fmt.Sprintf("execabs-test resolves to executable in current directory (.%c%s)", filepath.Separator, executable)
+               if err = cmd("execabs-test").Run(); err == nil {
+                       t.Fatalf("Command.Run didn't fail when exec.LookPath returned a relative path")
+               } else if err.Error() != expectedErr {
+                       t.Errorf("Command.Run returned unexpected error: want %q, got %q", expectedErr, err.Error())
+               }
+       }
+}
+
+func TestLookPath(t *testing.T) {
+       tmpDir, err := ioutil.TempDir("", "execabs-test")
+       if err != nil {
+               t.Fatalf("ioutil.TempDir failed: %s", err)
+       }
+       defer os.RemoveAll(tmpDir)
+       executable := "execabs-test"
+       if runtime.GOOS == "windows" {
+               executable += ".exe"
+       }
+       if err = ioutil.WriteFile(filepath.Join(tmpDir, executable), []byte{1, 2, 3}, 0111); err != nil {
+               t.Fatalf("ioutil.WriteFile failed: %s", err)
+       }
+       cwd, err := os.Getwd()
+       if err != nil {
+               t.Fatalf("os.Getwd failed: %s", err)
+       }
+       defer os.Chdir(cwd)
+       if err = os.Chdir(tmpDir); err != nil {
+               t.Fatalf("os.Chdir failed: %s", err)
+       }
+       if runtime.GOOS != "windows" {
+               // add "." to PATH so that exec.LookPath looks in the current directory on
+               // non-windows platforms as well
+               origPath := os.Getenv("PATH")
+               defer os.Setenv("PATH", origPath)
+               os.Setenv("PATH", fmt.Sprintf(".:%s", origPath))
+       }
+       expectedErr := fmt.Sprintf("execabs-test resolves to executable in current directory (.%c%s)", filepath.Separator, executable)
+       if _, err := LookPath("execabs-test"); err == nil {
+               t.Fatalf("LookPath didn't fail when finding a non-relative path")
+       } else if err.Error() != expectedErr {
+               t.Errorf("LookPath returned unexpected error: want %q, got %q", expectedErr, err.Error())
+       }
+}
index 0f541d734b0418e54ffcf9200180a4969bb42c7d..ce72bc389656a43681504ee97feff50b60526f1a 100644 (file)
@@ -7,8 +7,8 @@
 package goroot
 
 import (
+       exec "internal/execabs"
        "os"
-       "os/exec"
        "path/filepath"
        "strings"
        "sync"