validate paths.varRun against an allowlist before findBaseDirectory trims it

pkg/networks/validate.go

@@ -61,6 +61,13 @@ func (c *Config) Validate() error {
 		pathsMap[name] = path
 		// varPath will be created securely, but any existing parent directories must already be secure
 		if name == "varRun" {
+			// findBaseDirectory drops the trailing components that don't exist yet,
+			// so the whole value has to be checked before it is trimmed: it is
+			// interpolated into the sudoers file and into the sudo command line
+			// long before the directory is created.
+			if err := validateVarRunString(path); err != nil {
+				return fmt.Errorf("networks.yaml field `paths.%s` error: %w", name, err)
+			}
 			path = findBaseDirectory(path)
 		}
 		err := validatePath(path, name == "varRun")
@@ -94,6 +101,36 @@ func findBaseDirectory(path string) string {
 	return path
 }
 
+// validateVarRunString validates the parts of paths.varRun that don't depend on
+// the filesystem, so it can be applied to the whole value before findBaseDirectory
+// trims the components that don't exist yet. varRun is interpolated verbatim into
+// the sudoers file, which has no quoting: whitespace ends the command token, a
+// newline starts a directive of its own, and a comma ends the command list entry.
+// reconcile.go also splits the command line on " " before handing it to sudo.
+// Rather than enumerating those metacharacters, require every component to be an
+// identifier, the same way the network name, mode, interface and group are.
+//
+// This is deliberately stricter than validatePath: it only applies to varRun,
+// so existing socketVMNet/sudoers layouts with dotted components keep validating.
+func validateVarRunString(path string) error {
+	if path == "" {
+		return nil
+	}
+	if path[0] != '/' {
+		return fmt.Errorf("path %#q is not an absolute path", path)
+	}
+	for component := range strings.SplitSeq(path, "/") {
+		if component == "" {
+			// leading, trailing and repeated separators
+			continue
+		}
+		if err := identifiers.Validate(component); err != nil {
+			return fmt.Errorf("path %#q has an invalid component: %w", path, err)
+		}
+	}
+	return nil
+}
+
 func validatePath(path string, allowDaemonGroupWritable bool) error {
 	if path == "" {
 		return nil

pkg/networks/validate_test.go

@@ -4,6 +4,8 @@
 package networks
 
 import (
+	"errors"
+	"os"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -38,3 +40,69 @@ func TestValidateRejectsInjectableNetworkDefinitions(t *testing.T) {
 	}}).Validate()
 	assert.ErrorContains(t, err, "invalid network name")
 }
+
+func TestValidateRejectsInjectableVarRun(t *testing.T) {
+	// findBaseDirectory() trims the components that don't exist yet, so the
+	// injected sudoers directive never reached the path check.
+	err := (&Config{Paths: Paths{
+		VarRun: "/private/var/run/lima\n%staff ALL=(root:wheel) NOPASSWD:NOSETENV: ALL",
+	}}).Validate()
+	assert.ErrorContains(t, err, "invalid component")
+
+	// A space in the same position injects an extra argument into the sudo command.
+	err = (&Config{Paths: Paths{
+		VarRun: "/private/var/run/lima --extra-root-flag",
+	}}).Validate()
+	assert.ErrorContains(t, err, "invalid component")
+
+	// A comma ends the command list entry, so "ALL" becomes a command of its own.
+	err = (&Config{Paths: Paths{
+		VarRun: "/private/var/run/lima,ALL",
+	}}).Validate()
+	assert.ErrorContains(t, err, "invalid component")
+}
+
+func TestValidateVarRunString(t *testing.T) {
+	for _, path := range []string{
+		"",
+		"/",
+		"/private/var/run/lima",
+		"/private/var/run/lima/",
+		"/private/etc/sudoers.d/lima",
+		"/opt/socket_vmnet/bin/socket_vmnet",
+		"/opt/homebrew/opt/socket_vmnet/bin/socket_vmnet",
+	} {
+		assert.NilError(t, validateVarRunString(path), path)
+	}
+	for _, path := range []string{
+		"private/var/run/lima",
+		"/private/var/run/lima ALL",
+		"/private/var/run/lima\tALL",
+		"/private/var/run/lima\nALL",
+		"/private/var/run/lima,ALL",
+		"/private/var/run/lima:ALL",
+		"/private/var/run/lima\\ ALL",
+		"/private/var/run/../../etc",
+	} {
+		assert.Assert(t, validateVarRunString(path) != nil, path)
+	}
+}
+
+// The stricter identifier rule is applied to varRun only. socketVMNet and sudoers
+// keep the whitespace check from master, so layouts with dotted components (which
+// identifiers.Validate rejects) must still pass validatePath's string checks.
+func TestValidatePathAllowsDottedComponents(t *testing.T) {
+	for _, path := range []string{
+		"/lima-nonexistent-test/foo/.local/bin/socket_vmnet",
+		"/lima-nonexistent-test/etc/sudoers.d/lima",
+	} {
+		// os.Lstat runs after the string checks and fails because the path does
+		// not exist; reaching that error means the dotted component passed the
+		// stricter identifier rule that would apply to varRun.
+		err := validatePath(path, false)
+		assert.Assert(t, errors.Is(err, os.ErrNotExist), "%s: %v", path, err)
+	}
+
+	err := validatePath("/private/var/run/lima ALL", false)
+	assert.ErrorContains(t, err, "contains whitespace")
+}