From b48b2e689270ee7cc8c13464cbae1b5405fcb901 Mon Sep 17 00:00:00 2001
From: Matt Heon <mheon@redhat.com>
Date: Wed, 9 Oct 2024 15:23:03 -0400
Subject: [PATCH 3/4] Properly validate cache IDs and sources

The `--mount type=cache` argument to the `RUN` instruction in
Dockerfiles was using `filepath.Join` on user input, allowing
crafted paths to be used to gain access to paths on the host,
when the command should normally be limited only to Buildah;s own
cache and context directories. Switch to `filepath.SecureJoin` to
resolve the issue.

Fixes CVE-2024-9675

Signed-off-by: Matt Heon <mheon@redhat.com>
Signed-off-by: Danish Prakash <contact@danishpraka.sh>
---
 internal/volumes/volumes.go | 22 +++++++++++++++-------
 tests/bud.bats              | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/internal/volumes/volumes.go b/internal/volumes/volumes.go
index da6b768fdc21..2dd04d9c32a4 100644
--- a/internal/volumes/volumes.go
+++ b/internal/volumes/volumes.go
@@ -23,6 +23,7 @@ import (
 	"github.com/containers/storage/pkg/idtools"
 	"github.com/containers/storage/pkg/lockfile"
 	"github.com/containers/storage/pkg/unshare"
+	digest "github.com/opencontainers/go-digest"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	selinux "github.com/opencontainers/selinux/go-selinux"
 )
@@ -374,10 +375,13 @@ func GetCacheMount(args []string, store storage.Store, imageMountLabel string, a
 			return newMount, nil, fmt.Errorf("no stage found with name %s", fromStage)
 		}
 		// path should be /contextDir/specified path
-		newMount.Source = filepath.Join(mountPoint, filepath.Clean(string(filepath.Separator)+newMount.Source))
+		evaluated, err := copier.Eval(mountPoint, string(filepath.Separator)+newMount.Source, copier.EvalOptions{})
+		if err != nil {
+			return newMount, nil, err
+		}
+		newMount.Source = evaluated
 	} else {
-		// we need to create cache on host if no image is being used
-
+		// we need to create the cache directory on the host if no image is being used
 		// since type is cache and cache can be reused by consecutive builds
 		// create a common cache directory, which persists on hosts within temp lifecycle
 		// add subdirectory if specified
@@ -391,11 +395,15 @@ func GetCacheMount(args []string, store storage.Store, imageMountLabel string, a
 		}
 
 		if id != "" {
-			newMount.Source = filepath.Join(cacheParent, filepath.Clean(id))
-			buildahLockFilesDir = filepath.Join(BuildahCacheLockfileDir, filepath.Clean(id))
+			// Don't let the user control where we place the directory.
+			dirID := digest.FromString(id).Encoded()[:16]
+			newMount.Source = filepath.Join(cacheParent, dirID)
+			buildahLockFilesDir = filepath.Join(BuildahCacheLockfileDir, dirID)
 		} else {
-			newMount.Source = filepath.Join(cacheParent, filepath.Clean(newMount.Destination))
-			buildahLockFilesDir = filepath.Join(BuildahCacheLockfileDir, filepath.Clean(newMount.Destination))
+			// Don't let the user control where we place the directory.
+			dirID := digest.FromString(newMount.Destination).Encoded()[:16]
+			newMount.Source = filepath.Join(cacheParent, dirID)
+			buildahLockFilesDir = filepath.Join(BuildahCacheLockfileDir, dirID)
 		}
 		idPair := idtools.IDPair{
 			UID: uid,
diff --git a/tests/bud.bats b/tests/bud.bats
index b6982bbc0ee4..e28fc3dd8add 100644
--- a/tests/bud.bats
+++ b/tests/bud.bats
@@ -6659,3 +6659,37 @@ _EOF
   assert "$status" -eq 2 "exit code from ls"
   expect_output --substring "No such file or directory"
 }
+
+@test "build-check-cve-2024-9675" {
+  _prefetch alpine
+
+  touch ${TEST_SCRATCH_DIR}/file.txt
+
+  cat > ${TEST_SCRATCH_DIR}/Containerfile <<EOF
+FROM alpine
+RUN --mount=type=cache,id=../../../../../../../../../../../$TEST_SCRATCH_DIR,target=/var/tmp \
+ls -l /var/tmp && cat /var/tmp/file.txt
+EOF
+
+  run_buildah 1 build --no-cache ${TEST_SCRATCH_DIR}
+  expect_output --substring "cat: can't open '/var/tmp/file.txt': No such file or directory"
+
+  cat > ${TEST_SCRATCH_DIR}/Containerfile <<EOF
+FROM alpine
+RUN --mount=type=cache,source=../../../../../../../../../../../$TEST_SCRATCH_DIR,target=/var/tmp \
+ls -l /var/tmp && cat /var/tmp/file.txt
+EOF
+
+  run_buildah 1 build --no-cache ${TEST_SCRATCH_DIR}
+  expect_output --substring "cat: can't open '/var/tmp/file.txt': No such file or directory"
+
+  mkdir ${TEST_SCRATCH_DIR}/cve20249675
+  cat > ${TEST_SCRATCH_DIR}/cve20249675/Containerfile <<EOF
+FROM alpine
+RUN --mount=type=cache,from=testbuild,source=../,target=/var/tmp \
+ls -l /var/tmp && cat /var/tmp/file.txt
+EOF
+
+  run_buildah 1 build --security-opt label=disable --build-context testbuild=${TEST_SCRATCH_DIR}/cve20249675/ --no-cache ${TEST_SCRATCH_DIR}/cve20249675/
+  expect_output --substring "cat: can't open '/var/tmp/file.txt': No such file or directory"
+}
-- 
2.46.0