imagepull: reject symlink ancestors during OCI flatten

safeJoin previously did textual cleaning + dest-prefix check only.
That's enough to catch `../escape`, but not the symlink-ancestor
attack: a malicious OCI layer plants `etc -> /tmp/probe`, a later
layer writes/deletes/hardlinks against `etc/anything`, and the kernel
silently dereferences the symlink so the operation lands at
`/tmp/probe/anything` on the host.

The daemon runs flatten as the owner UID, so anywhere that UID can
write becomes a write target; anywhere it can delete (e.g. its own
home) becomes a delete target. Whiteouts and hardlinks make this
worse — a whiteout for `etc/.wh.victim` would `RemoveAll` the host
file `/tmp/probe/victim`, and a TypeLink would expose host files
inside the extracted rootfs.

safeJoin now Lstat-walks every intermediate component of the joined
path against the already-extracted tree, refusing if any ancestor is
a symlink. Walking is race-free against the extraction loop because
we process tar entries serially. Leaf components stay caller-owned
(TypeSymlink writes legitimately want a symlink leaf; TypeReg
RemoveAll's any prior leaf before opening; etc.).

Three new tests pin the protection: write through a symlinked
ancestor, whiteout through a symlinked ancestor, and hardlink target
through a symlinked ancestor — each must fail and leave the host
probe path untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

internal/imagepull/flatten.go

@@ -267,12 +267,57 @@ func applyEntry(tr *tar.Reader, hdr *tar.Header, dest string, meta *Metadata) er
 	}
 }
 
-// safeJoin returns dest+rel after verifying the result lies under dest.
+// safeJoin returns dest+rel after verifying:
+//
+//  1. The cleaned result lies textually under dest (catches "../escape").
+//  2. No INTERMEDIATE component of the result is a symlink (catches the
+//     OCI extraction-escape attack: a layer plants `etc -> /etc`, then a
+//     later layer writes `etc/passwd` — without this walk the kernel
+//     would dereference the symlink and the operation would land at
+//     /etc/passwd on the host, not at <dest>/etc/passwd).
+//
+// The leaf component is intentionally NOT Lstat'd here: it may legitimately
+// be a symlink (TypeSymlink entries), a missing file (TypeReg about to be
+// created), or an existing entry that the caller will RemoveAll before
+// re-creating. Leaf type is the caller's contract.
+//
+// Walking against the already-extracted tree is race-free in practice:
+// the only mutator is this same extraction loop, and we're processing
+// entries serially.
 func safeJoin(dest, rel string) (string, error) {
 	joined := filepath.Join(dest, rel)
 	if joined != dest && !strings.HasPrefix(joined, dest+string(filepath.Separator)) {
 		return "", fmt.Errorf("unsafe path: %q escapes %q", rel, dest)
 	}
+	if joined == dest {
+		return joined, nil
+	}
+	suffix := strings.TrimPrefix(joined, dest+string(filepath.Separator))
+	segs := strings.Split(suffix, string(filepath.Separator))
+	cur := dest
+	for i, seg := range segs {
+		if seg == "" {
+			continue
+		}
+		cur = filepath.Join(cur, seg)
+		if i == len(segs)-1 {
+			break
+		}
+		info, err := os.Lstat(cur)
+		if err != nil {
+			if os.IsNotExist(err) {
+				// Ancestor not yet materialised. Once an extraction
+				// op creates it (via this same routed code), it can't
+				// be a symlink — TypeSymlink writes go through this
+				// validator too.
+				return joined, nil
+			}
+			return "", err
+		}
+		if info.Mode()&os.ModeSymlink != 0 {
+			return "", fmt.Errorf("unsafe path: ancestor %q of %q is a symlink", cur, rel)
+		}
+	}
 	return joined, nil
 }
 

internal/imagepull/imagepull_test.go

@@ -327,6 +327,103 @@ func TestFlattenRejectsRelativeSymlinkEscape(t *testing.T) {
 	}
 }
 
