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

@@ -1422,37 +1422,66 @@ 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>.dockerignore` and
+	// `<containerfile>.containerignore`. When both exist, `.containerignore` wins.
+	// securejoin confines lookups with RESOLVE_IN_ROOT semantics, matching
+	// Docker BuildKit behavior. When the containerfile is inside contextDir
+	// we resolve relative to contextDir; when it is outside (e.g.
+	// overlay-mounted context or -f pointing elsewhere) we resolve relative
+	// to the containerfile's parent directory.
 	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)
 		}
-		if err := fileutils.Exists(containerfile + ".dockerignore"); err == nil {
-			containerfileIgnore = containerfile + ".dockerignore"
+		excludes, resolved, err := findIgnoreFile(rootDir, baseName+".dockerignore", baseName+".containerignore")
+		if err != nil {
+			return nil, "", err
 		}
-		if containerfileIgnore != "" {
-			excludes, err := imagebuilder.ParseIgnore(containerfileIgnore)
-			return excludes, containerfileIgnore, err
+		if resolved != "" {
+			return excludes, resolved, nil
 		}
 	}
-	path, symlinkErr := securejoin.SecureJoin(contextDir, ".containerignore")
-	if symlinkErr != nil {
-		return nil, "", symlinkErr
+	excludes, resolved, err := findIgnoreFile(contextDir, ".dockerignore", ".containerignore")
+	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)
+	if resolved != "" {
+		return excludes, resolved, nil
 	}
-	if errors.Is(err, os.ErrNotExist) {
-		return excludes, "", nil
+	return nil, "", nil
+}
+
+// findIgnoreFile tries each candidate name resolved under rootDir using
+// securejoin (RESOLVE_IN_ROOT semantics). When both exist the last one wins.
+func findIgnoreFile(rootDir string, candidates ...string) ([]string, string, error) {
+	var excludes []string
+	var matched string
+	for _, name := range candidates {
+		resolved, err := securejoin.SecureJoin(rootDir, name)
+		if err != nil {
+			continue
+		}
+		f, err := os.Open(resolved)
+		if err != nil {
+			continue
+		}
+		excludes, err = imagebuilder.ParseIgnoreReader(f)
+		f.Close()
+		if err != nil {
+			return nil, "", err
+		}
+		matched = resolved
 	}
-	return excludes, path, err
+	return excludes, matched, nil
 }

pkg/util/util.go

@@ -2,10 +2,12 @@ package util //nolint:revive,nolintlint
 
 import (
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"strings"
 
+	securejoin "github.com/cyphar/filepath-securejoin"
 	"go.podman.io/buildah/pkg/parse"
 )
 
@@ -41,42 +43,68 @@ 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 resolved, ok := isRegularFileInContext(path, ctrfile); ok {
+				return resolved, nil
 			}
 		}
+		return "", fmt.Errorf("cannot find Containerfile or Dockerfile in context directory: %w", fs.ErrNotExist)
 
-		// 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().IsRegular():
+		return path, nil
+
+	case target.Mode()&os.ModeSymlink != 0:
+		if resolved, ok := isRegularFileInContext(filepath.Dir(path), path); ok {
+			return resolved, 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 checks whether path resolves to a regular file
+// inside contextDir using RESOLVE_IN_ROOT semantics (securejoin.SecureJoin):
+// ".." components are clamped to the root and absolute symlink targets are
+// re-rooted under contextDir.  This matches Docker BuildKit's behavior.
+//
+// On success it returns the resolved host path.
+func isRegularFileInContext(contextDir, path string) (string, bool) {
+	name, err := filepath.Rel(contextDir, path)
+	if err != nil {
+		return "", false
+	}
+	resolved, err := securejoin.SecureJoin(contextDir, name)
+	if err != nil {
+		return "", false
+	}
+	fi, err := os.Stat(resolved)
+	if err != nil {
+		return "", false
+	}
+	if !fi.Mode().IsRegular() {
+		return "", false
+	}
+	return resolved, true
 }

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,88 @@ 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, "subdir", "Containerfile.real"), name)
+}
+
+func TestDiscoverContainerfileAcceptsEscapeClampedToRoot(t *testing.T) {
+	t.Parallel()
+	tmpDir := t.TempDir()
+
+	contextDir := filepath.Join(tmpDir, "context")
+	require.NoError(t, os.Mkdir(contextDir, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(contextDir, "file"), []byte("FROM scratch\n"), 0o644))
+	require.NoError(t, os.Symlink("../file", filepath.Join(contextDir, "Containerfile")))
+
+	name, err := DiscoverContainerfile(contextDir)
+	require.NoError(t, err)
+	assert.Equal(t, filepath.Join(contextDir, "file"), name)
+}
+
+func TestDiscoverContainerfileAcceptsAbsoluteSymlinkRerooted(t *testing.T) {
+	t.Parallel()
+	contextDir := t.TempDir()
+
+	subdir := filepath.Join(contextDir, "subdirectory")
+	require.NoError(t, os.Mkdir(subdir, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(subdir, "real.file"), []byte("FROM scratch\n"), 0o644))
+	require.NoError(t, os.Symlink("/subdirectory/real.file", filepath.Join(contextDir, "Containerfile")))
+
+	name, err := DiscoverContainerfile(contextDir)
+	require.NoError(t, err)
+	assert.Equal(t, filepath.Join(contextDir, "subdirectory", "real.file"), name)
+}
+
+func TestDiscoverContainerfileRejectsNonExistentClampedTarget(t *testing.T) {
+	t.Parallel()
+	tmpDir := t.TempDir()
+
+	contextDir := filepath.Join(tmpDir, "context")
+	require.NoError(t, os.Mkdir(contextDir, 0o755))
+	require.NoError(t, os.Symlink("../nonexistent", filepath.Join(contextDir, "Containerfile")))
+
+	_, err := DiscoverContainerfile(contextDir)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "cannot find Containerfile or Dockerfile")
 }