CVE-2018-25046: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Due to improper path sanitization, archives containing relative file paths can cause files to be written (or overwritten) outside of the target directory.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 9.1
Published: 2022-12-27T22:15Z
Repository: https://github.com/cloudfoundry/archiver
Language: Go
Files changed: extractor/extractor_test.go, extractor/tgz_extractor.go, extractor/zip_extractor.go
Commit date: 2018-05-23 15:22:29 -0700
Commit message: Refactored the path resolution to use securejoin

[#157757626]

Signed-off-by: Edwin Xie <exie@pivotal.io>

Diff: diff --git a/extractor/extractor_test.go b/extractor/extractor_test.go
index 0e57cea..c4cda6e 100644
--- a/extractor/extractor_test.go
+++ b/extractor/extractor_test.go
@@ -118,10 +118,28 @@ var _ = Describe("Extractor", func() {
 				_, err := exec.LookPath("unzip")
 				Expect(err).NotTo(HaveOccurred())
 			})
 
 			It("extracts the ZIP's files, generating directories, and honoring file permissions and symlinks", extractionTest)
+
+			Context("with a bad zip archive", func() {
+				BeforeEach(func() {
+					test_helper.CreateZipArchive(extractionSrc, []test_helper.ArchiveFile{
+						{
+							Name: "../some-file",
+							Body: "file-in-bad-dir-contents",
+						},
+					})
+				})
+
+				It("returns an error", func() {
+					subdir := filepath.Join(extractionDest, "subdir")
+					Expect(os.Mkdir(subdir, 0777)).To(Succeed())
+					err := extractor.Extract(extractionSrc, subdir)
+					Expect(err).To(HaveOccurred())
+				})
+			})
 		})
 
 		Context("when 'unzip' is not in the PATH", func() {
 			var oldPATH string
 
@@ -136,10 +154,31 @@ var _ = Describe("Extractor", func() {
 			AfterEach(func() {
 				os.Setenv("PATH", oldPATH)
 			})
 
 			It("extracts the ZIP's files, generating directories, and honoring file permissions and symlinks", extractionTest)
+
+			Context("with a bad zip archive", func() {
+				BeforeEach(func() {
+					test_helper.CreateZipArchive(extractionSrc, []test_helper.ArchiveFile{
+						{
+							Name: "../some-file",
+							Body: "file-in-bad-dir-contents",
+						},
+					})
+				})
+
+				It("does not insecurely extract the file outside of the provided destination", func() {
+					subdir := filepath.Join(extractionDest, "subdir")
+					Expect(os.Mkdir(subdir, 0777)).To(Succeed())
+					err := extractor.Extract(extractionSrc, subdir)
+					Expect(err).NotTo(HaveOccurred())
+
+					Expect(filepath.Join(extractionDest, "some-file")).NotTo(BeAnExistingFile())
+					Expect(filepath.Join(subdir, "some-file")).To(BeAnExistingFile())
+				})
+			})
 		})
 	})
 
 	Context("when the file is a tgz archive", func() {
 		BeforeEach(func() {
@@ -151,10 +190,28 @@ var _ = Describe("Extractor", func() {
 				_, err := exec.LookPath("tar")
 				Expect(err).NotTo(HaveOccurred())
 			})
 
 			It("extracts the TGZ's files, generating directories, and honoring file permissions and symlinks", extractionTest)
+
+			Context("with a bad tgz archive", func() {
+				BeforeEach(func() {
+					test_helper.CreateTarGZArchive(extractionSrc, []test_helper.ArchiveFile{
+						{
+							Name: "../some-file",
+							Body: "file-in-bad-dir-contents",
+						},
+					})
+				})
+
+				It("returns an error", func() {
+					subdir := filepath.Join(extractionDest, "subdir")
+					Expect(os.Mkdir(subdir, 0777)).To(Succeed())
+					err := extractor.Extract(extractionSrc, subdir)
+					Expect(err).To(HaveOccurred())
+				})
+			})
 		})
 
 		Context("when 'tar' is not in the PATH", func() {
 			var oldPATH string
 
@@ -169,17 +226,57 @@ var _ = Describe("Extractor", func() {
 			AfterEach(func() {
 				os.Setenv("PATH", oldPATH)
 			})
 
 			It("extracts the TGZ's files, generating directories, and honoring file permissions and symlinks", extractionTest)
+
+			Context("with a bad tgz archive", func() {
+				BeforeEach(func() {
+					test_helper.CreateTarGZArchive(extractionSrc, []test_helper.ArchiveFile{
+						{
+							Name: "../some-file",
+							Body: "file-in-bad-dir-contents",
+						},
+					})
+				})
+
+				It("does not insecurely extract the file outside of the provided destination", func() {
+					subdir := filepath.Join(extractionDest, "subdir")
+					Expect(os.Mkdir(subdir, 0777)).To(Succeed())
+					err := extractor.Extract(extractionSrc, subdir)
+					Expect(err).NotTo(HaveOccurred())
+					Expect(filepath.Join(extractionDest, "some-file")).NotTo(BeAnExistingFile())
+					Expect(filepath.Join(subdir, "some-file")).To(BeAnExistingFile())
+				})
+			})
 		})
 	})
 
 	Context("when the file is a tar archive", func() {
 		BeforeEach(func() {
 			extractor = NewTar()
 			test_helper.CreateTarArchive(extractionSrc, archiveFiles)
 		})
 
 		It("extracts the TAR's files, generating directories, and honoring file permissions and symlinks", extractionTest)
+
+		Context("with a bad tar archive", func() {
+			BeforeEach(func() {
+				test_helper.CreateTarArchive(extractionSrc, []test_helper.ArchiveFile{
+					{
+						Name: "../some-file",
+						Body: "file-in-bad-dir-contents",
+					},
+				})
+			})
+
+			It("does not insecurely extract the file outside of the provided destination", func() {
+				subdir := filepath.Join(extractionDest, "subdir")
+				Expect(os.Mkdir(subdir, 0777)).To(Succeed())
+				err := extractor.Extract(extractionSrc, subdir)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(filepath.Join(extractionDest, "some-file")).NotTo(BeAnExistingFile())
+				Expect(filepath.Join(subdir, "some-file")).To(BeAnExistingFile())
+			})
+		})
 	})
 })
