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

Standardize the filename of Archive Entries so path traversal characters cannot be used to manipulate file output location #171

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EthanArbuckle
Copy link

This PR aims to eliminate the Zipperdown security vulnerability. Relevant Github issue

Files inside of an archive may contain path traversal characters in their filename. When unarchiving these, SSZipArchive follows ../ sequences which can lead to a file being written to an unexpected destination. This could potentially lead to RCE under the worst of circumstances (such as overwriting a javascript file that the app is going to execute).

This addition sanitizes dot-dot-slash (../) characters from the destination path of files being unarchived, mimicking the default behavior of the Archive Utility found on macOS (as well as most other popular desktop archiving clients).

Consider an archive containing the following file:
../../../../../../../../../../../Users/test/Desktop/hello.txt

When unarchived using the current release of ZipZap, the directory traversal characters are followed causing unarchived files to break out of the current directory, resulting in /Users/test/Desktop/hello.txt being created.

Compare this to the default behavior of macOS's Archive Utility, which does not follow directory traversal characters. The same archive results in extraction restricted to the current directory:
%current_path%/Users/test/Desktop/hello.txt

From a security standpoint, discarding directory traversal characters when unarchiving should be the default behavior of this SDK.

…ers cannot be used to manipulate file output location
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant