Verify all symlinks using EvalSymlinks for untaring

vacation/vacation.go

@@ -74,6 +74,13 @@ func (ta TarArchive) Decompress(destination string) error {
 	// tarball, which can be seen in the test around there being no directory
 	// metadata.
 	directories := map[string]interface{}{}
+	type header struct {
+		name     string
+		linkname string
+		path     string
+	}
+
+	var symlinkHeaders []header
 
 	tarReader := tar.NewReader(ta.reader)
 	for {
@@ -142,15 +149,30 @@ func (ta TarArchive) Decompress(destination string) error {
 			}
 
 		case tar.TypeSymlink:
-			err = checkExtractPath(filepath.Join(filepath.Dir(hdr.Name), hdr.Linkname), destination)
-			if err != nil {
-				return err
-			}
+			// Collect all of the headers for symlinks so that they can be verified
+			// after all other files are written
+			symlinkHeaders = append(symlinkHeaders, header{
+				name:     hdr.Name,
+				linkname: hdr.Linkname,
+				path:     path,
+			})
+		}
+	}
 
-			err = os.Symlink(hdr.Linkname, path)
-			if err != nil {
-				return fmt.Errorf("failed to extract symlink: %s", err)
-			}
+	for _, h := range symlinkHeaders {
+		_, err := filepath.EvalSymlinks(filepath.Join(destination, h.linkname))
+		if err != nil {
+			return err
+		}
+
+		err = checkExtractPath(filepath.Join(filepath.Dir(h.name), h.linkname), destination)
+		if err != nil {
+			return err
+		}
+
+		err = os.Symlink(h.linkname, h.path)
+		if err != nil {
+			return fmt.Errorf("failed to extract symlink: %s", err)
 		}
 	}
 

vacation/vacation_tar_test.go

@@ -41,6 +41,10 @@ func testVacationTar(t *testing.T, context spec.G, it spec.S) {
 			_, err = tw.Write(nil)
 			Expect(err).NotTo(HaveOccurred())
 
+			Expect(tw.WriteHeader(&tar.Header{Name: "symlink", Mode: 0755, Size: int64(0), Typeflag: tar.TypeSymlink, Linkname: "first"})).To(Succeed())
+			_, err = tw.Write([]byte{})
+			Expect(err).NotTo(HaveOccurred())
+
 			nestedFile := filepath.Join("some-dir", "some-other-dir", "some-file")
 			Expect(tw.WriteHeader(&tar.Header{Name: nestedFile, Mode: 0755, Size: int64(len(nestedFile))})).To(Succeed())
 			_, err = tw.Write([]byte(nestedFile))
@@ -52,10 +56,6 @@ func testVacationTar(t *testing.T, context spec.G, it spec.S) {
 				Expect(err).NotTo(HaveOccurred())
 			}
 
-			Expect(tw.WriteHeader(&tar.Header{Name: "symlink", Mode: 0755, Size: int64(0), Typeflag: tar.TypeSymlink, Linkname: "first"})).To(Succeed())
-			_, err = tw.Write([]byte{})
-			Expect(err).NotTo(HaveOccurred())
-
 			Expect(tw.Close()).To(Succeed())
 
 			tarArchive = vacation.NewTarArchive(bytes.NewReader(buffer.Bytes()))
@@ -229,7 +229,7 @@ func testVacationTar(t *testing.T, context spec.G, it spec.S) {
 				})
 			})
 
-			context("when it tries to symlink that tries to link to a file outside of the directory", func() {
+			context("when it tries to symlinkto a file that does not exist", func() {
 				var zipSlipSymlinkTar vacation.TarArchive
 
 				it.Before(func() {
@@ -249,6 +249,33 @@ func testVacationTar(t *testing.T, context spec.G, it spec.S) {
 
 				it("returns an error", func() {
 					err := zipSlipSymlinkTar.Decompress(tempDir)
+					Expect(err).To(MatchError(ContainSubstring("no such file or directory")))
+				})
+			})
+
+			context("when it tries to symlink that tries to link to a file outside of the directory", func() {
+				var zipSlipSymlinkTar vacation.TarArchive
+
+				it.Before(func() {
+					var err error
+
+					Expect(os.MkdirAll(filepath.Join(tempDir, "sub-dir"), os.ModePerm)).To(Succeed())
+					Expect(os.WriteFile(filepath.Join(tempDir, "some-file"), nil, 0644)).To(Succeed())
+
+					buffer := bytes.NewBuffer(nil)
+					tw := tar.NewWriter(buffer)
+
+					Expect(tw.WriteHeader(&tar.Header{Name: "symlink", Mode: 0755, Size: int64(0), Typeflag: tar.TypeSymlink, Linkname: filepath.Join("..", "some-file")})).To(Succeed())
+					_, err = tw.Write([]byte{})
+					Expect(err).NotTo(HaveOccurred())
+
+					Expect(tw.Close()).To(Succeed())
+
+					zipSlipSymlinkTar = vacation.NewTarArchive(bytes.NewReader(buffer.Bytes()))
+				})
+
+				it("returns an error", func() {
+					err := zipSlipSymlinkTar.Decompress(filepath.Join(tempDir, "sub-dir"))
 					Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("illegal file path %q: the file path does not occur within the destination directory", filepath.Join("..", "some-file")))))
 				})
 			})
@@ -270,7 +297,8 @@ func testVacationTar(t *testing.T, context spec.G, it spec.S) {
 
 					// Create a symlink in the target to force the new symlink create to
 					// fail
-					Expect(os.Symlink("something", filepath.Join(tempDir, "symlink"))).To(Succeed())
+					Expect(os.WriteFile(filepath.Join(tempDir, "some-file"), nil, 0644)).To(Succeed())
+					Expect(os.Symlink("some-file", filepath.Join(tempDir, "symlink"))).To(Succeed())
 
 					brokenSymlinkTar = vacation.NewTarArchive(bytes.NewReader(buffer.Bytes()))
 				})