Skip to content

Commit

Permalink
Adds check to make sure path is inside destination to prevent zip sli…
Browse files Browse the repository at this point in the history
…pping
  • Loading branch information
ForestEckhardt authored and ryanmoran committed Apr 10, 2021
1 parent 2751e64 commit 513edf0
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 1 deletion.
20 changes: 20 additions & 0 deletions vacation/vacation.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ func (ta TarArchive) Decompress(destination string) error {
return fmt.Errorf("failed to read tar response: %s", err)
}

err = checkExtractPath(hdr.Name, destination)
if err != nil {
return err
}

fileNames := strings.Split(hdr.Name, string(filepath.Separator))

// Checks to see if file should be written when stripping components
Expand Down Expand Up @@ -284,6 +289,11 @@ func (z ZipArchive) Decompress(destination string) error {
}

for _, f := range zr.File {
err = checkExtractPath(f.Name, destination)
if err != nil {
return err
}

path := filepath.Join(destination, f.Name)

switch {
Expand Down Expand Up @@ -334,3 +344,13 @@ func (z ZipArchive) Decompress(destination string) error {

return nil
}

// This function checks to see that the given path is within the destination
// directory
func checkExtractPath(filePath string, destination string) error {
destpath := filepath.Join(destination, filePath)
if !strings.HasPrefix(destpath, filepath.Clean(destination)+string(os.PathSeparator)) {
return fmt.Errorf("illegal file path %q: the file path does not occur within the destination directory", filePath)
}
return nil
}
25 changes: 24 additions & 1 deletion vacation/vacation_tar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,29 @@ func testVacationTar(t *testing.T, context spec.G, it spec.S) {
})

context("failure cases", func() {
context("when a file is not inside of the destination director (Zip Slip)", func() {
it.Before(func() {
var err error

buffer := bytes.NewBuffer(nil)
tw := tar.NewWriter(buffer)

nestedFile := filepath.Join("..", "some-dir", "some-other-dir", "some-file")
Expect(tw.WriteHeader(&tar.Header{Name: nestedFile, Mode: 0755, Size: int64(len(nestedFile))})).To(Succeed())
_, err = tw.Write([]byte(nestedFile))
Expect(err).NotTo(HaveOccurred())

Expect(tw.Close()).To(Succeed())

tarArchive = vacation.NewTarArchive(bytes.NewReader(buffer.Bytes()))
})

it("returns an error", func() {
err := tarArchive.Decompress(tempDir)
Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("illegal file path %q: the file path does not occur within the destination directory", filepath.Join("..", "some-dir", "some-other-dir", "some-file")))))
})
})

context("when it fails to read the tar response", func() {
it("returns an error", func() {
readyArchive := vacation.NewTarArchive(bytes.NewBuffer([]byte(`something`)))
Expand All @@ -166,7 +189,7 @@ func testVacationTar(t *testing.T, context spec.G, it spec.S) {
Expect(err).To(MatchError(ContainSubstring("failed to create archived directory")))
})

context("there are no directory headers", func() {
context("when there are no directory headers", func() {
it.Before(func() {
var err error

Expand Down
22 changes: 22 additions & 0 deletions vacation/vacation_zip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,28 @@ func testVacationZip(t *testing.T, context spec.G, it spec.S) {
})
})

context("when a file is not inside of the destination director (Zip Slip)", func() {
var buffer *bytes.Buffer
it.Before(func() {
var err error
buffer = bytes.NewBuffer(nil)
zw := zip.NewWriter(buffer)

_, err = zw.Create(filepath.Join("..", "some-dir"))
Expect(err).NotTo(HaveOccurred())

Expect(zw.Close()).To(Succeed())
})

it("returns an error", func() {
readyArchive := vacation.NewZipArchive(buffer)

err := readyArchive.Decompress(tempDir)
Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("illegal file path %q: the file path does not occur within the destination directory", filepath.Join("..", "some-dir")))))
})

})

context("when it fails to unzip a directory", func() {
var buffer *bytes.Buffer
it.Before(func() {
Expand Down

0 comments on commit 513edf0

Please sign in to comment.