diff --git a/cmd/nvidia-container-runtime-hook/main.go b/cmd/nvidia-container-runtime-hook/main.go index faaf0b51..45e5be52 100644 --- a/cmd/nvidia-container-runtime-hook/main.go +++ b/cmd/nvidia-container-runtime-hook/main.go @@ -17,6 +17,7 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/internal/info" "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" "github.com/NVIDIA/nvidia-container-toolkit/internal/lookup" + "github.com/NVIDIA/nvidia-container-toolkit/internal/oci" ) var ( @@ -143,10 +144,10 @@ func doPrestart() { args = append(args, fmt.Sprintf("--pid=%s", strconv.FormatUint(uint64(container.Pid), 10))) args = append(args, rootfs) + args = oci.Escape(args) env := append(os.Environ(), cli.Environment...) - //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection? - err = syscall.Exec(args[0], args, env) + err = syscall.Exec(args[0], args, env) //nolint:gosec log.Panicln("exec failed:", err) } diff --git a/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go b/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go index 5a94a834..d14fecb9 100644 --- a/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go +++ b/cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go @@ -131,9 +131,9 @@ func (m command) run(c *cli.Context, cfg *options) error { // Explicitly specify using /etc/ld.so.conf since the host's ldconfig may // be configured to use a different config file by default. args = append(args, "-f", "/etc/ld.so.conf") + args = oci.Escape(args) - //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection - return syscall.Exec(ldconfigPath, args, nil) + return syscall.Exec(ldconfigPath, args, nil) //nolint:gosec } type root string diff --git a/internal/oci/runtime_syscall_exec.go b/internal/oci/runtime_syscall_exec.go index 6820e3a1..83e31896 100644 --- a/internal/oci/runtime_syscall_exec.go +++ b/internal/oci/runtime_syscall_exec.go @@ -19,16 +19,47 @@ package oci import ( "fmt" "os" + "regexp" + "strings" "syscall" ) +// shellMetachars represents a set of shell metacharacters that are commonly +// used for shell scripting and may lead to security vulnerabilities if not +// properly handled. +// +// These metacharacters include: | & ; ( ) < > \t \n $ \ ` +const shellMetachars = "|&;()<> \t\n$\\`" + type syscallExec struct{} var _ Runtime = (*syscallExec)(nil) +// Escape1 escapes shell metacharacters in a single command-line argument. +func Escape1(arg string) string { + if strings.ContainsAny(arg, shellMetachars) { + // Argument contains shell metacharacters. Double quote the + // argument, and backslash-escape any characters that still have + // meaning inside of double quotes. + e := regexp.MustCompile("([$`\"\\\\])").ReplaceAllString(arg, `\$1`) + return fmt.Sprintf(`"%s"`, e) + } + return arg +} + +// Escape escapes shell metacharacters in a slice of command-line arguments +// and returns a new slice containing the escaped arguments. +func Escape(args []string) []string { + escaped := make([]string, len(args)) + for i := range args { + escaped[i] = Escape1(args[i]) + } + return escaped +} + func (r syscallExec) Exec(args []string) error { - //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection - err := syscall.Exec(args[0], args, os.Environ()) + args = Escape(args) + err := syscall.Exec(args[0], args, os.Environ()) //nolint:gosec if err != nil { return fmt.Errorf("could not exec '%v': %v", args[0], err) } diff --git a/tools/container/container.go b/tools/container/container.go index c2c50c5b..3a306e86 100644 --- a/tools/container/container.go +++ b/tools/container/container.go @@ -24,6 +24,7 @@ import ( "github.com/sirupsen/logrus" "github.com/urfave/cli/v2" + "github.com/NVIDIA/nvidia-container-toolkit/internal/oci" "github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine" "github.com/NVIDIA/nvidia-container-toolkit/tools/container/operator" ) @@ -155,11 +156,11 @@ func (o Options) SystemdRestart(service string) error { args = append(args, "chroot", o.HostRootMount) } args = append(args, "systemctl", "restart", service) + args = oci.Escape(args) logrus.Infof("Restarting %v%v using systemd: %v", service, msg, args) - //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection - cmd := exec.Command(args[0], args[1:]...) + cmd := exec.Command(args[0], args[1:]...) //nolint:gosec cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr err := cmd.Run() diff --git a/tools/container/nvidia-toolkit/run.go b/tools/container/nvidia-toolkit/run.go index 87d5b111..2704f7aa 100644 --- a/tools/container/nvidia-toolkit/run.go +++ b/tools/container/nvidia-toolkit/run.go @@ -229,8 +229,7 @@ func installToolkit(o *options) error { filepath.Join(o.root, toolkitSubDir), } - //nolint:gosec // TODO: Can we harden this so that there is less risk of command injection - cmd := exec.Command("sh", "-c", strings.Join(cmdline, " ")) + cmd := exec.Command("sh", "-c", strings.Join(cmdline, " ")) //nolint:gosec cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr err := cmd.Run()