Skip to content

Commit

Permalink
replace uses of os/exec with golang.org/x/sys/execabs
Browse files Browse the repository at this point in the history
Go 1.15.7 contained a security fix for CVE-2021-3115, which allowed arbitrary
code to be executed at build time when using cgo on Windows. This issue also
affects Unix users who have “.” listed explicitly in their PATH and are running
“go get” outside of a module or with module mode disabled.

This issue is not limited to the go command itself, and can also affect binaries
that use `os.Command`, `os.LookPath`, etc.

From the related blogpost (ttps://blog.golang.org/path-security):

> Are your own programs affected?
>
> If you use exec.LookPath or exec.Command in your own programs, you only need to
> be concerned if you (or your users) run your program in a directory with untrusted
> contents. If so, then a subprocess could be started using an executable from dot
> instead of from a system directory. (Again, using an executable from dot happens
> always on Windows and only with uncommon PATH settings on Unix.)
>
> If you are concerned, then we’ve published the more restricted variant of os/exec
> as golang.org/x/sys/execabs. You can use it in your program by simply replacing

This patch replaces all uses of `os/exec` with `golang.org/x/sys/execabs`. While
some uses of `os/exec` should not be problematic (e.g. part of tests), it is
probably good to be consistent, in case code gets moved around.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
thaJeztah committed Aug 25, 2021
1 parent ebe8f8c commit 2ac9968
Show file tree
Hide file tree
Showing 41 changed files with 166 additions and 66 deletions.
2 changes: 1 addition & 1 deletion archive/compression/compression.go
Expand Up @@ -24,12 +24,12 @@ import (
"fmt"
"io"
"os"
"os/exec"
"strconv"
"sync"

"github.com/containerd/containerd/log"
"github.com/klauspost/compress/zstd"
exec "golang.org/x/sys/execabs"
)

type (
Expand Down
3 changes: 2 additions & 1 deletion archive/compression/compression_test.go
Expand Up @@ -24,11 +24,12 @@ import (
"io/ioutil"
"math/rand"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"testing"

exec "golang.org/x/sys/execabs"
)

func TestMain(m *testing.M) {
Expand Down
2 changes: 1 addition & 1 deletion archive/tar_test.go
Expand Up @@ -27,7 +27,6 @@ import (
"io"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"testing"
"time"
Expand All @@ -39,6 +38,7 @@ import (
"github.com/containerd/continuity/fs"
"github.com/containerd/continuity/fs/fstest"
"github.com/pkg/errors"
exec "golang.org/x/sys/execabs"
)

const tarCmd = "tar"
Expand Down
2 changes: 1 addition & 1 deletion cmd/containerd-shim/main_unix.go
Expand Up @@ -27,7 +27,6 @@ import (
"io"
"net"
"os"
"os/exec"
"os/signal"
"runtime"
"runtime/debug"
Expand All @@ -48,6 +47,7 @@ import (
ptypes "github.com/gogo/protobuf/types"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
exec "golang.org/x/sys/execabs"
"golang.org/x/sys/unix"
)

Expand Down
2 changes: 1 addition & 1 deletion cmd/containerd/command/service_windows.go
Expand Up @@ -21,7 +21,6 @@ import (
"io/ioutil"
"log"
"os"
"os/exec"
"path/filepath"
"time"
"unsafe"
Expand All @@ -31,6 +30,7 @@ import (
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/urfave/cli"
exec "golang.org/x/sys/execabs"
"golang.org/x/sys/windows"
"golang.org/x/sys/windows/svc"
"golang.org/x/sys/windows/svc/debug"
Expand Down
2 changes: 1 addition & 1 deletion cmd/ctr/commands/content/content.go
Expand Up @@ -21,7 +21,6 @@ import (
"io"
"io/ioutil"
"os"
"os/exec"
"strings"
"text/tabwriter"
"time"
Expand All @@ -35,6 +34,7 @@ import (
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/urfave/cli"
exec "golang.org/x/sys/execabs"
)

var (
Expand Down
2 changes: 1 addition & 1 deletion contrib/apparmor/template.go
Expand Up @@ -27,13 +27,13 @@ import (
"io"
"io/ioutil"
"os"
"os/exec"
"path"
"strconv"
"strings"
"text/template"

"github.com/pkg/errors"
exec "golang.org/x/sys/execabs"
)

// NOTE: This code is copied from <github.com/docker/docker/profiles/apparmor>.
Expand Down
11 changes: 6 additions & 5 deletions contrib/fuzz/container_fuzzer.go
Expand Up @@ -26,17 +26,18 @@ import (
"context"
"errors"
"fmt"
fuzz "github.com/AdaLogics/go-fuzz-headers"
"github.com/containerd/containerd"
"github.com/containerd/containerd/oci"
"github.com/containerd/containerd/sys"
"io"
"io/ioutil"
"net/http"
"os"
"os/exec"
"strings"
"time"

fuzz "github.com/AdaLogics/go-fuzz-headers"
"github.com/containerd/containerd"
"github.com/containerd/containerd/oci"
"github.com/containerd/containerd/sys"
exec "golang.org/x/sys/execabs"
)

var (
Expand Down
2 changes: 1 addition & 1 deletion contrib/nvidia/nvidia.go
Expand Up @@ -20,13 +20,13 @@ import (
"context"
"fmt"
"os"
"os/exec"
"strconv"
"strings"

"github.com/containerd/containerd/containers"
"github.com/containerd/containerd/oci"
specs "github.com/opencontainers/runtime-spec/specs-go"
exec "golang.org/x/sys/execabs"
)

// NvidiaCLI is the path to the Nvidia helper binary
Expand Down
2 changes: 1 addition & 1 deletion diff/stream_unix.go
Expand Up @@ -25,12 +25,12 @@ import (
"fmt"
"io"
"os"
"os/exec"
"sync"

"github.com/gogo/protobuf/proto"
"github.com/gogo/protobuf/types"
"github.com/pkg/errors"
exec "golang.org/x/sys/execabs"
)

// NewBinaryProcessor returns a binary processor for use with processing content streams
Expand Down
2 changes: 1 addition & 1 deletion diff/stream_windows.go
Expand Up @@ -23,7 +23,6 @@ import (
"io"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"sync"

Expand All @@ -32,6 +31,7 @@ import (
"github.com/gogo/protobuf/types"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
exec "golang.org/x/sys/execabs"
)

const processorPipe = "STREAM_PROCESSOR_PIPE"
Expand Down
2 changes: 1 addition & 1 deletion integration/client/client_test.go
Expand Up @@ -24,7 +24,6 @@ import (
"io"
"io/ioutil"
"os"
"os/exec"
"testing"
"time"

Expand All @@ -42,6 +41,7 @@ import (
"github.com/opencontainers/go-digest"
"github.com/opencontainers/image-spec/identity"
"github.com/sirupsen/logrus"
exec "golang.org/x/sys/execabs"
)

var (
Expand Down
2 changes: 1 addition & 1 deletion integration/client/container_linux_test.go
Expand Up @@ -23,7 +23,6 @@ import (
"io"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
Expand All @@ -50,6 +49,7 @@ import (
"github.com/containerd/typeurl"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
exec "golang.org/x/sys/execabs"
"golang.org/x/sys/unix"
)

Expand Down
9 changes: 4 additions & 5 deletions integration/client/container_test.go
Expand Up @@ -22,7 +22,6 @@ import (
"io"
"io/ioutil"
"os"
"os/exec"
"path"
"runtime"
"strings"
Expand All @@ -33,18 +32,18 @@ import (
. "github.com/containerd/containerd"
"github.com/containerd/containerd/cio"
"github.com/containerd/containerd/containers"
"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/namespaces"
"github.com/containerd/containerd/oci"
"github.com/containerd/containerd/platforms"
"github.com/containerd/containerd/plugin"
_ "github.com/containerd/containerd/runtime"
"github.com/containerd/containerd/runtime/v2/runc/options"
"github.com/containerd/typeurl"
specs "github.com/opencontainers/runtime-spec/specs-go"

"github.com/containerd/containerd/errdefs"
"github.com/containerd/go-runc"
"github.com/containerd/typeurl"
gogotypes "github.com/gogo/protobuf/types"
specs "github.com/opencontainers/runtime-spec/specs-go"
exec "golang.org/x/sys/execabs"
)

func empty() cio.Creator {
Expand Down
2 changes: 1 addition & 1 deletion integration/client/daemon_config_linux_test.go
Expand Up @@ -23,7 +23,6 @@ import (
"fmt"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"strings"
"syscall"
Expand All @@ -37,6 +36,7 @@ import (
"github.com/containerd/containerd/plugin"
"github.com/containerd/containerd/runtime/v2/runc/options"
srvconfig "github.com/containerd/containerd/services/server/config"
exec "golang.org/x/sys/execabs"
)

// the following nolint is for shutting up gometalinter on non-linux.
Expand Down
2 changes: 1 addition & 1 deletion integration/client/daemon_test.go
Expand Up @@ -19,12 +19,12 @@ package client
import (
"context"
"io"
"os/exec"
"sync"
"syscall"

. "github.com/containerd/containerd"
"github.com/pkg/errors"
exec "golang.org/x/sys/execabs"
)

type daemon struct {
Expand Down
2 changes: 1 addition & 1 deletion integration/image_load_test.go
Expand Up @@ -19,13 +19,13 @@ package integration
import (
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
exec "golang.org/x/sys/execabs"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
)

Expand Down
15 changes: 7 additions & 8 deletions integration/main_test.go
Expand Up @@ -22,7 +22,6 @@ import (
"flag"
"fmt"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
Expand All @@ -31,19 +30,19 @@ import (

"github.com/containerd/containerd"
cri "github.com/containerd/containerd/integration/cri-api/pkg/apis"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"

"github.com/containerd/containerd/integration/remote"
dialer "github.com/containerd/containerd/integration/util"
criconfig "github.com/containerd/containerd/pkg/cri/config"
"github.com/containerd/containerd/pkg/cri/constants"
"github.com/containerd/containerd/pkg/cri/server"
"github.com/containerd/containerd/pkg/cri/util"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
exec "golang.org/x/sys/execabs"
"google.golang.org/grpc"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
)

const (
Expand Down
2 changes: 1 addition & 1 deletion integration/volume_copy_up_test.go
Expand Up @@ -18,13 +18,13 @@ package integration

import (
"fmt"
"os/exec"
goruntime "runtime"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
exec "golang.org/x/sys/execabs"
)

func TestVolumeCopyUp(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion mount/lookup_linux_test.go
Expand Up @@ -20,7 +20,6 @@ import (
"fmt"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
Expand All @@ -29,6 +28,7 @@ import (
// so we use continuity/testutil instead.
"github.com/containerd/continuity/testutil"
"github.com/containerd/continuity/testutil/loopback"
exec "golang.org/x/sys/execabs"
"gotest.tools/v3/assert"
)

Expand Down
2 changes: 1 addition & 1 deletion mount/mount_freebsd.go
Expand Up @@ -18,10 +18,10 @@ package mount

import (
"os"
"os/exec"
"time"

"github.com/pkg/errors"
exec "golang.org/x/sys/execabs"
"golang.org/x/sys/unix"
)

Expand Down
2 changes: 1 addition & 1 deletion mount/mount_linux.go
Expand Up @@ -19,12 +19,12 @@ package mount
import (
"fmt"
"os"
"os/exec"
"path"
"strings"
"time"

"github.com/pkg/errors"
exec "golang.org/x/sys/execabs"
"golang.org/x/sys/unix"
)

Expand Down
2 changes: 1 addition & 1 deletion mount/mount_linux_test.go
Expand Up @@ -20,12 +20,12 @@ import (
"fmt"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"reflect"
"testing"

"github.com/containerd/continuity/testutil"
exec "golang.org/x/sys/execabs"
)

func TestLongestCommonPrefix(t *testing.T) {
Expand Down

0 comments on commit 2ac9968

Please sign in to comment.