Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential rule: path traversal when extracting zip archives #205

Closed
gcmurphy opened this issue Apr 26, 2018 · 1 comment
Closed

Potential rule: path traversal when extracting zip archives #205

gcmurphy opened this issue Apr 26, 2018 · 1 comment
Labels

Comments

@gcmurphy
Copy link
Member

It would be fairly typical to include the file path from the zip file when extracting it's contents to disk
like in this:

import (
	"archive/zip"
	"io"
	"os"
	"path/filepath"
	"strings"
)

func unzip(archive, target string) error {
	reader, err := zip.OpenReader(archive)
	if err != nil {
		return err
	}

	if err := os.MkdirAll(target, 0755); err != nil {
		return err
	}

	for _, file := range reader.File {
		path := filepath.Join(target, file.Name)
		if file.FileInfo().IsDir() {
			os.MkdirAll(path, file.Mode())
			continue
		}

		fileReader, err := file.Open()
		if err != nil {
			return err
		}
		defer fileReader.Close()

		targetFile, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, file.Mode())
		if err != nil {
			return err
		}
		defer targetFile.Close()

		if _, err := io.Copy(targetFile, fileReader); err != nil {
			return err
		}
	}

	return nil
}

(original source)

It could be possible to create a zip archive with: ../../../../../some_file to achieve path traversal here.
We should investigate whether this could be detected. I think if we detect filepath.Join inclusive of zip.File type, without sanitisation, and the use of that identifier in a write operation you could get a fairly high confidence here.

@hansmi
Copy link

hansmi commented May 1, 2018

If it can be reasonably detected it's a good idea. The same verification can be applied to archive/tar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants