Visitar URL original
pkg/v1/mutate: preserve relative symlinks that stay within rootfs in … · google/go-containerregistry@c29d91c · GitHub
Skip to content

Commit c29d91c

Browse files
authored
pkg/v1/mutate: preserve relative symlinks that stay within rootfs in Extract (#2279)
* pkg/v1/mutate: preserve relative symlinks that stay within rootfs in Extract The `extract()` function in `pkg/v1/mutate/mutate.go` had all symlink filtering removed in the #2250 revert. Without any filtering, images can contain symlinks with absolute targets (e.g. `/etc/passwd`) or relative targets that escape the rootfs (e.g. `../../../tmp/evil`), both of which are unsafe when layers are extracted to disk. Signed-off-by: anish k <ak8686@princeton.edu>
1 parent a70d75a commit c29d91c

2 files changed

Lines changed: 89 additions & 0 deletions

File tree

‎pkg/v1/mutate/mutate.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,22 @@ func extractLayer(tarWriter *tar.Writer, fileMap map[string]bool, layer v1.Layer
305305
// name, we may have duplicate entries, which angers tar-split.
306306
header.Name = filepath.Clean(header.Name)
307307

308+
// Reject relative symlinks and hardlinks whose targets escape the
309+
// image rootfs. Relative targets are resolved against the symlink's
310+
// own directory: if the clean result starts with ".." the link would
311+
// leave the rootfs. Relative symlinks that stay within the rootfs
312+
// (common for glibc, C toolchains, etc.) are preserved unchanged.
313+
// Absolute targets are left as-is; see #2238 for ongoing discussion
314+
// on whether they should be pruned.
315+
if header.Typeflag == tar.TypeSymlink || header.Typeflag == tar.TypeLink {
316+
if !filepath.IsAbs(header.Linkname) {
317+
resolved := filepath.Clean(filepath.Join(filepath.Dir(header.Name), header.Linkname)) //nolint:gosec // G305: path is only used for validation, not file I/O
318+
if strings.HasPrefix(resolved, "..") {
319+
continue
320+
}
321+
}
322+
}
323+
308324
// force PAX format to remove Name/Linkname length limit of 100 characters
309325
// required by USTAR and to not depend on internal tar package guess which
310326
// prefers USTAR over PAX

‎pkg/v1/mutate/mutate_test.go‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,79 @@ func TestAppendStreamableLayer(t *testing.T) {
686686
}
687687
}
688688

689+
// TestExtractSymlinkFiltering verifies that Extract preserves relative symlinks
690+
// that stay within the rootfs and absolute symlinks (left for callers to handle),
691+
// while dropping relative symlinks whose resolved path escapes the rootfs boundary.
692+
func TestExtractSymlinkFiltering(t *testing.T) {
693+
// Build a tar layer with several symlink entries.
694+
var buf bytes.Buffer
695+
tw := tar.NewWriter(&buf)
696+
entries := []struct {
697+
name string
698+
typeflag byte
699+
linkname string
700+
}{
701+
// Safe relative symlink: usr/local/bin/ld.so -> ../lib/ld-linux.so.2
702+
// resolves to usr/local/lib/ld-linux.so.2, inside rootfs.
703+
{name: "usr/local/bin/ld.so", typeflag: tar.TypeSymlink, linkname: "../lib/ld-linux.so.2"},
704+
// Safe relative symlink: var/lock -> ../run/lock (tailscale/glibc pattern).
705+
{name: "usr/local/lib/containers/app/var/lock", typeflag: tar.TypeSymlink, linkname: "../run/lock"},
706+
// Absolute target: preserved (see #2238 for ongoing discussion).
707+
{name: "usr/local/bin/abs-link", typeflag: tar.TypeSymlink, linkname: "/etc/passwd"},
708+
// Unsafe: relative target resolves outside rootfs.
709+
{name: "etc/foo", typeflag: tar.TypeSymlink, linkname: "../../../tmp/evil"},
710+
}
711+
for _, e := range entries {
712+
if err := tw.WriteHeader(&tar.Header{
713+
Typeflag: e.typeflag,
714+
Name: e.name,
715+
Linkname: e.linkname,
716+
}); err != nil {
717+
t.Fatalf("WriteHeader: %v", err)
718+
}
719+
}
720+
tw.Close()
721+
722+
layer, err := tarball.LayerFromOpener(func() (io.ReadCloser, error) {
723+
return io.NopCloser(bytes.NewReader(buf.Bytes())), nil
724+
})
725+
if err != nil {
726+
t.Fatalf("LayerFromOpener: %v", err)
727+
}
728+
img, err := mutate.AppendLayers(empty.Image, layer)
729+
if err != nil {
730+
t.Fatalf("AppendLayers: %v", err)
731+
}
732+
733+
extracted := map[string]string{}
734+
tr := tar.NewReader(mutate.Extract(img))
735+
for {
736+
hdr, err := tr.Next()
737+
if errors.Is(err, io.EOF) {
738+
break
739+
}
740+
if err != nil {
741+
t.Fatalf("reading extracted tar: %v", err)
742+
}
743+
if hdr.Typeflag == tar.TypeSymlink || hdr.Typeflag == tar.TypeLink {
744+
extracted[hdr.Name] = hdr.Linkname
745+
}
746+
}
747+
748+
// These symlinks must be preserved: safe relative ones and absolute ones.
749+
for _, name := range []string{"usr/local/bin/ld.so", "usr/local/lib/containers/app/var/lock", "usr/local/bin/abs-link"} {
750+
if _, ok := extracted[name]; !ok {
751+
t.Errorf("symlink %q was incorrectly dropped", name)
752+
}
753+
}
754+
// Relative targets that escape the rootfs must be filtered out.
755+
for _, name := range []string{"etc/foo"} {
756+
if target, ok := extracted[name]; ok {
757+
t.Errorf("unsafe symlink %q -> %q was not dropped", name, target)
758+
}
759+
}
760+
}
761+
689762
func TestCanonical(t *testing.T) {
690763
source := sourceImage(t)
691764
img, err := mutate.Canonical(source)

0 commit comments

Comments
 (0)