diff --git a/extractor/tgz_extractor.go b/extractor/tgz_extractor.go
index fee53ac..f1a1a70 100644
--- a/extractor/tgz_extractor.go
+++ b/extractor/tgz_extractor.go
@@ -6,10 +6,12 @@ import (
 	"fmt"
 	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
+
+	securejoin "github.com/cyphar/filepath-securejoin"
 )
 
 type tgzExtractor struct{}
 
 func NewTgz() Extractor {
@@ -85,18 +87,21 @@ func extractTarArchive(tarReader *tar.Reader, dest string) error {
 
 	return nil
 }
 
 func extractTarArchiveFile(header *tar.Header, dest string, input io.Reader) error {
-	filePath := filepath.Join(dest, header.Name)
+	filePath, err := securejoin.SecureJoin(dest, header.Name)
+	if err != nil {
+		return err
+	}
 	fileInfo := header.FileInfo()
 
 	if fileInfo.IsDir() {
 		return os.MkdirAll(filePath, fileInfo.Mode())
 	}
 
-	err := os.MkdirAll(filepath.Dir(filePath), 0755)
+	err = os.MkdirAll(filepath.Dir(filePath), 0755)
 	if err != nil {
 		return err
 	}
 
 	if fileInfo.Mode()&os.ModeSymlink != 0 {
diff --git a/extractor/zip_extractor.go b/extractor/zip_extractor.go
index 927644a..d61005c 100644
--- a/extractor/zip_extractor.go
+++ b/extractor/zip_extractor.go
@@ -6,10 +6,12 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
+
+	securejoin "github.com/cyphar/filepath-securejoin"
 )
 
 type zipExtractor struct{}
 
 func NewZip() Extractor {
@@ -75,20 +77,20 @@ func extractZip(src, dest string) error {
 		return nil
 	}
 }
 
 func extractZipArchiveFile(file *zip.File, dest string, input io.Reader) error {
-	filePath := filepath.Join(dest, file.Name)
+	filePath, err := securejoin.SecureJoin(dest, file.Name)
 	fileInfo := file.FileInfo()
 
 	if fileInfo.IsDir() {
-		err := os.MkdirAll(filePath, fileInfo.Mode())
+		err = os.MkdirAll(filePath, fileInfo.Mode())
 		if err != nil {
 			return err
 		}
 	} else {
-		err := os.MkdirAll(filepath.Dir(filePath), 0755)
+		err = os.MkdirAll(filepath.Dir(filePath), 0755)
 		if err != nil {
 			return err
 		}
 
 		if fileInfo.Mode()&os.ModeSymlink != 0 {

Vulnerable code: 	filePath := filepath.Join(dest, header.Name)
	err := os.MkdirAll(filepath.Dir(filePath), 0755)
	filePath := filepath.Join(dest, file.Name)
		err := os.MkdirAll(filePath, fileInfo.Mode())
		err := os.MkdirAll(filepath.Dir(filePath), 0755)
Fixed code: 
			Context("with a bad zip archive", func() {
				BeforeEach(func() {
					test_helper.CreateZipArchive(extractionSrc, []test_helper.ArchiveFile{
						{
							Name: "../some-file",
							Body: "file-in-bad-dir-contents",
						},
					})
				})

				It("returns an error", func() {
					subdir := filepath.Join(extractionDest, "subdir")
					Expect(os.Mkdir(subdir, 0777)).To(Succeed())
					err := extractor.Extract(extractionSrc, subdir)
					Expect(err).To(HaveOccurred())
				})
			})

			Context("with a bad zip archive", func() {
				BeforeEach(func() {
					test_helper.CreateZipArchive(extractionSrc, []test_helper.ArchiveFile{
						{
							Name: "../some-file",
							Body: "file-in-bad-dir-contents",
						},
					})
				})

				It("does not insecurely extract the file outside of the provided destination", func() {
					subdir := filepath.Join(extractionDest, "subdir")
					Expect(os.Mkdir(subdir, 0777)).To(Succeed())
					err := extractor.Extract(extractionSrc, subdir)
					Expect(err).NotTo(HaveOccurred())

					Expect(filepath.Join(extractionDest, "some-file")).NotTo(BeAnExistingFile())
					Expect(filepath.Join(subdir, "some-file")).To(BeAnExistingFile())
				})
			})

			Context("with a bad tgz archive", func() {
				BeforeEach(func() {
					test_helper.CreateTarGZArchive(extractionSrc, []test_helper.ArchiveFile{
						{
							Name: "../some-file",
							Body: "file-in-bad-dir-contents",
						},
					})
				})

				It("returns an error", func() {
					subdir := filepath.Join(extractionDest, "subdir")
					Expect(os.Mkdir(subdir, 0777)).To(Succeed())
					err := extractor.Extract(extractionSrc, subdir)
					Expect(err).To(HaveOccurred())
				})
			})

			Context("with a bad tgz archive", func() {
				BeforeEach(func() {
					test_helper.CreateTarGZArchive(extractionSrc, []test_helper.ArchiveFile{
						{
							Name: "../some-file",
							Body: "file-in-bad-dir-contents",
						},
					})
				})

				It("does not insecurely extract the file outside of the provided destination", func() {
					subdir := filepath.Join(extractionDest, "subdir")
					Expect(os.Mkdir(subdir, 0777)).To(Succeed())
					err := extractor.Extract(extractionSrc, subdir)
					Expect(err).NotTo(HaveOccurred())
					Expect(filepath.Join(extractionDest, "some-file")).NotTo(BeAnExistingFile())
					Expect(filepath.Join(subdir, "some-file")).To(BeAnExistingFile())
				})
			})

		Context("with a bad tar archive", func() {
			BeforeEach(func() {
				test_helper.CreateTarArchive(extractionSrc, []test_helper.ArchiveFile{
					{
						Name: "../some-file",
						Body: "file-in-bad-dir-contents",
					},
				})
			})

			It("does not insecurely extract the file outside of the provided destination", func() {
				subdir := filepath.Join(extractionDest, "subdir")
				Expect(os.Mkdir(subdir, 0777)).To(Succeed())
				err := extractor.Extract(extractionSrc, subdir)
				Expect(err).NotTo(HaveOccurred())
				Expect(filepath.Join(extractionDest, "some-file")).NotTo(BeAnExistingFile())
				Expect(filepath.Join(subdir, "some-file")).To(BeAnExistingFile())
			})
		})

	securejoin "github.com/cyphar/filepath-securejoin"
	filePath, err := securejoin.SecureJoin(dest, header.Name)
	if err != nil {
		return err
	}
	err = os.MkdirAll(filepath.Dir(filePath), 0755)

	securejoin "github.com/cyphar/filepath-securejoin"
	filePath, err := securejoin.SecureJoin(dest, file.Name)
		err = os.MkdirAll(filePath, fileInfo.Mode())
		err = os.MkdirAll(filepath.Dir(filePath), 0755)