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

haircommander · web-flow · commit c12bb210e988 · 2022-03-15T10:11:09.000-04:00
[1.19] config/sysctl: fail if there is a + in the value
diff --git a/internal/lib/sandbox/namespaces_linux.go b/internal/lib/sandbox/namespaces_linux.go
@@ -71,7 +71,11 @@ func pinNamespaces(nsTypes []NSType, cfg *config.Config, sysctls map[string]stri
 	}
 
 	if len(sysctls) != 0 {
-		pinnsArgs = append(pinnsArgs, "-s", getSysctlForPinns(sysctls))
+		pinnsSysctls, err := getSysctlForPinns(sysctls)
+		if err != nil {
+			return nil, errors.Wrapf(err, "invalid sysctl")
+		}
+		pinnsArgs = append(pinnsArgs, "-s", pinnsSysctls)
 	}
 
 	type namespaceInfo struct {
@@ -128,14 +132,18 @@ func pinNamespaces(nsTypes []NSType, cfg *config.Config, sysctls map[string]stri
 	return returnedNamespaces, nil
 }
 
-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
 }
 
 // getNamespace takes a path, checks if it is a namespace, and if so
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.19.5"
+const Version = "1.19.6"
 
 // Variables injected during build-time
 var (
diff --git a/test/pod.bats b/test/pod.bats
@@ -264,6 +264,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
 	run 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.19.5"`
	`24`	`+const Version = "1.19.6"`
`25`	`25`
`26`	`26`	`// Variables injected during build-time`
`27`	`27`	`var (`