imagepull + kernelcat: allow absolute symlink targets

Container (and kernel) layers routinely ship symlinks with absolute
targets — /usr/bin/mawk, /lib/modules/<ver>/build, etc. Those are
interpreted relative to the rootfs at runtime (`/` inside the VM),
not against the host filesystem, so they are rooted inside dest by
construction and need no escape check at write time.

The previous logic resolved absolute Linknames literally (against
the host root), compared to the staging dir, and rejected everything
that didn't happen to live under it. That made `banger image pull
docker.io/library/debian:bookworm` fail on the very first symlink
("etc/alternatives/awk -> /usr/bin/mawk").

Relative targets still get the traversal check — a relative
Linkname with ../s can genuinely escape dest at write time even if
in-VM resolution would be safe — so the defense against malicious
relative chains is intact.

Tests:
 - TestFlattenAcceptsAbsoluteSymlink replaces the old overly-strict
   test, using the exact etc/alternatives/awk -> /usr/bin/mawk case
   that broke debian:bookworm.
 - TestFlattenRejectsRelativeSymlinkEscape confirms relative-with-
   traversal is still rejected with the same "unsafe symlink"
   error.

Same fix applied in internal/kernelcat/fetch.go for consistency;
future kernel bundles with absolute symlinks in the modules tree
would otherwise hit the same wall.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Thales Maciel 2026-04-16 17:33:16 -03:00
parent d5f72dfad9
commit fdaf7cce0f
No known key found for this signature in database
GPG key ID: 33112E6833C34679
3 changed files with 57 additions and 22 deletions

View file

@ -132,16 +132,18 @@ func applyEntry(tr *tar.Reader, hdr *tar.Header, dest string) error {
if err := os.MkdirAll(filepath.Dir(abs), 0o755); err != nil { if err := os.MkdirAll(filepath.Dir(abs), 0o755); err != nil {
return err return err
} }
// Resolve the link target relative to the link's parent and // Container layers commonly use absolute symlink targets like
// require that it stays inside dest. Absolute targets that // "/usr/bin/mawk" — these are interpreted relative to the
// resolve outside dest are also rejected. // rootfs (`/` inside the eventual VM), so they're rooted at
resolved := hdr.Linkname // dest by construction and need no escape check.
if !filepath.IsAbs(resolved) { // Relative targets, however, can escape with "../"s and must
resolved = filepath.Join(filepath.Dir(abs), resolved) // be checked against dest at write time (we never follow them
} // during extraction, but a future caller might).
resolved = filepath.Clean(resolved) if !filepath.IsAbs(hdr.Linkname) {
if resolved != dest && !strings.HasPrefix(resolved, dest+string(filepath.Separator)) { resolved := filepath.Clean(filepath.Join(filepath.Dir(abs), hdr.Linkname))
return fmt.Errorf("unsafe symlink in layer: %q -> %q", hdr.Name, hdr.Linkname) if resolved != dest && !strings.HasPrefix(resolved, dest+string(filepath.Separator)) {
return fmt.Errorf("unsafe symlink in layer: %q -> %q", hdr.Name, hdr.Linkname)
}
} }
if err := os.RemoveAll(abs); err != nil && !errors.Is(err, os.ErrNotExist) { if err := os.RemoveAll(abs); err != nil && !errors.Is(err, os.ErrNotExist) {
return err return err

View file

@ -237,11 +237,43 @@ func TestFlattenRejectsPathTraversal(t *testing.T) {
} }
} }
func TestFlattenRejectsUnsafeSymlink(t *testing.T) { func TestFlattenAcceptsAbsoluteSymlink(t *testing.T) {
// Container layers regularly contain absolute symlinks like
// /usr/bin/mawk — they're interpreted relative to the rootfs at
// boot time, not against the host filesystem. They must extract
// cleanly.
host := startRegistry(t) host := startRegistry(t)
ref := pushImage(t, host, "banger/test", "evil-sym", ref := pushImage(t, host, "banger/test", "abs-sym",
makeLayer(t, []tarMember{ makeLayer(t, []tarMember{
{name: "evil", symlink: true, link: "/etc/passwd"}, // absolute target outside dest {name: "etc/alternatives/awk", symlink: true, link: "/usr/bin/mawk"},
}),
)
pulled, err := Pull(context.Background(), ref, t.TempDir())
if err != nil {
t.Fatalf("Pull: %v", err)
}
dest := t.TempDir()
if err := Flatten(context.Background(), pulled, dest); err != nil {
t.Fatalf("Flatten: %v", err)
}
link := filepath.Join(dest, "etc/alternatives/awk")
target, err := os.Readlink(link)
if err != nil {
t.Fatalf("readlink: %v", err)
}
if target != "/usr/bin/mawk" {
t.Errorf("link target = %q, want /usr/bin/mawk", target)
}
}
func TestFlattenRejectsRelativeSymlinkEscape(t *testing.T) {
// Relative symlinks with .. must still be rejected: the resolved
// path can escape dest at the host level even if the in-VM
// resolution would be safe.
host := startRegistry(t)
ref := pushImage(t, host, "banger/test", "rel-escape",
makeLayer(t, []tarMember{
{name: "etc/evil", symlink: true, link: "../../../../etc/passwd"},
}), }),
) )
pulled, err := Pull(context.Background(), ref, t.TempDir()) pulled, err := Pull(context.Background(), ref, t.TempDir())
@ -250,7 +282,7 @@ func TestFlattenRejectsUnsafeSymlink(t *testing.T) {
} }
err = Flatten(context.Background(), pulled, t.TempDir()) err = Flatten(context.Background(), pulled, t.TempDir())
if err == nil || !strings.Contains(err.Error(), "unsafe symlink") { if err == nil || !strings.Contains(err.Error(), "unsafe symlink") {
t.Fatalf("Flatten unsafe symlink: err=%v", err) t.Fatalf("Flatten relative escape: err=%v", err)
} }
} }

View file

@ -167,14 +167,15 @@ func extractTar(r io.Reader, target string) error {
if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil {
return err return err
} }
link := hdr.Linkname // Absolute targets are interpreted at runtime against the
resolved := link // eventual rootfs (`/` inside the VM), so they're rooted
if !filepath.IsAbs(link) { // inside absTarget by construction. Only relative targets
resolved = filepath.Join(filepath.Dir(dst), link) // need an escape check at write time.
} if !filepath.IsAbs(hdr.Linkname) {
resolved = filepath.Clean(resolved) resolved := filepath.Clean(filepath.Join(filepath.Dir(dst), hdr.Linkname))
if resolved != absTarget && !strings.HasPrefix(resolved, absTarget+string(filepath.Separator)) { if resolved != absTarget && !strings.HasPrefix(resolved, absTarget+string(filepath.Separator)) {
return fmt.Errorf("unsafe symlink in tarball: %q -> %q", hdr.Name, hdr.Linkname) return fmt.Errorf("unsafe symlink in tarball: %q -> %q", hdr.Name, hdr.Linkname)
}
} }
if err := os.Symlink(hdr.Linkname, dst); err != nil { if err := os.Symlink(hdr.Linkname, dst); err != nil {
return err return err