Merge pull request from GHSA-6x2m-w449-qwx7

haircommander · web-flow · commit d93b2dfb8d0f · 2022-03-15T10:11:09.000-04:00
[1.22] config/sysctl: fail if there is a + in the value
diff --git a/internal/config/nsmgr/nsmgr.go b/internal/config/nsmgr/nsmgr.go
@@ -87,7 +87,11 @@ func (mgr *NamespaceManager) NewPodNamespaces(cfg *PodNamespacesConfig) ([]Names
 	}
 
 	if len(cfg.Sysctls) != 0 {
-		pinnsArgs = append(pinnsArgs, "-s", getSysctlForPinns(cfg.Sysctls))
+		pinnsSysctls, err := getSysctlForPinns(cfg.Sysctls)
+		if err != nil {
+			return nil, errors.Wrapf(err, "invalid sysctl")
+		}
+		pinnsArgs = append(pinnsArgs, "-s", pinnsSysctls)
 	}
 
 	var rootPair idtools.IDPair
@@ -170,14 +174,18 @@ func getMappingsForPinns(mappings []idtools.IDMap) string {
 	return g.String()
 }
 
-func getSysctlForPinns(sysctls map[string]string) string {
-	// this assumes there's no sysctl with a `+` in it
+func getSysctlForPinns(sysctls map[string]string) (string, error) {
+	// This assumes there's no valid sysctl value with a `+` in it
+	// and as such errors if one is found.
 	const pinnsSysctlDelim = "+"
 	g := new(bytes.Buffer)
 	for key, value := range sysctls {
+		if strings.Contains(key, pinnsSysctlDelim) || strings.Contains(value, pinnsSysctlDelim) {
+			return "", errors.Errorf("'%s=%s' is invalid: %s found yet should not be present", key, value, pinnsSysctlDelim)
+		}
 		fmt.Fprintf(g, "'%s=%s'%s", key, value, pinnsSysctlDelim)
 	}
-	return strings.TrimSuffix(g.String(), pinnsSysctlDelim)
+	return strings.TrimSuffix(g.String(), pinnsSysctlDelim), nil
 }
 
 // dirForType returns the sub-directory for that particular NSType
diff --git a/internal/version/version.go b/internal/version/version.go
@@ -21,7 +21,7 @@ import (
 )
 
 // Version is the version of the build.
-const Version = "1.22.2"
+const Version = "1.22.3"
 
 // Variables injected during build-time
 var (
diff --git a/test/pod.bats b/test/pod.bats
@@ -189,6 +189,27 @@ function teardown() {
 	[[ "$output" != *"net.ipv4.ip_forward = 0"* ]]
 }
 
+@test "fail to pass pod sysctl to runtime if invalid value" {
+	if test -n "$CONTAINER_UID_MAPPINGS"; then
+		skip "userNS enabled"
+	fi
+	start_crio
+
+	jq --arg sysctl "1024 65000'+'net.ipv4.ip_forward=0'" \
+	'	  .linux.sysctls = {
+			"net.ipv4.ip_local_port_range": $sysctl,
+		}' "$TESTDATA"/sandbox_config.json > "$TESTDIR"/sandbox.json
+
+	! crictl runp "$TESTDIR"/sandbox.json
+
+	jq --arg sysctl "net.ipv4.ip_local_port_range=1024 65000'+'net.ipv4.ip_forward" \
+	'	  .linux.sysctls = {
+			($sysctl): "0",
+		}' "$TESTDATA"/sandbox_config.json > "$TESTDIR"/sandbox.json
+
+	! crictl runp "$TESTDIR"/sandbox.json
+}
+
 @test "pod stop idempotent" {
 	start_crio
 	pod_id=$(crictl runp "$TESTDATA"/sandbox_config.json)

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ import (`
`21`	`21`	`)`
`22`	`22`
`23`	`23`	`// Version is the version of the build.`
`24`		`-const Version = "1.22.2"`
	`24`	`+const Version = "1.22.3"`
`25`	`25`
`26`	`26`	`// Variables injected during build-time`
`27`	`27`	`var (`