From c2649534f35db54f3522fc85430a87f3177271fc Mon Sep 17 00:00:00 2001 From: Zelys-DFKH Date: Fri, 31 Jul 2026 18:58:03 -0500 Subject: [PATCH 1/2] fix(utils): reject tar hardlinks that escape the extraction root ExtractArchive pre-scans archive members and rejects symlinks, but tar hardlink entries carry a regular file mode and so pass that check. Header.Linkname was never validated, so an archive could create a link to a path outside the destination directory. Validate Linkname with the same path check already applied to member names. Hardlinks that resolve inside the extraction root still extract, so ordinary archives are unaffected. pkg/oci/image.go already resolves tar.TypeLink targets before using them; this brings the archive extraction path in line with it. Assisted-by: Claude:claude-opus-5 Signed-off-by: Zelys-DFKH --- pkg/utils/untar.go | 17 +++++++++++++++++ pkg/utils/untar_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/pkg/utils/untar.go b/pkg/utils/untar.go index cb65bb6b5ec0..c8521648e240 100644 --- a/pkg/utils/untar.go +++ b/pkg/utils/untar.go @@ -70,6 +70,11 @@ func ExtractArchive(archive, dst string) error { if f.FileInfo.Mode()&os.ModeSymlink != 0 { return fmt.Errorf("archive contains a symlink") } + if linkname, ok := archiveMemberLinkname(f); ok { + if err := validateArchiveMemberPath(extractRoot, linkname); err != nil { + return err + } + } return nil }) @@ -95,6 +100,18 @@ func archiveMemberName(f archiver.File) string { } } +// archiveMemberLinkname reports the target of a tar hardlink member, which carries a regular file mode and so is not caught by the symlink check. +func archiveMemberLinkname(f archiver.File) (string, bool) { + switch h := f.Header.(type) { + case tar.Header: + return h.Linkname, h.Typeflag == tar.TypeLink + case *tar.Header: + return h.Linkname, h.Typeflag == tar.TypeLink + default: + return "", false + } +} + func validateArchiveMemberPath(root, name string) error { if name == "" { return fmt.Errorf("archive contains an empty path") diff --git a/pkg/utils/untar_test.go b/pkg/utils/untar_test.go index e82b3611f790..8743267a3e94 100644 --- a/pkg/utils/untar_test.go +++ b/pkg/utils/untar_test.go @@ -59,6 +59,20 @@ var _ = Describe("utils/archive tests", func() { Expect(err.Error()).To(ContainSubstring("unsafe path")) Expect(filepath.Join(tmpDir, "escaped.txt")).ToNot(BeAnExistingFile()) }) + + It("rejects tar hardlinks that point outside the destination", func() { + tmpDir := GinkgoT().TempDir() + archivePath := filepath.Join(tmpDir, "model.tar") + extractPath := filepath.Join(tmpDir, "models") + + Expect(writeTarArchiveWithHardlink(archivePath, "payload.bin", "../../escaped.txt")).To(Succeed()) + + err := ExtractArchive(archivePath, extractPath) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("unsafe path")) + Expect(filepath.Join(tmpDir, "escaped.txt")).ToNot(BeAnExistingFile()) + }) }) func writeZipArchive(path string, files map[string]string) (err error) { @@ -126,3 +140,29 @@ func writeTarArchive(path string, files map[string]string) (err error) { return nil } + +func writeTarArchiveWithHardlink(path, name, linkname string) (err error) { + out, err := os.Create(path) + if err != nil { + return err + } + defer func() { + if closeErr := out.Close(); err == nil { + err = closeErr + } + }() + + writer := tar.NewWriter(out) + defer func() { + if closeErr := writer.Close(); err == nil { + err = closeErr + } + }() + + return writer.WriteHeader(&tar.Header{ + Name: name, + Linkname: linkname, + Typeflag: tar.TypeLink, + Mode: 0o600, + }) +} From 9aa41dc785c1deeb4ddb51e16fe6e9f6a50a5dea Mon Sep 17 00:00:00 2001 From: Zelys-DFKH Date: Fri, 31 Jul 2026 18:58:03 -0500 Subject: [PATCH 2/2] test(utils): cover hardlink overwrite and in-root hardlinks The existing hardlink test names a link target two levels above the extraction root, so its final assertion checked a path the link never resolved to and could not fail. Point the target one level up instead, at the path that assertion already names. Add two cases. The first uses a .tar.gz, where ExtractArchive binds a Tar config with OverwriteExisting set, and follows the link entry with a regular entry of the same name. Before the fix that pair linked to a file outside the root and then truncated it through the link, which the plain .tar case does not reach. The second extracts a hardlink whose target is an earlier member of the same archive, covering the claim that ordinary archives are unaffected. Assisted-by: Claude:claude-opus-5 Signed-off-by: Zelys-DFKH --- pkg/utils/untar_test.go | 128 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 127 insertions(+), 1 deletion(-) diff --git a/pkg/utils/untar_test.go b/pkg/utils/untar_test.go index 8743267a3e94..7d40125803b7 100644 --- a/pkg/utils/untar_test.go +++ b/pkg/utils/untar_test.go @@ -3,6 +3,7 @@ package utils_test import ( "archive/tar" "archive/zip" + "compress/gzip" "os" "path/filepath" @@ -60,12 +61,45 @@ var _ = Describe("utils/archive tests", func() { Expect(filepath.Join(tmpDir, "escaped.txt")).ToNot(BeAnExistingFile()) }) + It("rejects tar hardlinks that overwrite a file outside the destination", func() { + tmpDir := GinkgoT().TempDir() + archivePath := filepath.Join(tmpDir, "model.tar.gz") + extractPath := filepath.Join(tmpDir, "models") + outsidePath := filepath.Join(tmpDir, "outside.txt") + + Expect(os.WriteFile(outsidePath, []byte("original"), 0o600)).To(Succeed()) + Expect(writeTarGzArchiveWithHardlinkedFile(archivePath, "payload.bin", "../outside.txt", "overwritten")).To(Succeed()) + + err := ExtractArchive(archivePath, extractPath) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("unsafe path")) + + contents, readErr := os.ReadFile(outsidePath) + Expect(readErr).ToNot(HaveOccurred()) + Expect(string(contents)).To(Equal("original")) + }) + + It("extracts tar hardlinks that stay inside the destination", func() { + tmpDir := GinkgoT().TempDir() + archivePath := filepath.Join(tmpDir, "model.tar.gz") + extractPath := filepath.Join(tmpDir, "models") + + Expect(writeTarGzArchiveWithInternalHardlink(archivePath, "model.bin", "alias.bin", "weights")).To(Succeed()) + + Expect(ExtractArchive(archivePath, extractPath)).To(Succeed()) + + extracted, err := os.ReadFile(filepath.Join(extractPath, "alias.bin")) + Expect(err).ToNot(HaveOccurred()) + Expect(string(extracted)).To(Equal("weights")) + }) + It("rejects tar hardlinks that point outside the destination", func() { tmpDir := GinkgoT().TempDir() archivePath := filepath.Join(tmpDir, "model.tar") extractPath := filepath.Join(tmpDir, "models") - Expect(writeTarArchiveWithHardlink(archivePath, "payload.bin", "../../escaped.txt")).To(Succeed()) + Expect(writeTarArchiveWithHardlink(archivePath, "payload.bin", "../escaped.txt")).To(Succeed()) err := ExtractArchive(archivePath, extractPath) @@ -166,3 +200,95 @@ func writeTarArchiveWithHardlink(path, name, linkname string) (err error) { Mode: 0o600, }) } + +func writeTarGzArchiveWithHardlinkedFile(path, name, linkname, contents string) (err error) { + out, err := os.Create(path) + if err != nil { + return err + } + defer func() { + if closeErr := out.Close(); err == nil { + err = closeErr + } + }() + + compressor := gzip.NewWriter(out) + defer func() { + if closeErr := compressor.Close(); err == nil { + err = closeErr + } + }() + + writer := tar.NewWriter(compressor) + defer func() { + if closeErr := writer.Close(); err == nil { + err = closeErr + } + }() + + if err := writer.WriteHeader(&tar.Header{ + Name: name, + Linkname: linkname, + Typeflag: tar.TypeLink, + Mode: 0o600, + }); err != nil { + return err + } + + data := []byte(contents) + if err := writer.WriteHeader(&tar.Header{ + Name: name, + Mode: 0o600, + Size: int64(len(data)), + }); err != nil { + return err + } + _, err = writer.Write(data) + + return err +} + +func writeTarGzArchiveWithInternalHardlink(path, targetName, linkName, contents string) (err error) { + out, err := os.Create(path) + if err != nil { + return err + } + defer func() { + if closeErr := out.Close(); err == nil { + err = closeErr + } + }() + + compressor := gzip.NewWriter(out) + defer func() { + if closeErr := compressor.Close(); err == nil { + err = closeErr + } + }() + + writer := tar.NewWriter(compressor) + defer func() { + if closeErr := writer.Close(); err == nil { + err = closeErr + } + }() + + data := []byte(contents) + if err := writer.WriteHeader(&tar.Header{ + Name: targetName, + Mode: 0o600, + Size: int64(len(data)), + }); err != nil { + return err + } + if _, err := writer.Write(data); err != nil { + return err + } + + return writer.WriteHeader(&tar.Header{ + Name: linkName, + Linkname: targetName, + Typeflag: tar.TypeLink, + Mode: 0o600, + }) +}