Skip to content

Commit

Permalink
Merge pull request #30 from moul/dev/moul/fix-zip
Browse files Browse the repository at this point in the history 
fix: symlinks were not working as expected
  • Loading branch information
@moul
moul committed Nov 30, 2020
2 parents a1b8595 + a7bb0d3 commit 909480a
Show file tree
Hide file tree
Showing 3 changed files with 264 additions and 16 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/go.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,6 @@ jobs:
strategy:
matrix:
golang:
- 1.11.x
- 1.12.x
- 1.13.x
- 1.14.x
- 1.15.x
Expand Down
58 changes: 47 additions & 11 deletions archive.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,56 @@ func Unzip(src string, dest string) ([]string, error) {
return unzip(&r.Reader, dest)
}

func findFirstExistingDir(path string) string {
for path != "" && path != "/" {
_, err := os.Stat(path)
if err == nil {
return path
}
path = filepath.Dir(path)
}
return ""
}

func unzip(r *zip.Reader, dest string) ([]string, error) {
var (
filenames = make([]string, 0)
errs error
)

// eval symlink on dest dir to compare for zip slip
var destLink string
{
if firstExisting := findFirstExistingDir(dest); firstExisting != "" && firstExisting != "/" {
link, err := filepath.EvalSymlinks(firstExisting)
if err != nil {
return nil, err
}
destLink = link
}
}

for _, f := range r.File {
fpath := filepath.Join(dest, f.Name)

// check for ZipSlip. more Info: https://snyk.io/research/zip-slip-vulnerability#go
if !strings.HasPrefix(fpath, filepath.Clean(dest)+string(os.PathSeparator)) {
errs = multierr.Append(errs, fmt.Errorf("%s: illegal file path", fpath))
continue
{
if !strings.HasPrefix(fpath, filepath.Clean(dest)+string(os.PathSeparator)) {
errs = multierr.Append(errs, fmt.Errorf("%s: illegal file path", fpath))
continue
}

if firstExisting := findFirstExistingDir(fpath); firstExisting != "" && firstExisting != "/" {
link, err := filepath.EvalSymlinks(firstExisting)
if err != nil {
errs = multierr.Append(errs, err)
continue
}
if !strings.HasPrefix(link, filepath.Clean(destLink)) {
errs = multierr.Append(errs, fmt.Errorf("%s: illegal file path", fpath))
continue
}
}
}

if f.FileInfo().IsDir() {
Expand All @@ -57,22 +95,14 @@ func unzip(r *zip.Reader, dest string) ([]string, error) {
}
} else {
// file

if err := os.MkdirAll(filepath.Dir(fpath), os.ModePerm); err != nil {
errs = multierr.Append(errs, err)
continue
}

outFile, err := os.OpenFile(fpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
if err != nil {
errs = multierr.Append(errs, err)
continue
}

rc, err := f.Open()
if err != nil {
errs = multierr.Append(errs, err)
outFile.Close()
continue
}

Expand All @@ -85,6 +115,12 @@ func unzip(r *zip.Reader, dest string) ([]string, error) {
continue
}
} else {
outFile, err := os.OpenFile(fpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
if err != nil {
errs = multierr.Append(errs, err)
continue
}

_, err = io.Copy(outFile, rc)

outFile.Close()
Expand Down
220 changes: 217 additions & 3 deletions archive_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package u_test

import (
"archive/zip"
"bytes"
"fmt"
"io/ioutil"
"os"
Expand All @@ -11,7 +13,7 @@ import (

func ExampleUnzip() {
// create zipfile on fs
f, cleanup, err := u.TempfileWithContent(zipdata)
f, cleanup, err := u.TempfileWithContent(zipdata_simple)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -47,7 +49,7 @@ func ExampleUnzipBytes() {
defer os.RemoveAll(tempdir)

// unzip to dest
files, err := u.UnzipBytes(zipdata, tempdir)
files, err := u.UnzipBytes(zipdata_simple, tempdir)
if err != nil {
panic(err)
}
Expand All @@ -60,7 +62,179 @@ func ExampleUnzipBytes() {
// ./gophercolor16x16.png
}

var zipdata = []byte{
func ExampleUnzip_zipslip() {
// create zipfile on fs
f, cleanup, err := u.TempfileWithContent(zipdata_zipline)
if err != nil {
panic(err)
}
defer cleanup()

// create tempdir for dest
tempdir, err := ioutil.TempDir("", "u")
if err != nil {
panic(err)
}
defer os.RemoveAll(tempdir)

// unzip to dest
_, err = u.Unzip(f.Name(), tempdir)
fmt.Println(err)

// Output:
// /tmp/evil.txt: illegal file path
}

func ExampleUnzipBytes_createAndUnzip() {
// create a custom zip
buf := new(bytes.Buffer)
{
w := zip.NewWriter(buf)
// a.txt
{
hdr := zip.FileHeader{
Name: "a.txt",
Method: zip.Deflate,
}
hdr.SetMode(0o755)
f, err := w.CreateHeader(&hdr)
if err != nil {
panic(err)
}
_, err = f.Write([]byte("hello world!"))
if err != nil {
panic(err)
}
}

// b.txt -> a.txt
{
hdr := zip.FileHeader{
Name: "b.txt",
Comment: "c",
Method: zip.Deflate,
}
hdr.SetMode(0o755 | os.ModeSymlink)
f, err := w.CreateHeader(&hdr)
if err != nil {
panic(err)
}
_, err = f.Write([]byte("a.txt"))
if err != nil {
panic(err)
}
}

err := w.Close()
if err != nil {
panic(err)
}
}

// unzip it
{
// create tempdir for dest
tempdir, err := ioutil.TempDir("", "u")
if err != nil {
panic(err)
}
defer os.RemoveAll(tempdir)

// unzip to dest
files, err := u.UnzipBytes(buf.Bytes(), tempdir)
if err != nil {
panic(err)
}
for _, file := range files {
relPath := "." + strings.TrimPrefix(file, tempdir)
stat, err := os.Lstat(file)
if err != nil {
panic(err)
}
fmt.Println(relPath, stat.Mode()&os.ModeSymlink != 0)
}
}
// Output:
// ./a.txt false
// ./b.txt true
}

func ExampleUnzipBytes_createAndUnzipZipSlip() {
// create second temp dir
var victim string
{
var err error
victim, err = ioutil.TempDir("", "u")
if err != nil {
panic(err)
}
defer os.RemoveAll(victim)
}

// create a custom zip
buf := new(bytes.Buffer)
{
w := zip.NewWriter(buf)
// a.txt
{
hdr := zip.FileHeader{
Name: "a",
Method: zip.Deflate,
}
hdr.SetMode(0o755 | os.ModeSymlink)
f, err := w.CreateHeader(&hdr)
if err != nil {
panic(err)
}
_, err = f.Write([]byte(victim))
if err != nil {
panic(err)
}
}

// b.txt -> a.txt
{
hdr := zip.FileHeader{
Name: "a/b.txt",
Method: zip.Deflate,
}
hdr.SetMode(0o755)
f, err := w.CreateHeader(&hdr)
if err != nil {
panic(err)
}
_, err = f.Write([]byte("hello world!"))
if err != nil {
panic(err)
}
}

err := w.Close()
if err != nil {
panic(err)
}
}

// unzip it
{
// create tempdir for dest
tempdir, err := ioutil.TempDir("", "u")
if err != nil {
panic(err)
}
defer os.RemoveAll(tempdir)

// unzip to dest
_, err = u.UnzipBytes(buf.Bytes(), tempdir)
errStr := err.Error()
errStr = strings.Replace(errStr, tempdir, "TEMPDIR", -1)
fmt.Println(errStr)
}
// Output:
// TEMPDIR/a/b.txt: illegal file path
}

var zipdata_simple = []byte{
0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x00, 0x00, 0x08, 0x00, 0x81, 0x61, 0x25, 0x3d, 0xc0, 0xd7,
0xed, 0xc3, 0x19, 0x00, 0x00, 0x00, 0x1a, 0x00, 0x00, 0x00, 0x08, 0x00, 0x1c, 0x00, 0x74, 0x65,
0x73, 0x74, 0x2e, 0x74, 0x78, 0x74, 0x55, 0x54, 0x09, 0x00, 0x03, 0x71, 0xfc, 0x82, 0x4c, 0x76,
Expand Down Expand Up @@ -136,3 +310,43 @@ var zipdata = []byte{
0x61, 0x20, 0x7a, 0x69, 0x70, 0x66, 0x69, 0x6c, 0x65, 0x20, 0x63, 0x6f, 0x6d, 0x6d, 0x65, 0x6e,
0x74, 0x2e,
}

var zipdata_zipline = []byte{
0x50, 0x4b, 0x03, 0x04, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x8f, 0xb0, 0x8f, 0x4c, 0x0f,
0x6f, 0x4f, 0xf3, 0x13, 0x00, 0x00, 0x00, 0x13, 0x00, 0x00, 0x00, 0x08, 0x00, 0x1c, 0x00,
0x67, 0x6f, 0x6f, 0x64, 0x2e, 0x74, 0x78, 0x74, 0x55, 0x54, 0x09, 0x00, 0x03, 0x3d, 0xa2,
0xd3, 0x5a, 0x3e, 0xa2, 0xd3, 0x5a, 0x75, 0x78, 0x0b, 0x00, 0x01, 0x04, 0xf6, 0x01, 0x00,
0x00, 0x04, 0x14, 0x00, 0x00, 0x00, 0x74, 0x68, 0x69, 0x73, 0x20, 0x69, 0x73, 0x20, 0x61,
0x20, 0x67, 0x6f, 0x6f, 0x64, 0x20, 0x6f, 0x6e, 0x65, 0x0a, 0x50, 0x4b, 0x03, 0x04, 0x14,
0x00, 0x00, 0x00, 0x00, 0x00, 0x95, 0xb0, 0x8f, 0x4c, 0x60, 0x41, 0x7b, 0x39, 0x14, 0x00,
0x00, 0x00, 0x14, 0x00, 0x00, 0x00, 0x84, 0x00, 0x00, 0x00, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x74, 0x6d, 0x70, 0x2f, 0x65,
0x76, 0x69, 0x6c, 0x2e, 0x74, 0x78, 0x74, 0x74, 0x68, 0x69, 0x73, 0x20, 0x69, 0x73, 0x20,
0x61, 0x6e, 0x20, 0x65, 0x76, 0x69, 0x6c, 0x20, 0x6f, 0x6e, 0x65, 0x0a, 0x50, 0x4b, 0x01,
0x02, 0x1e, 0x03, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x8f, 0xb0, 0x8f, 0x4c, 0x0f, 0x6f,
0x4f, 0xf3, 0x13, 0x00, 0x00, 0x00, 0x13, 0x00, 0x00, 0x00, 0x08, 0x00, 0x18, 0x00, 0x00,
0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xa4, 0x81, 0x00, 0x00, 0x00, 0x00, 0x67, 0x6f,
0x6f, 0x64, 0x2e, 0x74, 0x78, 0x74, 0x55, 0x54, 0x05, 0x00, 0x03, 0x3d, 0xa2, 0xd3, 0x5a,
0x75, 0x78, 0x0b, 0x00, 0x01, 0x04, 0xf6, 0x01, 0x00, 0x00, 0x04, 0x14, 0x00, 0x00, 0x00,
0x50, 0x4b, 0x01, 0x02, 0x14, 0x03, 0x14, 0x00, 0x00, 0x00, 0x00, 0x00, 0x95, 0xb0, 0x8f,
0x4c, 0x60, 0x41, 0x7b, 0x39, 0x14, 0x00, 0x00, 0x00, 0x14, 0x00, 0x00, 0x00, 0x84, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xa4, 0x81, 0x55, 0x00, 0x00,
0x00, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e, 0x2f, 0x2e, 0x2e,
0x2f, 0x74, 0x6d, 0x70, 0x2f, 0x65, 0x76, 0x69, 0x6c, 0x2e, 0x74, 0x78, 0x74, 0x50, 0x4b,
0x05, 0x06, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x02, 0x00, 0x00, 0x01, 0x00, 0x00, 0x0b,
0x01, 0x00, 0x00, 0x00, 0x00,
}

0 comments on commit 909480a

Please sign in to comment.