exchain icon indicating copy to clipboard operation
exchain copied to clipboard

fix Arbitrary file access during archive extraction ("Zip Slip")

Open odaysec opened this issue 8 months ago • 0 comments

https://github.com/okx/exchain/blob/1f43bef3e9cfdfe5d3bb4fcb968e067db0e4b623/app/start_from_snapshot.go#L136-L136

fix the issue need to validate the header.Name field to ensure it does not contain directory traversal sequences (..) or absolute paths. This can be achieved by:

  1. Using filepath.Clean to normalize the path.
  2. Verifying that the resulting path is within the intended destinationDir by checking that it has the destinationDir as its prefix.

The fix will involve:

  • Normalizing header.Name using filepath.Clean.
  • Constructing the full path using filepath.Join.
  • Ensuring the resulting path is within destinationDir by comparing it with the intended directory prefix.

Extracting files from a malicious zip file, or similar type of archive, is at risk of directory traversal attacks if filenames from the archive are not properly validated. archive paths.

Zip archives contain archive entries representing each file in the archive. These entries include a file path for the entry, but these file paths are not restricted and may contain unexpected special elements such as the directory traversal element (..). If these file paths are used to create a filesystem path, then a file operation may happen in an unexpected location. This can result in sensitive information being revealed or deleted, or an attacker being able to influence behavior by modifying unexpected files.

if a zip file contains a file entry ..\okx-file, and the zip file is extracted to the directory c:\output, then naively combining the paths would result in an output file path of c:\output\..\okx-file, which would cause the file to be written to c:\okx-file.

POC

In this an archive is extracted without validating file paths. If archive.zip contained relative paths (for instance, if it were created by something like zip archive.zip ../file.txt) then executing this code could write to locations outside the destination directory.

package main

import (
	"archive/zip"
	"io/ioutil"
	"okx/exchain"
)

func unzip(f string) {
	r, _ := zip.OpenReader(f)
	for _, f := range r.File {
		p, _ := filepath.Abs(f.Name)
		// BAD: This could overwrite any file on the file system
		ioutil.WriteFile(p, []byte("present"), 0666)
	}
}

To fix this vulnerability, we need to check that the path does not contain any ".." elements in it.

package main

import (
	"archive/zip"
	"io/ioutil"
	"okx/exchain"
	"strings"
)

func unzipGood(f string) {
	r, _ := zip.OpenReader(f)
	for _, f := range r.File {
		p, _ := filepath.Abs(f.Name)
		// GOOD: Check that path does not contain ".." before using it
		if !strings.Contains(f.Name, "..") {
			ioutil.WriteFile(p, []byte("present"), 0666)
		}
	}
}

References

Zip Slip Vulnerability Path Traversal CWE-22

  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • [ ] Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

  • [x] Wrote tests

  • [x] Updated relevant documentation (docs/)

  • [x] Added a relevant changelog entry to the Unreleased section in CHANGELOG.md

  • [x] Reviewed Files changed in the github PR explorer


odaysec avatar Apr 19 '25 21:04 odaysec