Skip to content

Commit

Permalink
Properly validate cache IDs and sources
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
mheon committed Oct 10, 2024
1 parent 98382f0 commit 873bedd
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 5 deletions.
19 changes: 14 additions & 5 deletions internal/volumes/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,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"
)
Expand Down Expand Up @@ -374,7 +375,11 @@ func GetCacheMount(args []string, _ storage.Store, _ string, additionalMountPoin
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 the cache directory on the host if no image is being used

Expand All @@ -391,11 +396,15 @@ func GetCacheMount(args []string, _ storage.Store, _ string, additionalMountPoin
}

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,
Expand Down
34 changes: 34 additions & 0 deletions tests/bud.bats
Original file line number Diff line number Diff line change
Expand Up @@ -6971,3 +6971,37 @@ _EOF
run_buildah 125 build $WITH_POLICY_JSON ${TEST_SCRATCH_DIR}
expect_output --substring "invalid mount option"
}

@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"
}

0 comments on commit 873bedd

Please sign in to comment.