From 7ef86db1333bd7d433a9ea78f19bbd8cb5007d63 Mon Sep 17 00:00:00 2001 From: johnarok <21272981+johnarok@users.noreply.github.com> Date: Wed, 5 Sep 2018 19:45:34 -0700 Subject: [PATCH 1/2] Fixes #82 by processing destination string through filepath.Clean --- archiver.go | 2 +- archiver_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/archiver.go b/archiver.go index 32baf3e0..6bf20162 100644 --- a/archiver.go +++ b/archiver.go @@ -112,7 +112,7 @@ func sanitizeExtractPath(filePath string, destination string) error { // 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) { + if !strings.HasPrefix(destpath, filepath.Clean(destination)) { return fmt.Errorf("%s: illegal file path", filePath) } return nil diff --git a/archiver_test.go b/archiver_test.go index 22de14ea..5ea5329b 100644 --- a/archiver_test.go +++ b/archiver_test.go @@ -19,6 +19,7 @@ func TestArchiver(t *testing.T) { } testWriteRead(t, name, ar) testMakeOpen(t, name, ar) + testMakeOpenWithDestinationEndingInSlash(t, name, ar) }) } } @@ -83,6 +84,39 @@ func testMakeOpen(t *testing.T, name string, ar Archiver) { symmetricTest(t, name, dest) } +// testMakeOpenWithDestinationEndingInSlash is similar to testMakeOpen except that +// it tests the case where destination path has a terminating forward slash especially +// on Windows os. +func testMakeOpenWithDestinationEndingInSlash(t *testing.T, name string, ar Archiver) { + tmp, err := ioutil.TempDir("", "archiver") + if err != nil { + t.Fatalf("[%s] %v", name, err) + } + defer os.RemoveAll(tmp) + + // Test creating archive + outfile := filepath.Join(tmp, "test-"+name) + err = ar.Make(outfile, []string{"testdata"}) + if err != nil { + t.Fatalf("[%s] making archive: didn't expect an error, but got: %v", name, err) + } + + if !ar.Match(outfile) { + t.Fatalf("[%s] identifying format should be 'true', but got 'false'", name) + } + + // Test extracting archive with destination that has a slash at the end + dest := filepath.Join(tmp, "extraction_test") + os.Mkdir(dest, 0755) + err = ar.Open(outfile, dest+"/") + if err != nil { + t.Fatalf("[%s] extracting archive [%s -> %s]: didn't expect an error, but got: %v", name, outfile, dest, err) + } + + // Check that what was extracted is what was compressed + symmetricTest(t, name, dest) +} + // symmetricTest compares the contents of a destination directory to the contents // of the test corpus and tests that they are equal. func symmetricTest(t *testing.T, name, dest string) { From e1c92d5ffe78903d441bce112a1355663af26fa7 Mon Sep 17 00:00:00 2001 From: johnarok <21272981+johnarok@users.noreply.github.com> Date: Thu, 6 Sep 2018 17:21:06 -0700 Subject: [PATCH 2/2] Minor format change --- archiver_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archiver_test.go b/archiver_test.go index 5ea5329b..305030f7 100644 --- a/archiver_test.go +++ b/archiver_test.go @@ -86,7 +86,7 @@ func testMakeOpen(t *testing.T, name string, ar Archiver) { // testMakeOpenWithDestinationEndingInSlash is similar to testMakeOpen except that // it tests the case where destination path has a terminating forward slash especially -// on Windows os. +// on Windows os. func testMakeOpenWithDestinationEndingInSlash(t *testing.T, name string, ar Archiver) { tmp, err := ioutil.TempDir("", "archiver") if err != nil {