Merge pull request #29027 from Luap99/v5.8

[v5.8] fix image host env leak

pkg/specgen/generate/container.go

@@ -123,6 +123,18 @@ func applyHealthCheckOverrides(s *specgen.SpecGenerator, healthCheckFromImage *m
 	return nil
 }
 
+func ParseImageEnvs(imageEnvs []string) (map[string]string, error) {
+	envs := make(map[string]string, len(imageEnvs))
+	for _, env := range imageEnvs {
+		key, val, hasValue := strings.Cut(env, "=")
+		if !hasValue || key == "" {
+			return nil, fmt.Errorf("invalid image env variable %q", env)
+		}
+		envs[key] = val
+	}
+	return envs, nil
+}
+
 // Fill any missing parts of the spec generator (e.g. from the image).
 // Returns a set of warnings or any fatal error that occurred.
 func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerator) ([]string, error) {
@@ -168,15 +180,14 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 	if err != nil {
 		return nil, fmt.Errorf("parsing fields in containers.conf: %w", err)
 	}
-	var envs map[string]string
 
 	// Image Environment defaults
 	if inspectData != nil {
 		// Image envs from the image if they don't exist
 		// already, overriding the default environments
-		envs, err = envLib.ParseSlice(inspectData.Config.Env)
+		envs, err := ParseImageEnvs(inspectData.Config.Env)
 		if err != nil {
-			return nil, fmt.Errorf("env fields from image failed to parse: %w", err)
+			return nil, err
 		}
 		defaultEnvs = envLib.Join(envLib.DefaultEnvVariables(), envLib.Join(defaultEnvs, envs))
 	}

pkg/specgen/generate/container_test.go

@@ -0,0 +1,66 @@
+//go:build !remote && (linux || freebsd)
+
+package generate
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseImageEnvs(t *testing.T) {
+	tests := []struct {
+		name      string
+		imageEnvs []string
+		want      map[string]string
+		wantErr   bool
+	}{
+		{
+			name: "no env",
+			want: map[string]string{},
+		},
+		{
+			name:      "single env",
+			imageEnvs: []string{"TEST=1"},
+			want: map[string]string{
+				"TEST": "1",
+			},
+		},
+		{
+			name:      "multiple envs",
+			imageEnvs: []string{"TEST=1", "ABC=b", "PATH=/bin"},
+			want: map[string]string{
+				"TEST": "1",
+				"ABC":  "b",
+				"PATH": "/bin",
+			},
+		},
+		{
+			name:      "invalid env without value",
+			imageEnvs: []string{"HOST"},
+			wantErr:   true,
+		},
+		{
+			name:      "invalid env asterisk",
+			imageEnvs: []string{"*"},
+			wantErr:   true,
+		},
+		{
+			name:      "invalid env no key",
+			imageEnvs: []string{"=123"},
+			wantErr:   true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ParseImageEnvs(tt.imageEnvs)
+			if tt.wantErr {
+				require.Error(t, err)
+				return
+			}
+
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

pkg/specgen/generate/kube/kube.go

@@ -479,10 +479,9 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
 	s.Annotations[define.KubeHealthCheckAnnotation] = "true"
 
 	// Environment Variables
-	envs := map[string]string{}
-	for _, env := range imageData.Config.Env {
-		key, val, _ := strings.Cut(env, "=")
-		envs[key] = val
+	envs, err := generate.ParseImageEnvs(imageData.Config.Env)
+	if err != nil {
+		return nil, err
 	}
 
 	// Process envFrom first (lower precedence)

test/system/030-run.bats

@@ -1919,4 +1919,56 @@ EOF
     run_podman rmi $image
 }
 
+@test "podman run host env leak" {
+    # We need to create a invalid image config env value without "="
+    skopeo copy containers-storage:$IMAGE dir:$PODMAN_TMPDIR
+    config_digest=$(jq -r .config.digest $PODMAN_TMPDIR/manifest.json)
+    plain_digest=${config_digest#*:}
+    newfile="$PODMAN_TMPDIR/newfile"
+    # Append bad env to existing image envs
+    jq  '.config.Env += ["HOST*"]' $PODMAN_TMPDIR/$plain_digest >$newfile
+    # Get new digest and size so we can update the manifest
+    newdigest="$(sha256sum $newfile | cut -d" " -f 1)"
+    size=$(stat -c %s $newfile)
+    mv $newfile $PODMAN_TMPDIR/$newdigest
+    jq ".config.digest = \"sha256:$newdigest\" | .config.size=$size" $PODMAN_TMPDIR/manifest.json > $PODMAN_TMPDIR/manifest.json.new
+    mv $PODMAN_TMPDIR/manifest.json.new $PODMAN_TMPDIR/manifest.json
+
+    image="localhost/envimage:123"
+    skopeo copy dir:$PODMAN_TMPDIR containers-storage:$image
+
+    run_podman image inspect $image --format '{{.Config.Env}}'
+    assert "$output" =~ "HOST\*" "invalid env in image"
+
+    HOSTENV=123 run_podman 125 run --rm $image printenv HOSTENV
+    assert "$output" =~ 'invalid image env variable "HOST\*"' "Host env leak from image env on podman run"
+
+    podname="p-$(safename)"
+    ctrname="c-$(safename)"
+
+    fname="$PODMAN_TMPDIR/kube_$(safename).yaml"
+    echo "
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    app: test
+  name: $podname
+spec:
+  restartPolicy: Never
+  containers:
+    - name: $ctrname
+      image: $image
+      command:
+      - printenv
+      - HOSTENV
+" > $fname
+
+    run_podman 125 kube play $fname
+    assert "$output" =~ 'invalid image env variable "HOST\*"' "Host env leak from image env on kube play"
+
+    run_podman pod rm $podname
+    run_podman rmi $image
+}
+
 # vim: filetype=sh