mz560: anti-hijacking

integration/config.go

@@ -28,6 +28,7 @@ type integrationTestConfig struct {
 	onbuildBaseImage   string
 	onbuildCopyImage   string
 	hardlinkBaseImage  string
+	hijackBaseImage    string
 	serviceAccount     string
 	dockerMajorVersion int
 	gcsClient          *storage.Client

integration/dockerfiles/Dockerfile_test_ignore_path_subtree

@@ -0,0 +1,4 @@
+FROM scratch
+# mz560: in kaniko v1.27.0, when built with --ignore-path=/dest,
+# the file copied into /dest/subdir/ would slip through and end up in the layer.
+COPY context/foo /dest/subdir/file.txt

integration/dockerfiles/Dockerfile_test_issue_mz560

@@ -0,0 +1,46 @@
+# mz560: In kaniko v1.27.0 there is no consistency check for /kaniko binaries.
+# A bug was discovered where tarballs can be constructed to unpack arbitrary files into
+# /kaniko directory. As kaniko is no longer a single-binary application,
+# every RUN runs via tini, this means that that arbitrary payload will be executed.
+# But actually it is way simpler than that, as shown here the same can be achieved with ONBUILD.
+# This sounds bad, but is actually by design. It is not less secure than having an `ONBUILD` per-se.
+# Docker build is RCE as a service, that is the reason why kaniko does run without privileges.
+ARG IMAGE_REPO
+FROM busybox AS base
+# We simulate command injection by hijacking the tini binary
+
+# First we hijack it by adding a hijacked binary to this base image.
+# This base image gets built with docker, so it is part of the image.
+# When kaniko unrolls the filesystem it might already override the binary with our hijacked version.
+COPY --chmod=755 <<tini /kaniko/tini
+#!/bin/sh
+echo WARN HIJACKED
+tini
+
+# Secondly, if that doesn't work we can use `ONBUILD COPY` to force kaniko to override
+# the binary after initial unrolling of the filesystem.
+ONBUILD COPY --chmod=755 <<tini /kaniko/tini
+#!/bin/sh
+echo WARN HIJACKED
+tini
+
+ONBUILD RUN --mount=type=cache,id=hijack,target=/tmp/kaniko \
+    mv /kaniko/tini /dev/null || true \
+    && echo "#!/bin/sh" > /tmp/kaniko/tini \
+    && echo "echo 'WARN HIJACKED'" >> /tmp/kaniko/tini \
+    && chmod +x /tmp/kaniko/tini
+
+ONBUILD RUN --mount=type=cache,id=hijack,target=/kaniko \
+    ls -la /kaniko && cat /kaniko/tini || true
+
+# Thirdly, we can use a `ONBUILD RUN` statement directly
+# here kaniko has no chance to block us as RUN statements
+# are per design an all powerful blackbox.
+ONBUILD RUN mv /kaniko/tini /dev/null || true \
+    && echo "#!/bin/sh" > /kaniko/tini \
+    && echo "echo 'WARN HIJACKED'" >> /kaniko/tini \
+    && chmod +x /kaniko/tini
+
+FROM ${IMAGE_REPO}hijack:latest
+# /kaniko/tini gets implicitly executed here
+RUN ls -la /kaniko && cat /kaniko/tini || true

integration/dockerfiles/Dockerfile_test_prefix_match_only

@@ -0,0 +1,7 @@
+FROM alpine
+# mz560: Simulates apt-key writing temporary key material under /tmp/apt-key-gpghome.
+# The directory itself should appear in the image layer, but files inside it
+# should be excluded because /tmp/apt-key-gpghome is on the ignore list with
+# PrefixMatchOnly=true.
+RUN mkdir /tmp/apt-key-gpghome && \
+    echo "key material" > /tmp/apt-key-gpghome/pubring.gpg

integration/images.go

@@ -19,6 +19,7 @@ package integration
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -122,6 +123,7 @@ var additionalKanikoFlagsMap = map[string][]string{
 	"Dockerfile_test_maintainer":             {"--single-snapshot"},
 	"Dockerfile_test_target":                 {"--target=second"},
 	"Dockerfile_test_snapshotter_ignorelist": {"--use-new-run=true", "-v=trace"},
+	"Dockerfile_test_ignore_path_subtree":    {"--ignore-path=/dest"},
 	"Dockerfile_test_cache":                  {"--cache-copy-layers=true"},
 	"Dockerfile_test_cache_oci":              {"--cache-copy-layers=true"},
 	"Dockerfile_test_cache_install":          {"--cache-copy-layers=true"},
@@ -139,6 +141,10 @@ var additionalKanikoFlagsMap = map[string][]string{
 	"Dockerfile_test_issue_mz529": {"--cleanup"},
 }
 
+var expectErr = map[string]int{
+	"Dockerfile_test_issue_mz560": 1,
+}
+
 // Arguments to diffoci when comparing dockerfiles
 var diffArgsMap = map[string][]string{
 	// /root/.config 0x1c0 0x1ed
@@ -444,11 +450,21 @@ func (d *DockerFileBuilder) BuildImageWithContext(t *testing.T, config *integrat
 
 	kanikoImage := GetKanikoImage(imageRepo, dockerfile)
 	timer = timing.Start(dockerfile + "_kaniko")
-	if _, err := buildKanikoImage(t.Logf, dockerfilesPath, dockerfile, buildArgs, additionalKanikoFlags, kanikoImage,
-		contextDir, gcsBucket, gcsClient, serviceAccount, true); err != nil {
+	defer timing.DefaultRun.Stop(timer)
+	_, err := buildKanikoImage(t.Logf, dockerfilesPath, dockerfile, buildArgs, additionalKanikoFlags, kanikoImage,
+		contextDir, gcsBucket, gcsClient, serviceAccount, true)
+	if expectErr, ok := expectErr[dockerfile]; ok {
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) && exitErr.ExitCode() == expectErr {
+			return nil
+		}
+		if err == nil {
+			return fmt.Errorf("expected exit code %d but command succeeded", expectErr)
+		}
+	}
+	if err != nil {
 		return err
 	}
-	timing.DefaultRun.Stop(timer)
 
 	d.filesBuilt[dockerfile] = struct{}{}
 

integration/integration_test.go

@@ -183,6 +183,12 @@ func buildRequiredImages() error {
 	}, {
 		name:    "Building kaniko image with leftover stuff in the filesystem",
 		command: []string{"docker", "build", "-t", ExecutorImageTainted, "-f", fmt.Sprintf("%s/Dockerfile_test_issue_mz455", dockerfilesPath), "--target", "kaniko", "."},
+	}, {
+		name:    "Building hijack base image",
+		command: []string{"docker", "build", "-t", config.hijackBaseImage, "-f", fmt.Sprintf("%s/Dockerfile_test_issue_mz560", dockerfilesPath), "--target", "base", "."},
+	}, {
+		name:    "Pushing hijack base image",
+		command: []string{"docker", "push", config.hijackBaseImage},
 	}}
 
 	for _, setupCmd := range setupCommands {
@@ -635,6 +641,67 @@ func TestLayers(t *testing.T) {
 	}
 }
 
+// mz560: TestPrefixMatchOnlyIgnoreList verifies that an ignore list entry with
+// PrefixMatchOnly=true excludes files inside the directory from the snapshot
+// but still captures the directory node itself.
+// The canonical example is /tmp/apt-key-gpghome: apt-key writes temporary GPG
+// key files there during a build; they must not end up in the image layer.
+func TestPrefixMatchOnlyIgnoreList(t *testing.T) {
+	dockerfile := "Dockerfile_test_prefix_match_only"
+	buildImage(t, dockerfile, imageBuilder)
+	kanikoImage := GetKanikoImage(config.imageRepo, dockerfile)
+
+	kanikoFiles, err := getLastLayerFiles(kanikoImage)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	const (
+		gpghomeDir  = "tmp/apt-key-gpghome"
+		gpghomeFile = "tmp/apt-key-gpghome/pubring.gpg"
+	)
+
+	var hasDir, hasFile bool
+	for _, f := range kanikoFiles {
+		switch strings.TrimSuffix(f, "/") {
+		case gpghomeDir:
+			hasDir = true
+		case gpghomeFile:
+			hasFile = true
+		}
+	}
+
+	if !hasDir {
+		t.Errorf("expected %s directory to be present in layer (PrefixMatchOnly=true should not exclude the directory itself), got %v", gpghomeDir, kanikoFiles)
+	}
+	if hasFile {
+		t.Errorf("expected %s to be excluded from layer (child of PrefixMatchOnly=true ignore entry), got %v", gpghomeFile, kanikoFiles)
+	}
+}
+
+// mz560: TestIgnorePathSubtree verifies that --ignore-path excludes not just the named
+// path but also any files nested beneath it. The observable difference: with an
+// exact-match-only check (IsInProvidedIgnoreList), a COPY into /dest/subdir/
+// would pass the ResolvePaths input guard and appear in the layer even when
+// --ignore-path=/dest is set. CheckCleanedPathAgainstProvidedIgnoreList (prefix
+// match) catches the nested path and keeps it out of the layer.
+func TestIgnorePathSubtree(t *testing.T) {
+	dockerfile := "Dockerfile_test_ignore_path_subtree"
+	buildImage(t, dockerfile, imageBuilder)
+	kanikoImage := GetKanikoImage(config.imageRepo, dockerfile)
+
+	kanikoFiles, err := getLastLayerFiles(kanikoImage)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, f := range kanikoFiles {
+		if strings.HasPrefix(strings.TrimSuffix(f, "/"), "dest/") {
+			t.Errorf("file %s should be excluded from layer because it is under --ignore-path=/dest, got layer contents: %v", f, kanikoFiles)
+		}
+	}
+}
+
 func TestReplaceFolderWithFileOrLink(t *testing.T) {
 	dockerfiles := []string{"TestReplaceFolderWithFile", "TestReplaceFolderWithLink"}
 	for _, dockerfile := range dockerfiles {
@@ -1290,6 +1357,7 @@ func initIntegrationTestConfig() *integrationTestConfig {
 	c.onbuildBaseImage = c.imageRepo + "onbuild-base:latest"
 	c.onbuildCopyImage = c.imageRepo + "onbuild-copy:latest"
 	c.hardlinkBaseImage = c.imageRepo + "hardlink-base:latest"
+	c.hijackBaseImage = c.imageRepo + "hijack:latest"
 	return &c
 }
 

pkg/commands/copy.go

@@ -150,6 +150,10 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 		if err != nil {
 			return fmt.Errorf("find destination path: %w", err)
 		}
+		if util.CheckIgnoreList(destPath) {
+			logrus.Debugf("Skipping copy for ignored path: %s", destPath)
+			return nil
+		}
 
 		srcFile := strings.NewReader(src.Data)
 		err = util.CreateFile(destPath, srcFile, chmod, uint32(uid), uint32(gid))

pkg/commands/run.go

@@ -77,6 +77,7 @@ func runCommandWithFlags(config *v1.Config, buildArgs *dockerfile.BuildArgs, cmd
 			switch {
 			// https://docs.docker.com/reference/dockerfile/#run---mounttypecache
 			case m.Type == instructions.MountTypeCache:
+				assertProtectedKanikoDir(m.Target)
 				cacheId := m.CacheID
 				if cacheId == "" {
 					cacheId = filepath.Clean(m.Target)
@@ -177,6 +178,7 @@ func runCommandWithFlags(config *v1.Config, buildArgs *dockerfile.BuildArgs, cmd
 					if target == "" {
 						target = fmt.Sprintf("/run/secrets/%s", secretId)
 					}
+					assertProtectedKanikoDir(target)
 					parent := filepath.Dir(target)
 					created, err := ensureDir(parent)
 					if err != nil {
@@ -514,6 +516,18 @@ func ensureDir(target string) (string, error) {
 	return firstCreated, nil
 }
 
+func assertProtectedKanikoDir(target string) {
+	wl := []util.IgnoreListEntry{
+		{
+			Path:            kConfig.KanikoDir,
+			PrefixMatchOnly: false,
+		},
+	}
+	if util.CheckCleanedPathAgainstProvidedIgnoreList(target, wl) {
+		logrus.Fatalf("mount option targetting protected KanikoDir (%s), this could be indicative of a hijacking attempt", kConfig.KanikoDir)
+	}
+}
+
 func assignIfNil(dst *error, fn func() error) {
 	if err := fn(); err != nil && *dst == nil {
 		*dst = err

pkg/executor/build.go

@@ -82,7 +82,7 @@ func makeSnapshotter(opts *config.KanikoOptions) (*snapshot.Snapshotter, error)
 		return nil, err
 	}
 	l := snapshot.NewLayeredMap(hasher)
-	return snapshot.NewSnapshotter(l, config.RootDir), nil
+	return snapshot.NewSnapshotter(l, config.RootDir, util.IgnoreList()), nil
 }
 
 // newStageBuilder returns a new type stageBuilder which contains all the information required to build the stage
@@ -338,6 +338,13 @@ func (s *stageBuilder) build(compositeKey CompositeCache, opts *config.KanikoOpt
 		timing.DefaultRun.Stop(t)
 		initSnapshotTaken = true
 	}
+	kanikoDirSnapshotter := snapshot.NewSnapshotter(snapshot.NewLayeredMap(util.Hasher()), config.KanikoDir, []util.IgnoreListEntry{
+		{
+			Path:            config.KanikoCacheDir,
+			PrefixMatchOnly: true,
+		},
+	})
+	kanikoDirSnapshotter.Init()
 
 	cacheGroup := errgroup.Group{}
 	for index, command := range s.cmds {
@@ -381,9 +388,27 @@ func (s *stageBuilder) build(compositeKey CompositeCache, opts *config.KanikoOpt
 			initSnapshotTaken = true
 		}
 
+		if !command.ProvidesFilesToSnapshot() {
+			_, _, err = kanikoDirSnapshotter.ScanFullFilesystem()
+			if err != nil {
+				return err
+			}
+		}
+
 		if err := command.ExecuteCommand(&s.cf.Config, s.args); err != nil {
 			return fmt.Errorf("failed to execute command: %w", err)
 		}
+
+		if !command.ProvidesFilesToSnapshot() {
+			add, del, err := kanikoDirSnapshotter.ScanFullFilesystem()
+			if err != nil {
+				return err
+			}
+			if len(add) > 0 || len(del) > 0 {
+				logrus.Fatalf("We noticed a diff in the KanikoDir (%s), this could be indicative of a hijacking attempt", config.KanikoDir)
+			}
+		}
+
 		files = command.FilesToSnapshot()
 		timing.DefaultRun.Stop(t)
 

pkg/filesystem/resolve.go

@@ -42,7 +42,7 @@ func ResolvePaths(paths []string, wl []util.IgnoreListEntry) (pathsToAdd []strin
 
 	for _, f := range paths {
 		// If the given path is part of the ignorelist ignore it
-		if util.IsInProvidedIgnoreList(f, wl) {
+		if util.CheckCleanedPathAgainstProvidedIgnoreList(filepath.Clean(f), wl) {
 			logrus.Debugf("Path %s is in list to ignore, ignoring it", f)
 			continue
 		}

pkg/filesystem/resolve_test.go

@@ -118,7 +118,7 @@ func Test_ResolvePaths(t *testing.T) {
 					link := filepath.Join(dir, "link", f)
 					inputFiles = append(inputFiles, link)
 
-					if util.IsInProvidedIgnoreList(link, wl) {
+					if util.CheckCleanedPathAgainstProvidedIgnoreList(filepath.Clean(link), wl) {
 						t.Logf("skipping %s", link)
 						continue
 					}
@@ -127,7 +127,7 @@ func Test_ResolvePaths(t *testing.T) {
 
 					target := filepath.Join(dir, "target", f)
 
-					if util.IsInProvidedIgnoreList(target, wl) {
+					if util.CheckCleanedPathAgainstProvidedIgnoreList(filepath.Clean(target), wl) {
 						t.Logf("skipping %s", target)
 						continue
 					}