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>

Author: Honny1

pkg/parse/parse.go

@@ -17,7 +17,6 @@ import (
 	"unicode"
 
 	"github.com/containerd/platforms"
-	securejoin "github.com/cyphar/filepath-securejoin"
 	units "github.com/docker/go-units"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/selinux/go-selinux"
@@ -1403,37 +1402,67 @@ func ContainerIgnoreFile(contextDir, path string, containerFiles []string) ([]st
 		excludes, err := imagebuilder.ParseIgnore(path)
 		return excludes, path, err
 	}
-	// If path was not supplied give priority to `<containerfile>.containerignore` first.
+	// If path was not supplied, look for `<containerfile>.containerignore` and
+	// `<containerfile>.dockerignore`. When both exist, `.dockerignore` wins.
 	for _, containerfile := range containerFiles {
 		if !filepath.IsAbs(containerfile) {
 			containerfile = filepath.Join(contextDir, containerfile)
 		}
-		containerfileIgnore := ""
-		if err := fileutils.Exists(containerfile + ".containerignore"); err == nil {
-			containerfileIgnore = containerfile + ".containerignore"
+		cleanPath := filepath.Clean(containerfile)
+		relPath, relErr := filepath.Rel(contextDir, cleanPath)
+		insideContext := relErr == nil && relPath != ".." && !strings.HasPrefix(relPath, ".."+string(filepath.Separator))
+		var rootDir, baseName string
+		if insideContext {
+			rootDir = contextDir
+			baseName = relPath
+		} else {
+			rootDir = filepath.Dir(cleanPath)
+			baseName = filepath.Base(cleanPath)
+		}
+		root, err := os.OpenRoot(rootDir)
+		if err != nil {
+			continue
 		}
-		if err := fileutils.Exists(containerfile + ".dockerignore"); err == nil {
-			containerfileIgnore = containerfile + ".dockerignore"
+		excludes, name, err := findIgnoreFile(root, baseName+".containerignore", baseName+".dockerignore")
+		root.Close()
+		if err != nil {
+			return nil, "", err
 		}
-		if containerfileIgnore != "" {
-			excludes, err := imagebuilder.ParseIgnore(containerfileIgnore)
-			return excludes, containerfileIgnore, err
+		if name != "" {
+			return excludes, filepath.Join(rootDir, name), nil
 		}
 	}
-	path, symlinkErr := securejoin.SecureJoin(contextDir, ".containerignore")
-	if symlinkErr != nil {
-		return nil, "", symlinkErr
+	root, err := os.OpenRoot(contextDir)
+	if err != nil {
+		return nil, "", err
 	}
-	excludes, err := imagebuilder.ParseIgnore(path)
-	if errors.Is(err, os.ErrNotExist) {
-		path, symlinkErr = securejoin.SecureJoin(contextDir, ".dockerignore")
-		if symlinkErr != nil {
-			return nil, "", symlinkErr
-		}
-		excludes, err = imagebuilder.ParseIgnore(path)
+	defer root.Close()
+	excludes, name, err := findIgnoreFile(root, ".containerignore", ".dockerignore")
+	if err != nil {
+		return nil, "", err
 	}
-	if errors.Is(err, os.ErrNotExist) {
-		return excludes, "", nil
+	if name != "" {
+		return excludes, filepath.Join(contextDir, name), nil
+	}
+	return nil, "", nil
+}
+
+// findIgnoreFile tries each candidate name inside root.  When both exist the
+// last one wins (i.e. .dockerignore takes precedence over .containerignore).
+func findIgnoreFile(root *os.Root, candidates ...string) ([]string, string, error) {
+	var excludes []string
+	var matched string
+	for _, name := range candidates {
+		f, err := root.Open(name)
+		if err != nil {
+			continue
+		}
+		excludes, err = imagebuilder.ParseIgnoreReader(f)
+		f.Close()
+		if err != nil {
+			return nil, "", err
+		}
+		matched = name
 	}
-	return excludes, path, err
+	return excludes, matched, nil
 }

pkg/util/util.go

@@ -41,42 +41,67 @@ func MirrorToTempFileIfPathIsDescriptor(file string) (string, bool) {
 }
 
 // DiscoverContainerfile tries to find a Containerfile or a Dockerfile within the provided `path`.
+// The path may be a directory (in which case Containerfile/Dockerfile is searched inside it)
+// or a direct path to a container file.
+//
+// Symlinked Containerfile/Dockerfile entries are only used when their real
+// target stays inside the build context directory. Symlinks that resolve
+// outside the context or are dangling are skipped.
 func DiscoverContainerfile(path string) (foundCtrFile string, err error) {
-	// Test for existence of the file
-	target, err := os.Stat(path)
+	path, err = filepath.Abs(path)
 	if err != nil {
 		return "", fmt.Errorf("discovering Containerfile: %w", err)
 	}
 
-	switch mode := target.Mode(); {
-	case mode.IsDir():
-		// If the path is a real directory, we assume a Containerfile or a Dockerfile within it
-		ctrfile := filepath.Join(path, "Containerfile")
-
-		// Test for existence of the Containerfile file
-		file, err := os.Stat(ctrfile)
-		if err != nil {
-			// See if we have a Dockerfile within it
-			ctrfile = filepath.Join(path, "Dockerfile")
+	target, err := os.Lstat(path)
+	if err != nil {
+		return "", fmt.Errorf("discovering Containerfile: %w", err)
+	}
 
-			// Test for existence of the Dockerfile file
-			file, err = os.Stat(ctrfile)
-			if err != nil {
-				return "", fmt.Errorf("cannot find Containerfile or Dockerfile in context directory: %w", err)
+	switch {
+	case target.IsDir():
+		for _, name := range []string{"Containerfile", "Dockerfile"} {
+			ctrfile := filepath.Join(path, name)
+			if isRegularFileInContext(path, ctrfile) {
+				return ctrfile, nil
 			}
 		}
+		return "", fmt.Errorf("cannot find Containerfile or Dockerfile in context directory")
+
+	case target.Mode().IsRegular():
+		return path, nil
 
-		// The file exists, now verify the correct mode
-		if mode := file.Mode(); mode.IsRegular() {
-			foundCtrFile = ctrfile
-		} else {
-			return "", fmt.Errorf("assumed Containerfile %q is not a file", ctrfile)
+	case target.Mode()&os.ModeSymlink != 0:
+		if isRegularFileInContext(filepath.Dir(path), path) {
+			return path, nil
 		}
+		return "", fmt.Errorf("assumed Containerfile %q is not a file", path)
 
-	case mode.IsRegular():
-		// If the context dir is a file, we assume this as Containerfile
-		foundCtrFile = path
+	default:
+		return "", fmt.Errorf("assumed Containerfile %q is not a file", path)
 	}
+}
 
-	return foundCtrFile, nil
+// isRegularFileInContext returns true if path is a regular file (or a symlink
+// to one) whose real target is inside contextDir.
+// It uses os.Root to confine all lookups within contextDir, so symlinks
+// that escape the context at any point. Including intermediate components
+// and absolute targets are rejected.
+func isRegularFileInContext(contextDir, path string) bool {
+	root, err := os.OpenRoot(contextDir)
+	if err != nil {
+		return false
+	}
+	defer root.Close()
+	name, err := filepath.Rel(contextDir, path)
+	if err != nil {
+		return false
+	}
+	// Stat will follow the symlink and verify the target is
+	// a regular file inside the root. If any hop escapes, Stat will error out.
+	fi, err := root.Stat(name)
+	if err != nil {
+		return false
+	}
+	return fi.Mode().IsRegular()
 }

pkg/util/util_test.go

@@ -1,11 +1,21 @@
 package util //nolint:revive,nolintlint
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+func absPath(t *testing.T, rel string) string {
+	t.Helper()
+	p, err := filepath.Abs(rel)
+	require.NoError(t, err)
+	return p
+}
+
 func TestDiscoverContainerfile(t *testing.T) {
 	t.Parallel()
 	_, err := DiscoverContainerfile("./bogus")
@@ -16,17 +26,47 @@ func TestDiscoverContainerfile(t *testing.T) {
 
 	name, err := DiscoverContainerfile("test/test1/Dockerfile")
 	assert.Nil(t, err)
-	assert.Equal(t, name, "test/test1/Dockerfile")
+	assert.Equal(t, absPath(t, "test/test1/Dockerfile"), name)
 
 	name, err = DiscoverContainerfile("test/test1/Containerfile")
 	assert.Nil(t, err)
-	assert.Equal(t, name, "test/test1/Containerfile")
+	assert.Equal(t, absPath(t, "test/test1/Containerfile"), name)
 
 	name, err = DiscoverContainerfile("test/test1")
 	assert.Nil(t, err)
-	assert.Equal(t, name, "test/test1/Containerfile")
+	assert.Equal(t, absPath(t, "test/test1/Containerfile"), name)
 
 	name, err = DiscoverContainerfile("test/test2")
 	assert.Nil(t, err)
-	assert.Equal(t, name, "test/test2/Dockerfile")
+	assert.Equal(t, absPath(t, "test/test2/Dockerfile"), name)
+}
+
+func TestDiscoverContainerfileRejectsSymlinkOutsideContext(t *testing.T) {
+	t.Parallel()
+	tmpDir := t.TempDir()
+
+	secretFile := filepath.Join(tmpDir, "secret-Containerfile")
+	require.NoError(t, os.WriteFile(secretFile, []byte("FROM scratch\n"), 0o644))
+
+	contextDir := filepath.Join(tmpDir, "context")
+	require.NoError(t, os.Mkdir(contextDir, 0o755))
+	require.NoError(t, os.Symlink(secretFile, filepath.Join(contextDir, "Containerfile")))
+
+	_, err := DiscoverContainerfile(contextDir)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "cannot find Containerfile or Dockerfile")
+}
+
+func TestDiscoverContainerfileAcceptsSymlinkInsideContext(t *testing.T) {
+	t.Parallel()
+	contextDir := t.TempDir()
+
+	subdir := filepath.Join(contextDir, "subdir")
+	require.NoError(t, os.Mkdir(subdir, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(subdir, "Containerfile.real"), []byte("FROM scratch\n"), 0o644))
+	require.NoError(t, os.Symlink(filepath.Join("subdir", "Containerfile.real"), filepath.Join(contextDir, "Containerfile")))
+
+	name, err := DiscoverContainerfile(contextDir)
+	require.NoError(t, err)
+	assert.Equal(t, filepath.Join(contextDir, "Containerfile"), name)
 }