Skip to content

Commit

Permalink
Added test cases for archive formats
Browse files Browse the repository at this point in the history
the extraction process now continues with other files  by default even if there is a dot-dot filename, which is of course skipped.
The test evilarchives have been generated with following script:
https://github.com/giuliocomi/evilarchiver
Command:
$ mkdir safedir
$ touch security_test.txt safedir/safefile
$ python2 evilarchiver.py -e security_test.txt -n ../evilfile -s safedir/safefile
  • Loading branch information
giuliocomi authored and petemoore committed Sep 28, 2020
1 parent 8217ed3 commit a723054
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 3 deletions.
52 changes: 52 additions & 0 deletions archiver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 }

2 changes: 1 addition & 1 deletion rar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion tar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Binary file added testdata/testarchives/evilarchives/evil.tar
Binary file not shown.
Binary file added testdata/testarchives/evilarchives/evil.tar.bz2
Binary file not shown.
Binary file added testdata/testarchives/evilarchives/evil.tar.gz
Binary file not shown.
Binary file added testdata/testarchives/evilarchives/evil.zip
Binary file not shown.
2 changes: 1 addition & 1 deletion zip.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit a723054

Please sign in to comment.