ptar: reject path traversal and duplicate paths in layers

Co-authored-by: Jason Hall <imjasonh@users.noreply.github.com>

pymage/internal/ptar/ptar.go

@@ -17,6 +17,7 @@ package ptar
 import (
 	"archive/tar"
 	"bytes"
+	"fmt"
 	"io"
 	"path"
 	"sort"
@@ -63,7 +64,10 @@ func WriteTar(w io.Writer, files []File) error {
 	// stable and parents always precede children.
 	dirs := map[string]struct{}{}
 	for _, f := range files {
-		clean := path.Clean(strings.TrimPrefix(f.Path, "/"))
+		clean, err := cleanPath(f.Path)
+		if err != nil {
+			return err
+		}
 		dir := path.Dir(clean)
 		for dir != "." && dir != "/" && dir != "" {
 			dirs[dir] = struct{}{}
@@ -80,11 +84,17 @@ func WriteTar(w io.Writer, files []File) error {
 	for d := range dirs {
 		entries = append(entries, entry{name: d + "/", dir: true})
 	}
+	// Collisions are surfaced as errors rather than silently keeping an
+	// arbitrary winner: within a single layer two members must not map to the
+	// same path (it would make the layer depend on input ordering).
 	seen := map[string]bool{}
 	for _, f := range files {
-		clean := path.Clean(strings.TrimPrefix(f.Path, "/"))
+		clean, err := cleanPath(f.Path)
+		if err != nil {
+			return err
+		}
 		if seen[clean] {
-			continue
+			return fmt.Errorf("ptar: duplicate path %q in layer", clean)
 		}
 		seen[clean] = true
 		entries = append(entries, entry{name: clean, file: f})
@@ -126,6 +136,24 @@ func WriteTar(w io.Writer, files []File) error {
 	return tw.Close()
 }
 
+// cleanPath returns the normalized, slash-rooted-relative archive name for p,
+// rejecting paths that would escape the archive root via "..". Absolute paths
+// are made relative.
+func cleanPath(p string) (string, error) {
+	clean := path.Clean("/" + strings.TrimPrefix(p, "/"))
+	rel := strings.TrimPrefix(clean, "/")
+	if rel == "" || rel == "." {
+		return "", fmt.Errorf("ptar: empty path %q", p)
+	}
+	// path.Clean("/"+...) already collapses any ".." that would escape the
+	// root, but a literal traversal prefix on the input is a strong signal of a
+	// malicious archive, so reject it explicitly.
+	if orig := path.Clean(p); orig == ".." || strings.HasPrefix(orig, "../") {
+		return "", fmt.Errorf("ptar: path %q escapes archive root", p)
+	}
+	return rel, nil
+}
+
 // TarBytes returns the deterministic tar stream for files.
 func TarBytes(files []File) ([]byte, error) {
 	var buf bytes.Buffer

pymage/internal/ptar/ptar_test.go

@@ -76,6 +76,24 @@ func TestTarSynthesizesParentDirsAndSorts(t *testing.T) {
 	}
 }
 
+func TestWriteTarRejectsTraversal(t *testing.T) {
+	for _, p := range []string{"../etc/passwd", "../../x", "a/../../../etc/shadow"} {
+		if _, err := TarBytes([]File{{Path: p, Data: []byte("x")}}); err == nil {
+			t.Errorf("expected error for traversal path %q", p)
+		}
+	}
+}
+
+func TestWriteTarRejectsDuplicatePaths(t *testing.T) {
+	_, err := TarBytes([]File{
+		{Path: "a/x.txt", Data: []byte("1")},
+		{Path: "a/x.txt", Data: []byte("2")},
+	})
+	if err == nil {
+		t.Fatal("expected error for duplicate path")
+	}
+}
+
 func TestLayerStableDigest(t *testing.T) {
 	files := []File{
 		{Path: "site-packages/foo/__init__.py", Data: []byte("print('hi')\n")},