120 lines
4.7 KiB
Diff
120 lines
4.7 KiB
Diff
From 7efe216bed829677dd10fa960f8be2459fef752d 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/3] 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 | 20 +++++++++++++++-----
|
|
tests/bud.bats | 34 ++++++++++++++++++++++++++++++++++
|
|
2 files changed, 49 insertions(+), 5 deletions(-)
|
|
|
|
diff --git a/internal/volumes/volumes.go b/internal/volumes/volumes.go
|
|
index ccb81b27ed46..037586f704b6 100644
|
|
--- a/internal/volumes/volumes.go
|
|
+++ b/internal/volumes/volumes.go
|
|
@@ -11,6 +11,7 @@ import (
|
|
|
|
"errors"
|
|
|
|
+ "github.com/containers/buildah/copier"
|
|
"github.com/containers/buildah/define"
|
|
"github.com/containers/buildah/internal"
|
|
internalParse "github.com/containers/buildah/internal/parse"
|
|
@@ -22,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"
|
|
)
|
|
@@ -368,7 +370,11 @@ 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
|
|
|
|
@@ -385,11 +391,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 878a1597a0a7..c2038b0377bc 100644
|
|
--- a/tests/bud.bats
|
|
+++ b/tests/bud.bats
|
|
@@ -6527,3 +6527,37 @@ _EOF
|
|
expect_output --substring "localhost/foo/bar"
|
|
expect_output --substring "localhost/bar"
|
|
}
|
|
+
|
|
+@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
|
|
|