Skip to content

Commit

Permalink
fix: mholt#65#issuecomment-395988244 - zip slip by symlink
Browse files Browse the repository at this point in the history
  • Loading branch information
aviadatsnyk committed Jun 11, 2018
1 parent e4ef56d commit 09aa582
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 8 deletions.
10 changes: 8 additions & 2 deletions archiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"path/filepath"
"runtime"
"strings"

"github.com/cyphar/filepath-securejoin"
)

// Archiver represent a archive format
Expand Down Expand Up @@ -111,8 +113,12 @@ func sanitizeExtractPath(filePath string, destination string) error {
// to avoid zip slip (writing outside of the destination), we resolve
// 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) {
destpath, err := securejoin.SecureJoin(destination, filePath)
if err != nil {
return fmt.Errorf(
"%s: error calculating destination path", filePath)
}
if !strings.HasPrefix(destpath, destination+string(os.PathSeparator)) {
return fmt.Errorf("%s: illegal file path", filePath)
}
return nil
Expand Down
7 changes: 5 additions & 2 deletions rar.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"path/filepath"
"strings"

"github.com/cyphar/filepath-securejoin"
"github.com/nwaples/rardecode"
)

Expand Down Expand Up @@ -77,8 +78,10 @@ func (rarFormat) Read(input io.Reader, destination string) error {
return err
}

destpath := filepath.Join(destination, header.Name)

destpath, err := securejoin.SecureJoin(destination, header.Name)
if err != nil {
return err
}
if header.IsDir {
err = mkdir(destpath)
if err != nil {
Expand Down
14 changes: 12 additions & 2 deletions tar.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"path/filepath"
"strconv"
"strings"

"github.com/cyphar/filepath-securejoin"
)

// Tar is for Tar format
Expand Down Expand Up @@ -224,7 +226,10 @@ func untarFile(tr *tar.Reader, header *tar.Header, destination string) error {
return err
}

destpath := filepath.Join(destination, header.Name)
destpath, err := securejoin.SecureJoin(destination, header.Name)
if err != nil {
return err
}

switch header.Typeflag {
case tar.TypeDir:
Expand All @@ -234,7 +239,12 @@ func untarFile(tr *tar.Reader, header *tar.Header, destination string) error {
case tar.TypeSymlink:
return writeNewSymbolicLink(destpath, header.Linkname)
case tar.TypeLink:
return writeNewHardLink(destpath, filepath.Join(destination, header.Linkname))
linkpath, err := securejoin.SecureJoin(destination, header.Linkname)
if err != nil {
return err
}
return writeNewHardLink(
destpath, linkpath)
default:
return fmt.Errorf("%s: unknown type flag: %c", header.Name, header.Typeflag)
}
Expand Down
10 changes: 8 additions & 2 deletions zip.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"path"
"path/filepath"
"strings"

"github.com/cyphar/filepath-securejoin"
)

// Zip is for Zip format
Expand Down Expand Up @@ -192,8 +194,12 @@ func unzipFile(zf *zip.File, destination string) error {
return err
}

destpath, err := securejoin.SecureJoin(destination, zf.Name)
if err != nil {
return fmt.Errorf("%s: calculating file path: %v", zf.Name, err)
}
if strings.HasSuffix(zf.Name, "/") {
return mkdir(filepath.Join(destination, zf.Name))
return mkdir(destpath)
}

rc, err := zf.Open()
Expand All @@ -202,7 +208,7 @@ func unzipFile(zf *zip.File, destination string) error {
}
defer rc.Close()

return writeNewFile(filepath.Join(destination, zf.Name), rc, zf.FileInfo().Mode())
return writeNewFile(destpath, rc, zf.FileInfo().Mode())
}

// compressedFormats is a (non-exhaustive) set of lowercased
Expand Down

0 comments on commit 09aa582

Please sign in to comment.