Skip to content

Commit

Permalink
Merge commit from fork
Browse files Browse the repository at this point in the history 
More zipslip cases
  • Loading branch information
@cmaglie
cmaglie authored Aug 8, 2024
2 parents 685dc25 + b347945 commit 4a98568
Show file tree
Hide file tree
Showing 5 changed files with 265 additions and 35 deletions.
23 changes: 23 additions & 0 deletions evil_generator/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ func main() {
log.Fatalf("Output path %s is not a directory", outputDir)
}

generateEvilZipSlip(outputDir)
generateEvilSymLinkPathTraversalTar(outputDir)
}

func generateEvilZipSlip(outputDir *paths.Path) {
evilPathTraversalFiles := []string{
"..",
"../../../../../../../../../../../../../../../../../../../../tmp/evil.txt",
Expand Down Expand Up @@ -104,3 +109,21 @@ func main() {
}
}
}

func generateEvilSymLinkPathTraversalTar(outputDir *paths.Path) {
outputTarFile, err := outputDir.Join("evil-link-traversal.tar").Create()
if err != nil {
log.Fatal(err)
}
defer outputTarFile.Close()

tw := tar.NewWriter(outputTarFile)
defer tw.Close()

if err := tw.WriteHeader(&tar.Header{
Name: "leak", Linkname: "../../../../../../../../../../../../../../../tmp/something-important",
Mode: 0o0777, Size: 0, Typeflag: tar.TypeLink,
}); err != nil {
log.Fatal(err)
}
}
45 changes: 33 additions & 12 deletions extractor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,10 @@ func (e *Extractor) Tar(ctx context.Context, body io.Reader, location string, re
name = rename(name)
}

name = filepath.Join(location, name)
name, err = safeJoin(location, name)
if err != nil {
continue
}
links = append(links, &link{Path: path, Name: name})
case tar.TypeSymlink:
symlinks = append(symlinks, &link{Path: path, Name: header.Linkname})
Expand All @@ -237,6 +240,32 @@ func (e *Extractor) Tar(ctx context.Context, body io.Reader, location string, re
}
}

if err := e.extractSymlinks(ctx, symlinks); err != nil {
return err
}

return nil
}

func (e *Extractor) extractSymlinks(ctx context.Context, symlinks []*link) error {
for _, symlink := range symlinks {
select {
case <-ctx.Done():
return errors.New("interrupted")
default:
}

// Make a placeholder and replace it after unpacking everything
_ = e.FS.Remove(symlink.Path)
f, err := e.FS.OpenFile(symlink.Path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, os.FileMode(0666))
if err != nil {
return fmt.Errorf("creating symlink placeholder %s: %w", symlink.Path, err)
}
if err := f.Close(); err != nil {
return fmt.Errorf("creating symlink placeholder %s: %w", symlink.Path, err)
}
}

for _, symlink := range symlinks {
select {
case <-ctx.Done():
Expand All @@ -248,6 +277,7 @@ func (e *Extractor) Tar(ctx context.Context, body io.Reader, location string, re
return errors.Annotatef(err, "Create link %s", symlink.Path)
}
}

return nil
}

Expand Down Expand Up @@ -340,17 +370,8 @@ func (e *Extractor) Zip(ctx context.Context, body io.Reader, location string, re
}
}

