diff --git a/archiver_test.go b/archiver_test.go index ea8b871d..36b7cafe 100644 --- a/archiver_test.go +++ b/archiver_test.go @@ -452,6 +452,57 @@ func symmetricTest(t *testing.T, formatName, dest string, testSymlinks, testMode } } +// test at runtime if the CheckFilename function is behaving properly for the archive formats +func TestSafeExtraction(t *testing.T) { + + testArchives := []string{ + "testdata/testarchives/evilarchives/evil.zip", + "testdata/testarchives/evilarchives/evil.tar", + "testdata/testarchives/evilarchives/evil.tar.gz", + "testdata/testarchives/evilarchives/evil.tar.bz2", + } + + for _, archiveName := range testArchives { + + expected := true // 'evilfile' should not be extracted outside of destination directory and 'safefile' should be extracted anyway in the destination folder anyway + + if _, err := os.Stat(archiveName); os.IsNotExist(err) { + t.Errorf("archive not found") + } + + actual := CheckFilenames(archiveName) + + if actual != expected { + t.Errorf("CheckFilename is misbehaving for archive format type %s", filepath.Ext(archiveName)) + } + } +} + +func CheckFilenames(archiveName string) bool { + + evilNotExtracted := false // by default we cannot assume that the path traversal filename is mitigated by CheckFilename + safeExtracted := false // by default we cannot assume that a benign file can be extracted successfully + + // clean the destination folder after this test + defer os.RemoveAll("testdata/testarchives/destarchives/") + + err := Unarchive(archiveName, "testdata/testarchives/destarchives/") + if err != nil { + fmt.Println(err) + } + + // is 'evilfile' prevented to be extracted outside of the destination folder? + if _, err := os.Stat("testdata/testarchives/evilfile"); os.IsNotExist(err) { + evilNotExtracted = true + } + // is 'safefile' safely extracted without errors inside the destination path? + if _, err := os.Stat("testdata/testarchives/destarchives/safedir/safefile"); !os.IsNotExist(err) { + safeExtracted = true + } + + return evilNotExtracted && safeExtracted +} + var archiveFormats = []interface{}{ DefaultZip, DefaultTar, @@ -484,3 +535,4 @@ func (ffi fakeFileInfo) Mode() os.FileMode { return ffi.mode } func (ffi fakeFileInfo) ModTime() time.Time { return ffi.modTime } func (ffi fakeFileInfo) IsDir() bool { return ffi.isDir } func (ffi fakeFileInfo) Sys() interface{} { return ffi.sys } + diff --git a/rar.go b/rar.go index 4677b7b5..8c42372f 100644 --- a/rar.go +++ b/rar.go @@ -105,7 +105,7 @@ func (r *Rar) Unarchive(source, destination string) error { break } if err != nil { - if r.ContinueOnError { + if r.ContinueOnError || strings.Contains(err.Error(), "illegal file path") { log.Printf("[ERROR] Reading file in rar archive: %v", err) continue } diff --git a/tar.go b/tar.go index 6942aedf..9434748f 100644 --- a/tar.go +++ b/tar.go @@ -161,7 +161,7 @@ func (t *Tar) Unarchive(source, destination string) error { break } if err != nil { - if t.ContinueOnError { + if t.ContinueOnError || strings.Contains(err.Error(), "illegal file path") { log.Printf("[ERROR] Reading file in tar archive: %v", err) continue } diff --git a/testdata/testarchives/evilarchives/evil.tar b/testdata/testarchives/evilarchives/evil.tar new file mode 100644 index 00000000..9fe114ba Binary files /dev/null and b/testdata/testarchives/evilarchives/evil.tar differ diff --git a/testdata/testarchives/evilarchives/evil.tar.bz2 b/testdata/testarchives/evilarchives/evil.tar.bz2 new file mode 100644 index 00000000..4b979e61 Binary files /dev/null and b/testdata/testarchives/evilarchives/evil.tar.bz2 differ diff --git a/testdata/testarchives/evilarchives/evil.tar.gz b/testdata/testarchives/evilarchives/evil.tar.gz new file mode 100644 index 00000000..6a68871a Binary files /dev/null and b/testdata/testarchives/evilarchives/evil.tar.gz differ diff --git a/testdata/testarchives/evilarchives/evil.zip b/testdata/testarchives/evilarchives/evil.zip new file mode 100644 index 00000000..665e1856 Binary files /dev/null and b/testdata/testarchives/evilarchives/evil.zip differ diff --git a/zip.go b/zip.go index 2b6f03c2..5af36aba 100644 --- a/zip.go +++ b/zip.go @@ -225,7 +225,7 @@ func (z *Zip) Unarchive(source, destination string) error { break } if err != nil { - if z.ContinueOnError { + if z.ContinueOnError || strings.Contains(err.Error(), "illegal file path") { log.Printf("[ERROR] Reading file in zip archive: %v", err) continue }