Skip to content

Commit b845b72

Browse files
Honny1Honny1
committed
Reject symlinked Containerfile and ignore files escaping build context
Fixes: #6861 Fixes: podman-container-tools/podman#28749 Signed-off-by: Jan Rodák <hony.com@seznam.cz>
1 parent fc53f95 commit b845b72

4 files changed

Lines changed: 273 additions & 37 deletions

File tree

‎pkg/parse/parse.go‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,17 +1403,37 @@ func ContainerIgnoreFile(contextDir, path string, containerFiles []string) ([]st
14031403
excludes, err := imagebuilder.ParseIgnore(path)
14041404
return excludes, path, err
14051405
}
1406-
// If path was not supplied give priority to `<containerfile>.containerignore` first.
1406+
// If path was not supplied, look for `<containerfile>.containerignore` and
1407+
// `<containerfile>.dockerignore`. When both exist, `.dockerignore` wins.
1408+
// Use securejoin.SecureJoin to prevent symlinks from escaping.
1409+
// When the containerfile is inside contextDir we resolve relative to
1410+
// contextDir. When it is outside (e.g. overlay-mounted context or -f
1411+
// pointing elsewhere) we resolve relative to the containerfile's parent
1412+
// directory so symlink escapes are still blocked.
14071413
for _, containerfile := range containerFiles {
14081414
if !filepath.IsAbs(containerfile) {
14091415
containerfile = filepath.Join(contextDir, containerfile)
14101416
}
1411-
containerfileIgnore := ""
1412-
if err := fileutils.Exists(containerfile + ".containerignore"); err == nil {
1413-
containerfileIgnore = containerfile + ".containerignore"
1417+
cleanPath := filepath.Clean(containerfile)
1418+
relPath, relErr := filepath.Rel(contextDir, cleanPath)
1419+
insideContext := relErr == nil && relPath != ".." && !strings.HasPrefix(relPath, ".."+string(filepath.Separator))
1420+
var joinRoot, joinBase string
1421+
if insideContext {
1422+
joinRoot = contextDir
1423+
joinBase = relPath
1424+
} else {
1425+
joinRoot = filepath.Dir(cleanPath)
1426+
joinBase = filepath.Base(cleanPath)
14141427
}
1415-
if err := fileutils.Exists(containerfile + ".dockerignore"); err == nil {
1416-
containerfileIgnore = containerfile + ".dockerignore"
1428+
containerfileIgnore := ""
1429+
for _, suffix := range []string{".containerignore", ".dockerignore"} {
1430+
candidate, err := securejoin.SecureJoin(joinRoot, joinBase+suffix)
1431+
if err != nil {
1432+
continue
1433+
}
1434+
if err := fileutils.Exists(candidate); err == nil {
1435+
containerfileIgnore = candidate
1436+
}
14171437
}
14181438
if containerfileIgnore != "" {
14191439
excludes, err := imagebuilder.ParseIgnore(containerfileIgnore)

‎pkg/util/util.go‎

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -41,42 +41,68 @@ func MirrorToTempFileIfPathIsDescriptor(file string) (string, bool) {
4141
}
4242

4343
// DiscoverContainerfile tries to find a Containerfile or a Dockerfile within the provided `path`.
44+
// The path may be a directory (in which case Containerfile/Dockerfile is searched inside it)
45+
// or a direct path to a container file.
46+
//
47+
// Symlinked Containerfile/Dockerfile entries are only used when their real
48+
// target stays inside the build context directory. Symlinks that resolve
49+
// outside the context or are dangling are skipped.
4450
func DiscoverContainerfile(path string) (foundCtrFile string, err error) {
45-
// Test for existence of the file
46-
target, err := os.Stat(path)
51+
path, err = filepath.Abs(path)
4752
if err != nil {
4853
return "", fmt.Errorf("discovering Containerfile: %w", err)
4954
}
5055

51-
switch mode := target.Mode(); {
52-
case mode.IsDir():
53-
// If the path is a real directory, we assume a Containerfile or a Dockerfile within it
54-
ctrfile := filepath.Join(path, "Containerfile")
55-
56-
// Test for existence of the Containerfile file
57-
file, err := os.Stat(ctrfile)
58-
if err != nil {
59-
// See if we have a Dockerfile within it
60-
ctrfile = filepath.Join(path, "Dockerfile")
56+
target, err := os.Lstat(path)
57+
if err != nil {
58+
return "", fmt.Errorf("discovering Containerfile: %w", err)
59+
}
6160

62-
// Test for existence of the Dockerfile file
63-
file, err = os.Stat(ctrfile)
64-
if err != nil {
65-
return "", fmt.Errorf("cannot find Containerfile or Dockerfile in context directory: %w", err)
61+
switch {
62+
case target.IsDir():
63+
for _, name := range []string{"Containerfile", "Dockerfile"} {
64+
ctrfile := filepath.Join(path, name)
65+
if isRegularFileInContext(path, ctrfile) {
66+
return ctrfile, nil
6667
}
6768
}
69+
return "", fmt.Errorf("cannot find Containerfile or Dockerfile in context directory")
6870

69-
// The file exists, now verify the correct mode
70-
if mode := file.Mode(); mode.IsRegular() {
71-
foundCtrFile = ctrfile
72-
} else {
73-
return "", fmt.Errorf("assumed Containerfile %q is not a file", ctrfile)
71+
case target.Mode().IsRegular():
72+
return path, nil
73+
74+
case target.Mode()&os.ModeSymlink != 0:
75+
if isRegularFileInContext(filepath.Dir(path), path) {
76+
return path, nil
7477
}
78+
return "", fmt.Errorf("assumed Containerfile %q is not a file", path)
7579

76-
case mode.IsRegular():
77-
// If the context dir is a file, we assume this as Containerfile
78-
foundCtrFile = path
80+
default:
81+
return "", fmt.Errorf("assumed Containerfile %q is not a file", path)
7982
}
83+
}
8084

81-
return foundCtrFile, nil
85+
// isRegularFileInContext returns true if path is a regular file (or a symlink
86+
// to one) whose real target is inside contextDir.
87+
func isRegularFileInContext(contextDir, path string) bool {
88+
resolved, err := filepath.EvalSymlinks(path)
89+
if err != nil {
90+
return false
91+
}
92+
cleanContext, err := filepath.EvalSymlinks(contextDir)
93+
if err != nil {
94+
return false
95+
}
96+
rel, err := filepath.Rel(cleanContext, resolved)
97+
if err != nil {
98+
return false
99+
}
100+
if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
101+
return false
102+
}
103+
fi, err := os.Stat(path)
104+
if err != nil {
105+
return false
106+
}
107+
return fi.Mode().IsRegular()
82108
}

‎pkg/util/util_test.go‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,21 @@
11
package util //nolint:revive,nolintlint
22

33
import (
4+
"os"
5+
"path/filepath"
46
"testing"
57

68
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
710
)
811

12+
func absPath(t *testing.T, rel string) string {
13+
t.Helper()
14+
p, err := filepath.Abs(rel)
15+
require.NoError(t, err)
16+
return p
17+
}
18+
919
func TestDiscoverContainerfile(t *testing.T) {
1020
t.Parallel()
1121
_, err := DiscoverContainerfile("./bogus")
@@ -16,17 +26,47 @@ func TestDiscoverContainerfile(t *testing.T) {
1626

1727
name, err := DiscoverContainerfile("test/test1/Dockerfile")
1828
assert.Nil(t, err)
19-
assert.Equal(t, name, "test/test1/Dockerfile")
29+
assert.Equal(t, absPath(t, "test/test1/Dockerfile"), name)
2030

2131
name, err = DiscoverContainerfile("test/test1/Containerfile")
2232
assert.Nil(t, err)
23-
assert.Equal(t, name, "test/test1/Containerfile")
33+
assert.Equal(t, absPath(t, "test/test1/Containerfile"), name)
2434

2535
name, err = DiscoverContainerfile("test/test1")
2636
assert.Nil(t, err)
27-
assert.Equal(t, name, "test/test1/Containerfile")
37+
assert.Equal(t, absPath(t, "test/test1/Containerfile"), name)
2838

2939
name, err = DiscoverContainerfile("test/test2")
3040
assert.Nil(t, err)
31-
assert.Equal(t, name, "test/test2/Dockerfile")
41+
assert.Equal(t, absPath(t, "test/test2/Dockerfile"), name)
42+
}
43+
44+
func TestDiscoverContainerfileRejectsSymlinkOutsideContext(t *testing.T) {
45+
t.Parallel()
46+
tmpDir := t.TempDir()
47+
48+
secretFile := filepath.Join(tmpDir, "secret-Containerfile")
49+
require.NoError(t, os.WriteFile(secretFile, []byte("FROM scratch\n"), 0o644))
50+
51+
contextDir := filepath.Join(tmpDir, "context")
52+
require.NoError(t, os.Mkdir(contextDir, 0o755))
53+
require.NoError(t, os.Symlink(secretFile, filepath.Join(contextDir, "Containerfile")))
54+
55+
_, err := DiscoverContainerfile(contextDir)
56+
assert.Error(t, err)
57+
assert.Contains(t, err.Error(), "cannot find Containerfile or Dockerfile")
58+
}
59+
60+
func TestDiscoverContainerfileAcceptsSymlinkInsideContext(t *testing.T) {
61+
t.Parallel()
62+
contextDir := t.TempDir()
63+
64+
subdir := filepath.Join(contextDir, "subdir")
65+
require.NoError(t, os.Mkdir(subdir, 0o755))
66+
require.NoError(t, os.WriteFile(filepath.Join(subdir, "Containerfile.real"), []byte("FROM scratch\n"), 0o644))
67+
require.NoError(t, os.Symlink(filepath.Join("subdir", "Containerfile.real"), filepath.Join(contextDir, "Containerfile")))
68+
69+
name, err := DiscoverContainerfile(contextDir)
70+
require.NoError(t, err)
71+
assert.Equal(t, filepath.Join(contextDir, "Containerfile"), name)
3272
}

‎tests/bud.bats‎

Lines changed: 152 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4994,13 +4994,13 @@ _EOF
49944994
@test "bud without any arguments should fail when no Dockerfile exists" {
49954995
cd $TEST_SCRATCH_DIR
49964996
run_buildah 125 build --signature-policy ${TEST_SOURCES}/policy.json
4997-
expect_output --substring "no such file or directory"
4997+
expect_output --substring "cannot find Containerfile or Dockerfile"
49984998
}
49994999

50005000
@test "bud with specified context should fail if directory contains no Dockerfile" {
50015001
mkdir -p $TEST_SCRATCH_DIR/empty-dir
50025002
run_buildah 125 build $WITH_POLICY_JSON "$TEST_SCRATCH_DIR"/empty-dir
5003-
expect_output --substring "no such file or directory"
5003+
expect_output --substring "cannot find Containerfile or Dockerfile"
50045004
}
50055005

50065006
@test "bud with specified context should fail if Dockerfile in context directory is actually a file" {
@@ -8294,6 +8294,156 @@ srv.serve_forever()
82948294
assert "$output" = "ORIGINAL_CONTENT"
82958295
}
82968296

8297+
# https://github.com/podman-container-tools/buildah/issues/6861
8298+
@test "bud with local context symlinked Containerfile outside context" {
8299+
_prefetch alpine
8300+
8301+
local secretfile=${TEST_SCRATCH_DIR}/secretfile
8302+
cat > ${secretfile} << _EOF
8303+
FROM alpine
8304+
RUN echo SECRETHOSTCONTENT
8305+
_EOF
8306+
8307+
local contextdir=${TEST_SCRATCH_DIR}/dir2
8308+
mkdir -p ${contextdir}
8309+
ln -s ${secretfile} ${contextdir}/Containerfile
8310+
8311+
run_buildah 125 build $WITH_POLICY_JSON ${contextdir}
8312+
assert "$output" !~ "SECRETHOSTCONTENT"
8313+
}
8314+
8315+
# https://github.com/podman-container-tools/buildah/issues/6861
8316+
@test "bud with local context symlinked Containerfile within context" {
8317+
_prefetch alpine
8318+
8319+
local contextdir=${TEST_SCRATCH_DIR}/context
8320+
mkdir -p ${contextdir}/subdir
8321+
cat > ${contextdir}/subdir/Containerfile.real << _EOF
8322+
FROM alpine
8323+
RUN echo symlink-within-context-works
8324+
_EOF
8325+
ln -s subdir/Containerfile.real ${contextdir}/Containerfile
8326+
8327+
run_buildah build $WITH_POLICY_JSON ${contextdir}
8328+
assert "$output" =~ "symlink-within-context-works"
8329+
}
8330+
8331+
# https://github.com/podman-container-tools/buildah/issues/6861
8332+
@test "bud accepts symlinked Dockerfile traversing outside then back into context" {
8333+
_prefetch alpine
8334+
8335+
local contextdir=${TEST_SCRATCH_DIR}/context
8336+
mkdir -p ${contextdir}
8337+
cat > ${contextdir}/file << _EOF
8338+
FROM alpine
8339+
RUN echo traversal-back-inside-works
8340+
_EOF
8341+
# Symlink path goes ../context/file -- traverses outside but real target resolves inside.
8342+
(cd ${contextdir} && ln -s ../context/file Dockerfile)
8343+
8344+
run_buildah build $WITH_POLICY_JSON ${contextdir}
8345+
assert "$output" =~ "traversal-back-inside-works"
8346+
}
8347+
8348+
# https://github.com/podman-container-tools/buildah/issues/6861
8349+
@test "bud rejects symlinked Dockerfile to non-existent outside target" {
8350+
_prefetch alpine
8351+
8352+
local contextdir=${TEST_SCRATCH_DIR}/context
8353+
mkdir -p ${contextdir}
8354+
cat > ${contextdir}/file << _EOF
8355+
FROM alpine
8356+
RUN echo SHOULD-NOT-RUN
8357+
_EOF
8358+
# Symlink to ../file -- target does not exist on the filesystem.
8359+
(cd ${contextdir} && ln -s ../file Dockerfile)
8360+
8361+
run_buildah 125 build $WITH_POLICY_JSON ${contextdir}
8362+
expect_output --substring "cannot find Containerfile or Dockerfile"
8363+
assert "$output" !~ "SHOULD-NOT-RUN"
8364+
}
8365+
8366+
# https://github.com/podman-container-tools/buildah/issues/6861
8367+
@test "bud with stdin tar context rejects symlinked Dockerfile pointing outside temp dir" {
8368+
local targetfile=${TEST_SCRATCH_DIR}/targetfile
8369+
echo "SECRET_CONTENT" > ${targetfile}
8370+
8371+
local tarsrc=${TEST_SCRATCH_DIR}/tarsrc
8372+
mkdir -p ${tarsrc}
8373+
ln -s ${targetfile} ${tarsrc}/Dockerfile
8374+
local context_tar=${TEST_SCRATCH_DIR}/context.tar
8375+
tar -cf ${context_tar} -C ${tarsrc} Dockerfile
8376+
8377+
run_buildah 125 build $WITH_POLICY_JSON - < ${context_tar}
8378+
assert "$output" =~ "cannot find Containerfile or Dockerfile"
8379+
assert "$output" !~ "SECRET_CONTENT"
8380+
}
8381+
8382+
# https://github.com/podman-container-tools/buildah/issues/6861
8383+
@test "bud with http tar context rejects symlinked Dockerfile pointing outside temp dir" {
8384+
local targetfile=${TEST_SCRATCH_DIR}/targetfile
8385+
echo "SECRET_CONTENT" > ${targetfile}
8386+
8387+
local tarsrc=${TEST_SCRATCH_DIR}/tarsrc
8388+
mkdir -p ${tarsrc}
8389+
ln -s ${targetfile} ${tarsrc}/Dockerfile
8390+
local contentdir=${TEST_SCRATCH_DIR}/content
8391+
mkdir -p ${contentdir}
8392+
tar -cf ${contentdir}/context.tar -C ${tarsrc} Dockerfile
8393+
starthttpd ${contentdir}
8394+
8395+
run_buildah 125 build $WITH_POLICY_JSON http://0.0.0.0:${HTTP_SERVER_PORT}/context.tar
8396+
assert "$output" =~ "cannot find Containerfile or Dockerfile"
8397+
assert "$output" !~ "SECRET_CONTENT"
8398+
}
8399+
8400+
# https://github.com/podman-container-tools/buildah/issues/6861
8401+
# https://github.com/podman-container-tools/podman/issues/28749
8402+
@test "bud does not follow symlinked dockerignore outside context" {
8403+
_prefetch alpine
8404+
8405+
# dir/
8406+
# ign <- ignore file outside context (excludes "file")
8407+
# context/
8408+
# Dockerfile
8409+
# file
8410+
# Dockerfile.dockerignore -> ../ign (relative symlink escaping context)
8411+
local dir=${TEST_SCRATCH_DIR}
8412+
local contextdir=${dir}/context
8413+
mkdir -p ${contextdir}
8414+
echo "file" > ${dir}/ign
8415+
8416+
cat > ${contextdir}/Dockerfile << _EOF
8417+
FROM alpine
8418+
COPY file /dir/
8419+
RUN test -f /dir/file
8420+
_EOF
8421+
touch ${contextdir}/file
8422+
(cd ${contextdir} && ln -s ../ign Dockerfile.dockerignore)
8423+
8424+
run_buildah build $WITH_POLICY_JSON ${contextdir}
8425+
}
8426+
8427+
# https://github.com/podman-container-tools/buildah/issues/6861
8428+
# https://github.com/podman-container-tools/podman/issues/28749
8429+
@test "bud follows symlinked containerignore within context" {
8430+
_prefetch alpine
8431+
8432+
local contextdir=${TEST_SCRATCH_DIR}/context
8433+
mkdir -p ${contextdir}/conf
8434+
echo "file" > ${contextdir}/conf/ignore-rules
8435+
8436+
cat > ${contextdir}/Containerfile << _EOF
8437+
FROM alpine
8438+
COPY * /dir/
8439+
RUN test ! -f /dir/file
8440+
_EOF
8441+
touch ${contextdir}/file
8442+
ln -s conf/ignore-rules ${contextdir}/.containerignore
8443+
8444+
run_buildah build $WITH_POLICY_JSON ${contextdir}
8445+
}
8446+
82978447
@test "build-validates-bind-bind-propagation" {
82988448
_prefetch alpine
82998449

0 commit comments

Comments
 (0)