+// TestFlattenRejectsWriteThroughSymlinkAncestor exercises the OCI
+// extraction-escape attack: layer 1 plants `etc -> /tmp` (a directory
+// the daemon can write to), layer 2 writes `etc/probe`. Without the
+// ancestor walk in safeJoin the write would land at /tmp/probe on the
+// host. With it, the second layer's write is refused.
+func TestFlattenRejectsWriteThroughSymlinkAncestor(t *testing.T) {
+	host := startRegistry(t)
+	probeDir := t.TempDir() // a path the daemon user can write to
+	ref := pushImage(t, host, "banger/test", "sym-ancestor",
+		makeLayer(t, []tarMember{
+			{name: "etc", symlink: true, link: probeDir},
+		}),
+		makeLayer(t, []tarMember{
+			{name: "etc/probe", body: []byte("escaped")},
+		}),
+	)
+	pulled, err := Pull(context.Background(), ref, t.TempDir())
+	if err != nil {
+		t.Fatalf("Pull: %v", err)
+	}
+	dest := t.TempDir()
+	_, err = Flatten(context.Background(), pulled, dest)
+	if err == nil || !strings.Contains(err.Error(), "symlink") {
+		t.Fatalf("Flatten: err=%v, want symlink-ancestor rejection", err)
+	}
+	// The escape file must NOT have been written outside dest.
+	if _, statErr := os.Stat(filepath.Join(probeDir, "probe")); !errors.Is(statErr, os.ErrNotExist) {
+		t.Fatalf("escape file at %s should not exist; got %v", filepath.Join(probeDir, "probe"), statErr)
+	}
+}
+
+// TestFlattenRejectsWhiteoutThroughSymlinkAncestor pins the same
+// guarantee for the whiteout path: a symlinked ancestor must not let
+// the extractor RemoveAll on a host file outside dest.
+func TestFlattenRejectsWhiteoutThroughSymlinkAncestor(t *testing.T) {
+	host := startRegistry(t)
+	probeDir := t.TempDir()
+	probeFile := filepath.Join(probeDir, "victim")
+	if err := os.WriteFile(probeFile, []byte("preserved"), 0o644); err != nil {
+		t.Fatalf("write probe: %v", err)
+	}
+	ref := pushImage(t, host, "banger/test", "wh-sym-ancestor",
+		makeLayer(t, []tarMember{
+			{name: "etc", symlink: true, link: probeDir},
+		}),
+		makeLayer(t, []tarMember{
+			{name: "etc/.wh.victim"},
+		}),
+	)
+	pulled, err := Pull(context.Background(), ref, t.TempDir())
+	if err != nil {
+		t.Fatalf("Pull: %v", err)
+	}
+	dest := t.TempDir()
+	_, err = Flatten(context.Background(), pulled, dest)
+	if err == nil || !strings.Contains(err.Error(), "symlink") {
+		t.Fatalf("Flatten: err=%v, want symlink-ancestor rejection on whiteout", err)
+	}
+	if _, statErr := os.Stat(probeFile); statErr != nil {
+		t.Fatalf("probe file %s removed via whiteout escape: %v", probeFile, statErr)
+	}
+}
+
+// TestFlattenRejectsHardlinkTargetThroughSymlinkAncestor covers the
+// hardlink-target validator: a symlinked ancestor on the link source
+// must not let `os.Link` resolve through it and hard-link a host file
+// (e.g. /etc/passwd) into the extraction tree.
+func TestFlattenRejectsHardlinkTargetThroughSymlinkAncestor(t *testing.T) {
+	host := startRegistry(t)
+	probeDir := t.TempDir()
+	probeFile := filepath.Join(probeDir, "secret")
+	if err := os.WriteFile(probeFile, []byte("hands off"), 0o644); err != nil {
+		t.Fatalf("write probe: %v", err)
+	}
+	ref := pushImage(t, host, "banger/test", "ln-sym-ancestor",
+		makeLayer(t, []tarMember{
+			{name: "etc", symlink: true, link: probeDir},
+		}),
+		makeLayer(t, []tarMember{
+			{name: "leaked", hardlink: true, link: "etc/secret"},
+		}),
+	)
+	pulled, err := Pull(context.Background(), ref, t.TempDir())
+	if err != nil {
+		t.Fatalf("Pull: %v", err)
+	}
+	dest := t.TempDir()
+	_, err = Flatten(context.Background(), pulled, dest)
+	if err == nil || !strings.Contains(err.Error(), "symlink") {
+		t.Fatalf("Flatten: err=%v, want symlink-ancestor rejection on hardlink target", err)
+	}
+	// dest must not contain a hardlink to the host secret.
+	if _, statErr := os.Lstat(filepath.Join(dest, "leaked")); !errors.Is(statErr, os.ErrNotExist) {
+		t.Fatalf("hardlink leaked file should not exist in dest; got %v", statErr)
+	}
+}
+
 func TestFlattenTarRejectsDebugFSHostilePath(t *testing.T) {
 	tarData := buildTar(t, []tarMember{
 		{name: "etc/bad\tname", body: []byte("bad")},