fix: mholt#65#issuecomment-395988244 - zip slip by symlink

Author: aviadatsnyk

archiver.go

@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
+
+	"github.com/cyphar/filepath-securejoin"
 )
 
 // Archiver represent a archive format
@@ -111,8 +113,12 @@ func sanitizeExtractPath(filePath string, destination string) error {
 	// to avoid zip slip (writing outside of the destination), we resolve
 	// the target path, and make sure it's nested in the intended
 	// destination, or bail otherwise.
-	destpath := filepath.Join(destination, filePath)
-	if !strings.HasPrefix(destpath, destination) {
+	destpath, err := securejoin.SecureJoin(destination, filePath)
+	if err != nil {
+		return fmt.Errorf(
+			"%s: error calculating destination path", filePath)
+	}
+	if !strings.HasPrefix(destpath, destination+string(os.PathSeparator)) {
 		return fmt.Errorf("%s: illegal file path", filePath)
 	}
 	return nil

rar.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/cyphar/filepath-securejoin"
 	"github.com/nwaples/rardecode"
 )
 
@@ -77,8 +78,10 @@ func (rarFormat) Read(input io.Reader, destination string) error {
 			return err
 		}
 
-		destpath := filepath.Join(destination, header.Name)
-
+		destpath, err := securejoin.SecureJoin(destination, header.Name)
+		if err != nil {
+			return err
+		}
 		if header.IsDir {
 			err = mkdir(destpath)
 			if err != nil {

tar.go

@@ -9,6 +9,8 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+
+	"github.com/cyphar/filepath-securejoin"
 )
 
 // Tar is for Tar format
@@ -224,7 +226,10 @@ func untarFile(tr *tar.Reader, header *tar.Header, destination string) error {
 		return err
 	}
 
-	destpath := filepath.Join(destination, header.Name)
+	destpath, err := securejoin.SecureJoin(destination, header.Name)
+	if err != nil {
+		return err
+	}
 
 	switch header.Typeflag {
 	case tar.TypeDir:
@@ -234,7 +239,12 @@ func untarFile(tr *tar.Reader, header *tar.Header, destination string) error {
 	case tar.TypeSymlink:
 		return writeNewSymbolicLink(destpath, header.Linkname)
 	case tar.TypeLink:
-		return writeNewHardLink(destpath, filepath.Join(destination, header.Linkname))
+		linkpath, err := securejoin.SecureJoin(destination, header.Linkname)
+		if err != nil {
+			return err
+		}
+		return writeNewHardLink(
+			destpath, linkpath)
 	default:
 		return fmt.Errorf("%s: unknown type flag: %c", header.Name, header.Typeflag)
 	}

zip.go

@@ -12,6 +12,8 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
+
+	"github.com/cyphar/filepath-securejoin"
 )
 
 // Zip is for Zip format
@@ -192,8 +194,12 @@ func unzipFile(zf *zip.File, destination string) error {
 		return err
 	}
 
+	destpath, err := securejoin.SecureJoin(destination, zf.Name)
+	if err != nil {
+		return fmt.Errorf("%s: calculating file path: %v", zf.Name, err)
+	}
 	if strings.HasSuffix(zf.Name, "/") {
-		return mkdir(filepath.Join(destination, zf.Name))
+		return mkdir(destpath)
 	}
 
 	rc, err := zf.Open()
@@ -202,7 +208,7 @@ func unzipFile(zf *zip.File, destination string) error {
 	}
 	defer rc.Close()
 
-	return writeNewFile(filepath.Join(destination, zf.Name), rc, zf.FileInfo().Mode())
+	return writeNewFile(destpath, rc, zf.FileInfo().Mode())
 }
 
 // compressedFormats is a (non-exhaustive) set of lowercased