Merge pull request #5248 from Nashit-h/networks-validate-definitions

validate network names and fields used in sudoers commands

pkg/networks/validate.go

@@ -13,10 +13,40 @@ import (
 	"runtime"
 	"strings"
 
+	"github.com/lima-vm/lima/v2/pkg/identifiers"
 	"github.com/lima-vm/lima/v2/pkg/osutil"
 )
 
 func (c *Config) Validate() error {
+	// The group name and the per-network name/mode/interface are interpolated
+	// verbatim into the sudoers file (sudoers.go) and into the socket_vmnet
+	// command that reconcile.go runs via sudo after splitting it on spaces
+	// (commands.go). A value with whitespace injects an extra argument, and a
+	// newline in a network name adds an arbitrary directive to the generated
+	// sudoers file, so require them to be valid identifiers. Interface is empty
+	// for non-bridged networks, and group defaults to "admin" when unset, so
+	// only validate those when a value is actually present.
+	if c.Group != "" {
+		if err := identifiers.Validate(c.Group); err != nil {
+			return fmt.Errorf("invalid group %#q: %w", c.Group, err)
+		}
+	}
+	for name, nw := range c.Networks {
+		if err := identifiers.Validate(name); err != nil {
+			return fmt.Errorf("invalid network name %#q: %w", name, err)
+		}
+		if nw.Mode != "" {
+			if err := identifiers.Validate(nw.Mode); err != nil {
+				return fmt.Errorf("invalid mode %#q for network %#q: %w", nw.Mode, name, err)
+			}
+		}
+		if nw.Interface != "" {
+			if err := identifiers.Validate(nw.Interface); err != nil {
+				return fmt.Errorf("invalid interface %#q for network %#q: %w", nw.Interface, name, err)
+			}
+		}
+	}
+
 	// validate all paths.* values
 	paths := reflect.ValueOf(&c.Paths).Elem()
 	pathsMap := make(map[string]string, paths.NumField())
@@ -51,7 +81,6 @@ func (c *Config) Validate() error {
 	if socketVMNetNotFound {
 		return fmt.Errorf("networks.yaml: %#q (`paths.socketVMNet`) has to be installed", pathsMap["socketVMNet"])
 	}
-	// TODO(jandubois): validate network definitions
 	return nil
 }
 

pkg/networks/validate_test.go

@@ -0,0 +1,40 @@
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package networks
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func TestValidateRejectsInjectableNetworkDefinitions(t *testing.T) {
+	// A newline in a network name adds a directive to the generated sudoers file.
+	err := (&Config{Networks: map[string]Network{
+		"evil\n%staff ALL=(root) NOPASSWD: ALL": {Mode: ModeShared},
+	}}).Validate()
+	assert.ErrorContains(t, err, "invalid network name")
+
+	// A space in the interface injects an extra argument into the sudo command.
+	err = (&Config{Networks: map[string]Network{
+		"bridged": {Mode: ModeBridged, Interface: "en0 --extra-root-flag"},
+	}}).Validate()
+	assert.ErrorContains(t, err, "invalid interface")
+
+	// Whitespace in the mode injects an extra argument into the sudo command.
+	err = (&Config{Networks: map[string]Network{
+		"shared": {Mode: "shared --extra"},
+	}}).Validate()
+	assert.ErrorContains(t, err, "invalid mode")
+
+	// Whitespace in the group breaks the sudoers header line.
+	err = (&Config{Group: "admin bad", Networks: map[string]Network{}}).Validate()
+	assert.ErrorContains(t, err, "invalid group")
+
+	// A path-traversal network name would redirect the pidfile/socket path.
+	err = (&Config{Networks: map[string]Network{
+		"../../etc/foo": {Mode: ModeShared},
+	}}).Validate()
+	assert.ErrorContains(t, err, "invalid network name")
+}