feat: Introduce shell metacharacter escaping for exec

Signed-off-by: Nathanael Bracy <[email protected]>

Author: servusdei2018

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)
 }
 

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

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)
 	}

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()

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()