Skip to content

Commit

Permalink
Fix false positive ZipSlip when an archive starts with dot (#54)
Browse files Browse the repository at this point in the history
  • Loading branch information
yahavi authored Mar 11, 2024
1 parent e56d3cf commit 2c6edc6
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 17 deletions.
7 changes: 6 additions & 1 deletion unarchive/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ package unarchive
import (
"encoding/json"
"fmt"
"github.com/jfrog/archiver/v3"
"io"
"os"
"path/filepath"
"strings"

"github.com/jfrog/archiver/v3"

"github.com/jfrog/gofrog/datastructures"
)

Expand Down Expand Up @@ -135,6 +136,10 @@ func (u *Unarchiver) byExtension(filename string) (interface{}, error) {

// Make sure the archive is free from Zip Slip and Zip symlinks attacks
func inspectArchive(archive interface{}, localArchivePath, destinationDir string) error {
// If the destination directory ends with a slash, delete it.
// This is necessary to handle a situation where the entry path might be at the root of the destination directory,
// but in such a case "<destination-dir>/" is not a prefix of "<destination-dir>".
destinationDir = strings.TrimSuffix(destinationDir, string(os.PathSeparator))
walker, ok := archive.(archiver.Walker)
if !ok {
return fmt.Errorf("couldn't inspect archive: " + localArchivePath)
Expand Down
34 changes: 18 additions & 16 deletions unarchive/archive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ func TestUnarchive(t *testing.T) {
for _, extension := range tests {
t.Run(extension, func(t *testing.T) {
// Create temp directory
tmpDir, createTempDirCallback := createTempDirWithCallbackAndAssert(t)
defer createTempDirCallback()
tmpDir := t.TempDir()

// Run unarchive on archive created on Unix
err := runUnarchive(t, uarchiver, "unix."+extension, "archives", filepath.Join(tmpDir, "unix"))
assert.NoError(t, err)
Expand Down Expand Up @@ -48,8 +48,7 @@ func TestUnarchiveSymlink(t *testing.T) {
for _, testCase := range unarchiveSymlinksCases {
t.Run(testCase.prefix, func(t *testing.T) {
// Create temp directory
tmpDir, createTempDirCallback := createTempDirWithCallbackAndAssert(t)
defer createTempDirCallback()
tmpDir := t.TempDir()

// Run unarchive
err := runUnarchive(t, uarchiver, testCase.prefix+"."+extension, "archives", tmpDir)
Expand Down Expand Up @@ -84,8 +83,8 @@ func TestUnarchiveZipSlip(t *testing.T) {
for _, test := range tests {
t.Run(test.testType, func(t *testing.T) {
// Create temp directory
tmpDir, createTempDirCallback := createTempDirWithCallbackAndAssert(t)
defer createTempDirCallback()
tmpDir := t.TempDir()

for _, archive := range test.archives {
// Unarchive and make sure an error returns
err := runUnarchive(t, uarchiver, test.testType+"."+archive, "zipslip", tmpDir)
Expand All @@ -103,8 +102,8 @@ func TestUnarchiveWithStripComponents(t *testing.T) {
for _, extension := range tests {
t.Run(extension, func(t *testing.T) {
// Create temp directory
tmpDir, createTempDirCallback := createTempDirWithCallbackAndAssert(t)
defer createTempDirCallback()
tmpDir := t.TempDir()

// Run unarchive on archive created on Unix
err := runUnarchive(t, uarchiver, "strip-components."+extension, "archives", filepath.Join(tmpDir, "unix"))
assert.NoError(t, err)
Expand All @@ -120,16 +119,19 @@ func TestUnarchiveWithStripComponents(t *testing.T) {
}
}

// Test unarchive file with a directory named "." in the root directory
func TestUnarchiveDotDir(t *testing.T) {
// Create temp directory
tmpDir := t.TempDir()

// Run unarchive
err := runUnarchive(t, Unarchiver{}, "dot-dir.tar.gz", "archives", tmpDir+string(os.PathSeparator))
assert.NoError(t, err)
assert.DirExists(t, filepath.Join(tmpDir, "dir"))
}

func runUnarchive(t *testing.T, uarchiver Unarchiver, archiveFileName, sourceDir, targetDir string) error {
archivePath := filepath.Join("testdata", sourceDir, archiveFileName)
assert.True(t, uarchiver.IsSupportedArchive(archivePath))
return uarchiver.Unarchive(filepath.Join("testdata", sourceDir, archiveFileName), archiveFileName, targetDir)
}

func createTempDirWithCallbackAndAssert(t *testing.T) (string, func()) {
tempDirPath, err := os.MkdirTemp("", "archiver_test")
assert.NoError(t, err, "Couldn't create temp dir")
return tempDirPath, func() {
assert.NoError(t, os.RemoveAll(tempDirPath), "Couldn't remove temp dir")
}
}
Binary file added unarchive/testdata/archives/dot-dir.tar.gz
Binary file not shown.

0 comments on commit 2c6edc6

Please sign in to comment.