// Now we make another pass creating the links
for _, link := range links {
select {
case <-ctx.Done():
return errors.New("interrupted")
default:
}
_ = e.FS.Remove(link.Path)
if err := e.FS.Symlink(link.Name, link.Path); err != nil {
return errors.Annotatef(err, "Create link %s", link.Path)
}
if err := e.extractSymlinks(ctx, links); err != nil {
return err
}

return nil
Expand Down
171 changes: 166 additions & 5 deletions extractor_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package extract_test

import (
"archive/tar"
"archive/zip"
"bytes"
"context"
"fmt"
Expand Down Expand Up @@ -65,7 +67,7 @@ func testArchive(t *testing.T, archivePath *paths.Path) {
}

func TestZipSlipHardening(t *testing.T) {
{
t.Run("ZipTraversal", func(t *testing.T) {
logger := &LoggingFS{}
extractor := extract.Extractor{FS: logger}
data, err := os.Open("testdata/zipslip/evil.zip")
Expand All @@ -74,8 +76,9 @@ func TestZipSlipHardening(t *testing.T) {
require.NoError(t, data.Close())
fmt.Print(logger)
require.Empty(t, logger.Journal)
}
{
})

t.Run("TarTraversal", func(t *testing.T) {
logger := &LoggingFS{}
extractor := extract.Extractor{FS: logger}
data, err := os.Open("testdata/zipslip/evil.tar")
Expand All @@ -84,9 +87,23 @@ func TestZipSlipHardening(t *testing.T) {
require.NoError(t, data.Close())
fmt.Print(logger)
require.Empty(t, logger.Journal)
}
})

t.Run("TarLinkTraversal", func(t *testing.T) {
logger := &LoggingFS{}
extractor := extract.Extractor{FS: logger}
data, err := os.Open("testdata/zipslip/evil-link-traversal.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" {
t.Run("WindowsTarTraversal", func(t *testing.T) {
if runtime.GOOS != "windows" {
t.Skip("Skipped on non-Windows host")
}
logger := &LoggingFS{}
extractor := extract.Extractor{FS: logger}
data, err := os.Open("testdata/zipslip/evil-win.tar")
Expand All @@ -95,7 +112,151 @@ func TestZipSlipHardening(t *testing.T) {
require.NoError(t, data.Close())
fmt.Print(logger)
require.Empty(t, logger.Journal)
})
}

func mkTempDir(t *testing.T) *paths.Path {
tmp, err := paths.MkTempDir("", "test")
require.NoError(t, err)
t.Cleanup(func() { tmp.RemoveAll() })
return tmp
}

func TestSymLinkMazeHardening(t *testing.T) {
addTarSymlink := func(t *testing.T, tw *tar.Writer, new, old string) {
err := tw.WriteHeader(&tar.Header{
Mode: 0o0777, Typeflag: tar.TypeSymlink, Name: new, Linkname: old,
})
require.NoError(t, err)
}
addZipSymlink := func(t *testing.T, zw *zip.Writer, new, old string) {
h := &zip.FileHeader{Name: new, Method: zip.Deflate}
h.SetMode(os.ModeSymlink)
w, err := zw.CreateHeader(h)
require.NoError(t, err)
_, err = w.Write([]byte(old))
require.NoError(t, err)
}

t.Run("TarWithSymlinkToAbsPath", func(t *testing.T) {
// Create target dir
tmp := mkTempDir(t)
targetDir := tmp.Join("test")
require.NoError(t, targetDir.Mkdir())

// Make a tar archive with symlink maze
outputTar := bytes.NewBuffer(nil)
tw := tar.NewWriter(outputTar)
addTarSymlink(t, tw, "aaa", tmp.String())
addTarSymlink(t, tw, "aaa/sym", "something")
require.NoError(t, tw.Close())

// Run extract
extractor := extract.Extractor{FS: &LoggingFS{}}
require.Error(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
require.NoFileExists(t, tmp.Join("sym").String())
})

t.Run("ZipWithSymlinkToAbsPath", func(t *testing.T) {
// Create target dir
tmp := mkTempDir(t)
targetDir := tmp.Join("test")
require.NoError(t, targetDir.Mkdir())

// Make a zip archive with symlink maze
outputZip := bytes.NewBuffer(nil)
zw := zip.NewWriter(outputZip)
addZipSymlink(t, zw, "aaa", tmp.String())
addZipSymlink(t, zw, "aaa/sym", "something")
require.NoError(t, zw.Close())

// Run extract
extractor := extract.Extractor{FS: &LoggingFS{}}
err := extractor.Zip(context.Background(), outputZip, targetDir.String(), nil)
require.NoFileExists(t, tmp.Join("sym").String())
require.Error(t, err)
})

t.Run("TarWithSymlinkToRelativeExternalPath", func(t *testing.T) {
// Create target dir
tmp := mkTempDir(t)
targetDir := tmp.Join("test")
require.NoError(t, targetDir.Mkdir())
checkDir := tmp.Join("secret")
require.NoError(t, checkDir.MkdirAll())

// Make a tar archive with regular symlink maze
outputTar := bytes.NewBuffer(nil)
tw := tar.NewWriter(outputTar)
addTarSymlink(t, tw, "aaa", "../secret")
addTarSymlink(t, tw, "aaa/sym", "something")
require.NoError(t, tw.Close())

extractor := extract.Extractor{FS: &LoggingFS{}}
require.Error(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
require.NoFileExists(t, checkDir.Join("sym").String())
})

t.Run("TarWithSymlinkToInternalPath", func(t *testing.T) {
// Create target dir
tmp := mkTempDir(t)
targetDir := tmp.Join("test")
require.NoError(t, targetDir.Mkdir())

// Make a tar archive with regular symlink maze
outputTar := bytes.NewBuffer(nil)
tw := tar.NewWriter(outputTar)
require.NoError(t, tw.WriteHeader(&tar.Header{Mode: 0o0777, Typeflag: tar.TypeDir, Name: "tmp"}))
addTarSymlink(t, tw, "aaa", "tmp")
addTarSymlink(t, tw, "aaa/sym", "something")
require.NoError(t, tw.Close())

extractor := extract.Extractor{FS: &LoggingFS{}}
require.Error(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
require.NoFileExists(t, targetDir.Join("tmp", "sym").String())
})

t.Run("TarWithDoubleSymlinkToExternalPath", func(t *testing.T) {
// Create target dir
tmp := mkTempDir(t)
targetDir := tmp.Join("test")
require.NoError(t, targetDir.Mkdir())
fmt.Println("TMP:", tmp)
fmt.Println("TARGET DIR:", targetDir)

// Make a tar archive with regular symlink maze
outputTar := bytes.NewBuffer(nil)
tw := tar.NewWriter(outputTar)
tw.WriteHeader(&tar.Header{Name: "fake", Mode: 0777, Typeflag: tar.TypeDir})
addTarSymlink(t, tw, "sym-maze", tmp.String())
addTarSymlink(t, tw, "sym-maze", "fake")
addTarSymlink(t, tw, "sym-maze/oops", "/tmp/something")
require.NoError(t, tw.Close())

extractor := extract.Extractor{FS: &LoggingFS{}}
require.Error(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
require.NoFileExists(t, tmp.Join("oops").String())
})

t.Run("TarWithSymlinkToExternalPathWithoutMazing", func(t *testing.T) {
// Create target dir
tmp := mkTempDir(t)
targetDir := tmp.Join("test")
require.NoError(t, targetDir.Mkdir())

// Make a tar archive with valid symlink maze
outputTar := bytes.NewBuffer(nil)
tw := tar.NewWriter(outputTar)
require.NoError(t, tw.WriteHeader(&tar.Header{Mode: 0o0777, Typeflag: tar.TypeDir, Name: "tmp"}))
addTarSymlink(t, tw, "aaa", "../tmp")
require.NoError(t, tw.Close())

extractor := extract.Extractor{FS: &LoggingFS{}}
require.NoError(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
st, err := targetDir.Join("aaa").Lstat()
require.NoError(t, err)
require.Equal(t, "aaa", st.Name())
})
}

// MockDisk is a disk that chroots to a directory
Expand Down
Loading

0 comments on commit 4a98568

Please sign in to comment.