Potential fix for code scanning alert no. 2391: Arbitrary file write extracting an archive containing symbolic links

evetest/utils/tar.go

@@ -9,7 +9,8 @@
 	"fmt"
 	"io"
 	"os"
-	"path"
+	"path/filepath"
+	"strings"
 )
 
 // MaxDecompressedContentSize is the maximum size of a file that can be written to disk after decompression.
@@ -18,10 +19,60 @@
 
 // ExtractFromTar extracts files from a tar reader into the destination directory
 func ExtractFromTar(u io.Reader, destination string) error {
-	// path inside tar is relative
-	pathBuilder := func(oldPath string) string {
-		return path.Join(destination, oldPath)
+	cleanDestination := filepath.Clean(destination)
+	if err := os.MkdirAll(cleanDestination, 0o755); err != nil {
+		return fmt.Errorf("ExtractFromTar: MkdirAll(destination) failed: %w", err)
 	}
+	realDestination, err := filepath.EvalSymlinks(cleanDestination)
+	if err != nil {
+		return fmt.Errorf("ExtractFromTar: EvalSymlinks(destination) failed: %w", err)
+	}
+
+	isWithinDestination := func(candidate string) bool {
+		rel, err := filepath.Rel(realDestination, candidate)
+		return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator))
+	}
+
+	resolveArchivePath := func(archivePath string) (string, error) {
+		if filepath.IsAbs(archivePath) {
+			return "", fmt.Errorf("absolute path is not allowed: %s", archivePath)
+		}
+		candidate := filepath.Join(realDestination, archivePath)
+		parent := filepath.Dir(candidate)
+		resolvedParent, err := filepath.EvalSymlinks(parent)
+		if err != nil {
+			return "", fmt.Errorf("cannot resolve parent path %s: %w", parent, err)
+		}
+		finalPath := filepath.Join(resolvedParent, filepath.Base(candidate))
+		if !isWithinDestination(finalPath) {
+			return "", fmt.Errorf("path escapes destination: %s", archivePath)
+		}
+		return finalPath, nil
+	}
+
+	resolveLinkTarget := func(linkName, linkTarget string) (string, error) {
+		linkPath, err := resolveArchivePath(linkName)
+		if err != nil {
+			return "", err
+		}
+		linkParent := filepath.Dir(linkPath)
+		var targetCandidate string
+		if filepath.IsAbs(linkTarget) {
+			targetCandidate = filepath.Clean(linkTarget)
+		} else {
+			targetCandidate = filepath.Join(linkParent, linkTarget)
+		}
+		resolvedTargetParent, err := filepath.EvalSymlinks(filepath.Dir(targetCandidate))
+		if err != nil {
+			return "", fmt.Errorf("cannot resolve symlink target parent %s: %w", filepath.Dir(targetCandidate), err)
+		}
+		finalTarget := filepath.Join(resolvedTargetParent, filepath.Base(targetCandidate))
+		if !isWithinDestination(finalTarget) {
+			return "", fmt.Errorf("symlink target escapes destination: %s", linkTarget)
+		}
+		return finalTarget, nil
+	}
+
 	tarReader := tar.NewReader(u)
 	for {
 		header, err := tarReader.Next()
@@ -33,17 +84,25 @@
 		}
 		switch header.Typeflag {
 		case tar.TypeDir:
-			if err := os.MkdirAll(pathBuilder(header.Name), os.FileMode(header.Mode)); err != nil {
+			dirPath, err := resolveArchivePath(header.Name)
+			if err != nil {
+				return fmt.Errorf("ExtractFromTar: invalid directory path %s: %w", header.Name, err)
+			}
+			if err := os.MkdirAll(dirPath, os.FileMode(header.Mode)); err != nil {
 				return fmt.Errorf("ExtractFromTar: Mkdir() failed: %w", err)
 			}
 		case tar.TypeReg:
-			if _, err := os.Lstat(pathBuilder(header.Name)); err == nil {
-				err = os.Remove(pathBuilder(header.Name))
+			filePath, err := resolveArchivePath(header.Name)
+			if err != nil {
+				return fmt.Errorf("ExtractFromTar: invalid file path %s: %w", header.Name, err)
+			}
+			if _, err := os.Lstat(filePath); err == nil {
+				err = os.Remove(filePath)
 				if err != nil {
 					return fmt.Errorf("ExtractFromTar: cannot remove old file: %w", err)
 				}
 			}
-			outFile, err := os.OpenFile(pathBuilder(header.Name), os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode))
+			outFile, err := os.OpenFile(filePath, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode))
 			if err != nil {
 				return fmt.Errorf("ExtractFromTar: OpenFile() failed: %w", err)
 			}
@@ -60,15 +119,23 @@
 				return fmt.Errorf("ExtractFromTar: outFile.Close() failed: %w", err)
 			}
 		case tar.TypeLink, tar.TypeSymlink:
-			if _, err := os.Lstat(pathBuilder(header.Name)); err == nil {
-				err = os.Remove(pathBuilder(header.Name))
+			linkPath, err := resolveArchivePath(header.Name)
+			if err != nil {
+				return fmt.Errorf("ExtractFromTar: invalid symlink path %s: %w", header.Name, err)
+			}
+			linkTarget, err := resolveLinkTarget(header.Name, header.Linkname)
+			if err != nil {
+				return fmt.Errorf("ExtractFromTar: invalid symlink target %s: %w", header.Linkname, err)
+			}
+			if _, err := os.Lstat(linkPath); err == nil {
+				err = os.Remove(linkPath)
 				if err != nil {
 					return fmt.Errorf("ExtractFromTar: cannot remove old symlink: %w", err)
 				}
 			}
-			if err := os.Symlink(pathBuilder(header.Linkname), pathBuilder(header.Name)); err != nil {
+			if err := os.Symlink(linkTarget, linkPath); err != nil {
 				return fmt.Errorf("ExtractFromTar: Symlink(%s, %s) failed: %w",
-					pathBuilder(header.Name), pathBuilder(header.Linkname), err)
+					linkPath, linkTarget, err)
 			}
 		default:
 			return fmt.Errorf(