Skip to content

Commit a83f676

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 a83f676

4 files changed

Lines changed: 292 additions & 38 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: 67 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -41,42 +41,84 @@ 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")
70+
71+
case target.Mode().IsRegular():
72+
return path, nil
6873

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)
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. Symlinks whose raw target
87+
// traverses outside the context at any intermediate step are rejected, even if
88+
// the final resolved path lands back inside.
89+
func isRegularFileInContext(contextDir, path string) bool {
90+
cleanContext, err := filepath.EvalSymlinks(contextDir)
91+
if err != nil {
92+
return false
93+
}
94+
info, err := os.Lstat(path)
95+
if err != nil {
96+
return false
97+
}
98+
if info.Mode()&os.ModeSymlink != 0 {
99+
target, err := os.Readlink(path)
100+
if err != nil {
101+
return false
102+
}
103+
clean := filepath.Clean(target)
104+
if !filepath.IsAbs(target) && (clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator))) {
105+
return false
106+
}
107+
}
108+
resolved, err := filepath.EvalSymlinks(path)
109+
if err != nil {
110+
return false
111+
}
112+
rel, err := filepath.Rel(cleanContext, resolved)
113+
if err != nil {
114+
return false
115+
}
116+
if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
117+
return false
118+
}
119+
fi, err := os.Stat(path)
120+
if err != nil {
121+
return false
122+
}
123+
return fi.Mode().IsRegular()
82124
}

‎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
}

0 commit comments

Comments
 (0)