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..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" @@ -59,6 +60,53 @@ 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 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()) + + 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 +174,121 @@ 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, + }) +} + +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, + }) +}