diff --git a/evil_generator/main.go b/evil_generator/main.go
new file mode 100644
index 0000000..4e93732
--- /dev/null
+++ b/evil_generator/main.go
@@ -0,0 +1,106 @@
+// This utility is used to generate the archives used as testdata for zipslip vulnerability
+package main
+
+//go:generate go run main.go ../testdata/zipslip
+
+import (
+ "archive/tar"
+ "archive/zip"
+ "bytes"
+ "log"
+ "os"
+
+ "github.com/arduino/go-paths-helper"
+)
+
+func main() {
+ if len(os.Args) != 2 {
+ log.Fatal("Missing output directory")
+ }
+ outputDir := paths.New(os.Args[1])
+ if outputDir.IsNotDir() {
+ log.Fatalf("Output path %s is not a directory", outputDir)
+ }
+
+ evilPathTraversalFiles := []string{
+ "..",
+ "../../../../../../../../../../../../../../../../../../../../tmp/evil.txt",
+ "some/path/../../../../../../../../../../../../../../../../../../../../tmp/evil.txt",
+ "/../../../../../../../../../../../../../../../../../../../../tmp/evil.txt",
+ "/some/path/../../../../../../../../../../../../../../../../../../../../tmp/evil.txt",
+ }
+ winSpecificPathTraversalFiles := []string{
+ "..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\tmp\\evil.txt",
+ "some\\path\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\tmp\\evil.txt",
+ "\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\tmp\\evil.txt",
+ "\\some\\path\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\tmp\\evil.txt",
+ }
+ winSpecificPathTraversalFiles = append(winSpecificPathTraversalFiles, evilPathTraversalFiles...)
+
+ // Generate evil zip
+ {
+ buf := new(bytes.Buffer)
+ w := zip.NewWriter(buf)
+ for _, file := range winSpecificPathTraversalFiles {
+ if f, err := w.Create(file); err != nil {
+ log.Fatal(err)
+ } else if _, err = f.Write([]byte("TEST")); err != nil {
+ log.Fatal(err)
+ }
+ }
+ if err := w.Close(); err != nil {
+ log.Fatal(err)
+ }
+ if err := outputDir.Join("evil.zip").WriteFile(buf.Bytes()); err != nil {
+ log.Fatal(err)
+ }
+ }
+
+ // Generate evil tar
+ {
+ buf := new(bytes.Buffer)
+ w := tar.NewWriter(buf)
+ for _, file := range evilPathTraversalFiles {
+ if err := w.WriteHeader(&tar.Header{
+ Name: file,
+ Size: 4,
+ Mode: 0666,
+ }); err != nil {
+ log.Fatal(err)
+ }
+ if _, err := w.Write([]byte("TEST")); err != nil {
+ log.Fatal(err)
+ }
+ }
+ if err := w.Close(); err != nil {
+ log.Fatal(err)
+ }
+ if err := outputDir.Join("evil.tar").WriteFile(buf.Bytes()); err != nil {
+ log.Fatal(err)
+ }
+ }
+
+ // Generate evil tar for windows
+ {
+ buf := new(bytes.Buffer)
+ w := tar.NewWriter(buf)
+ for _, file := range winSpecificPathTraversalFiles {
+ if err := w.WriteHeader(&tar.Header{
+ Name: file,
+ Size: 4,
+ Mode: 0666,
+ }); err != nil {
+ log.Fatal(err)
+ }
+ if _, err := w.Write([]byte("TEST")); err != nil {
+ log.Fatal(err)
+ }
+ }
+ if err := w.Close(); err != nil {
+ log.Fatal(err)
+ }
+ if err := outputDir.Join("evil-win.tar").WriteFile(buf.Bytes()); err != nil {
+ log.Fatal(err)
+ }
+ }
+}
diff --git a/extractor.go b/extractor.go
index 6bf9085..4fbfc29 100644
--- a/extractor.go
+++ b/extractor.go
@@ -132,7 +132,10 @@ func (e *Extractor) Tar(ctx context.Context, body io.Reader, location string, re
continue
}
- path = filepath.Join(location, path)
+ if path, err = safeJoin(location, path); err != nil {
+ continue
+ }
+
info := header.FileInfo()
switch header.Typeflag {
@@ -232,7 +235,10 @@ func (e *Extractor) Zip(ctx context.Context, body io.Reader, location string, re
continue
}
- path = filepath.Join(location, path)
+ if path, err = safeJoin(location, path); err != nil {
+ continue
+ }
+
info := header.FileInfo()
switch {
@@ -316,3 +322,16 @@ func match(r io.Reader) (io.Reader, types.Type, error) {
return r, typ, err
}
+
+// safeJoin performs a filepath.Join of 'parent' and 'subdir' but returns an error
+// if the resulting path points outside of 'parent'.
+func safeJoin(parent, subdir string) (string, error) {
+ res := filepath.Join(parent, subdir)
+ if !strings.HasSuffix(parent, string(os.PathSeparator)) {
+ parent += string(os.PathSeparator)
+ }
+ if !strings.HasPrefix(res, parent) {
+ return res, errors.Errorf("unsafe path join: '%s' with '%s'", parent, subdir)
+ }
+ return res, nil
+}
diff --git a/extractor_test.go b/extractor_test.go
index 60ce252..a56a863 100644
--- a/extractor_test.go
+++ b/extractor_test.go
@@ -3,12 +3,15 @@ package extract_test
import (
"bytes"
"context"
+ "fmt"
"io/ioutil"
"os"
"path/filepath"
+ "runtime"
"testing"
"github.com/codeclysm/extract/v3"
+ "github.com/stretchr/testify/require"
)
func TestExtractor_Tar(t *testing.T) {
@@ -77,6 +80,40 @@ func TestExtractor_Zip(t *testing.T) {
testWalk(t, tmp, files)
}
+func TestZipSlipHardening(t *testing.T) {
+ {
+ logger := &LoggingFS{}
+ extractor := extract.Extractor{FS: logger}
+ data, err := os.Open("testdata/zipslip/evil.zip")
+ require.NoError(t, err)
+ require.NoError(t, extractor.Zip(context.Background(), data, "/tmp/test", nil))
+ require.NoError(t, data.Close())
+ fmt.Print(logger)
+ require.Empty(t, logger.Journal)
+ }
+ {
+ logger := &LoggingFS{}
+ extractor := extract.Extractor{FS: logger}
+ data, err := os.Open("testdata/zipslip/evil.tar")
+ require.NoError(t, err)
+ require.NoError(t, extractor.Tar(context.Background(), data, "/tmp/test", nil))
+ require.NoError(t, data.Close())
+ fmt.Print(logger)
+ require.Empty(t, logger.Journal)
+ }
+
+ if runtime.GOOS == "windows" {
+ logger := &LoggingFS{}
+ extractor := extract.Extractor{FS: logger}
+ data, err := os.Open("testdata/zipslip/evil-win.tar")
+ require.NoError(t, err)
+ require.NoError(t, extractor.Tar(context.Background(), data, "/tmp/test", nil))
+ require.NoError(t, data.Close())
+ fmt.Print(logger)
+ require.Empty(t, logger.Journal)
+ }
+}
+
// MockDisk is a disk that chroots to a directory
type MockDisk struct {
Base string
diff --git a/go.mod b/go.mod
index 6fa9fd1..f088227 100644
--- a/go.mod
+++ b/go.mod
@@ -3,6 +3,7 @@ module github.com/codeclysm/extract/v3
go 1.14
require (
+ github.com/arduino/go-paths-helper v1.2.0
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/h2non/filetype v1.0.6
github.com/juju/errors v0.0.0-20181118221551-089d3ea4e4d5
diff --git a/go.sum b/go.sum
index 599ffb6..6b33dcb 100644
--- a/go.sum
+++ b/go.sum
@@ -1,3 +1,5 @@
+github.com/arduino/go-paths-helper v1.2.0 h1:qDW93PR5IZUN/jzO4rCtexiwF8P4OIcOmcSgAYLZfY4=
+github.com/arduino/go-paths-helper v1.2.0/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
diff --git a/loggingfs_test.go b/loggingfs_test.go
new file mode 100644
index 0000000..b3d20a9
--- /dev/null
+++ b/loggingfs_test.go
@@ -0,0 +1,80 @@
+package extract_test
+
+import (
+ "fmt"
+ "os"
+)
+
+// LoggingFS is a disk that logs every operation, useful for unit-testing.
+type LoggingFS struct {
+ Journal []*LoggedOp
+}
+
+// LoggedOp is an operation logged in a LoggingFS journal.
+type LoggedOp struct {
+ Op string
+ Path string
+ OldPath string
+ Mode os.FileMode
+ Flags int
+}
+
+func (op *LoggedOp) String() string {
+ switch op.Op {
+ case "link":
+ return fmt.Sprintf("link %s -> %s", op.Path, op.OldPath)
+ case "symlink":
+ return fmt.Sprintf("symlink %s -> %s", op.Path, op.OldPath)
+ case "mkdirall":
+ return fmt.Sprintf("mkdirall %v %s", op.Mode, op.Path)
+ case "open":
+ return fmt.Sprintf("open %v %s (flags=%04x)", op.Mode, op.Path, op.Flags)
+ }
+ panic("unknown LoggedOP " + op.Op)
+}
+
+func (m *LoggingFS) Link(oldname, newname string) error {
+ m.Journal = append(m.Journal, &LoggedOp{
+ Op: "link",
+ OldPath: oldname,
+ Path: newname,
+ })
+ return nil
+}
+
+func (m *LoggingFS) MkdirAll(path string, perm os.FileMode) error {
+ m.Journal = append(m.Journal, &LoggedOp{
+ Op: "mkdirall",
+ Path: path,
+ Mode: perm,
+ })
+ return nil
+}
+
+func (m *LoggingFS) Symlink(oldname, newname string) error {
+ m.Journal = append(m.Journal, &LoggedOp{
+ Op: "symlink",
+ OldPath: oldname,
+ Path: newname,
+ })
+ return nil
+}
+
+func (m *LoggingFS) OpenFile(name string, flags int, perm os.FileMode) (*os.File, error) {
+ m.Journal = append(m.Journal, &LoggedOp{
+ Op: "open",
+ Path: name,
+ Mode: perm,
+ Flags: flags,
+ })
+ return os.OpenFile(os.DevNull, flags, perm)
+}
+
+func (m *LoggingFS) String() string {
+ res := ""
+ for _, op := range m.Journal {
+ res += op.String()
+ res += "\n"
+ }
+ return res
+}
diff --git a/safejoin_test.go b/safejoin_test.go
new file mode 100644
index 0000000..66c019b
--- /dev/null
+++ b/safejoin_test.go
@@ -0,0 +1,35 @@
+package extract
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+func TestSafeJoin(t *testing.T) {
+ ok := func(parent, subdir string) {
+ _, err := safeJoin(parent, subdir)
+ require.NoError(t, err, "joining '%s' and '%s'", parent, subdir)
+ }
+ ko := func(parent, subdir string) {
+ _, err := safeJoin(parent, subdir)
+ require.Error(t, err, "joining '%s' and '%s'", parent, subdir)
+ }
+ ok("/", "more/path")
+ ok("/path", "more/path")
+ ok("/path/", "more/path")
+ ok("/path/subdir", "more/path")
+ ok("/path/subdir/", "more/path")
+
+ ok("/", "..") // ! since we are extracting to / is ok-ish to accept ".."?
+ ko("/path", "..")
+ ko("/path/", "..")
+ ko("/path/subdir", "..")
+ ko("/path/subdir/", "..")
+
+ ok("/", "../pathpath") // ! since we are extracting to / is ok-ish to accept "../pathpath"?
+ ko("/path", "../pathpath")
+ ko("/path/", "../pathpath")
+ ko("/path/subdir", "../pathpath")
+ ko("/path/subdir/", "../pathpath")
+}
diff --git a/testdata/zipslip/evil-win.tar b/testdata/zipslip/evil-win.tar
new file mode 100644
index 0000000..b07302f
Binary files /dev/null and b/testdata/zipslip/evil-win.tar differ
diff --git a/testdata/zipslip/evil.tar b/testdata/zipslip/evil.tar
new file mode 100644
index 0000000..e42cedf
Binary files /dev/null and b/testdata/zipslip/evil.tar differ
diff --git a/testdata/zipslip/evil.zip b/testdata/zipslip/evil.zip
new file mode 100644
index 0000000..8353c00
Binary files /dev/null and b/testdata/zipslip/evil.zip differ