Skip to content

Fix path traversal in extract_all#1

Merged
krippee merged 1 commit intopocfrom
kr/path-traversal-protections
Dec 19, 2025
Merged

Fix path traversal in extract_all#1
krippee merged 1 commit intopocfrom
kr/path-traversal-protections

Conversation

@krippee
Copy link
Contributor

@krippee krippee commented Dec 19, 2025

Summary

Prevent path traversal (Zip Slip) in extract_all by validating entry paths stay within the target directory.

Changes

  • Add safe_extract_path helper to validate and resolve paths
  • Reject entries containing ../ sequences that escape the extraction directory

Before

output_path = File.join(output_dir, entry_path)
# Malicious entry "../../../etc/passwd" could write outside output_dir

After

output_path = safe_extract_path(base_dir, entry_path)
# Raises Error if resolved path escapes base_dir

@krippee krippee requested a review from jkorany December 19, 2025 18:26
# @param entry [Entry, Integer] Entry or entry index to extract
# @param output_path [String] Path where the file should be extracted
# @raise [Error] if extraction fails
def extract(entry, output_path)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any concerns about the single entry path?

Copy link
Contributor Author

@krippee krippee Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question, normally yes. In this context though, this change is not even 100% necessary. The way we use 7-Zip in the application would prevent this type of attack, as we extract to a memory buffer, and the entry path is actually used as a label. That being said, this change is primarily for the gem itself, if it were ever to be used outside our application.

When using it outside of our application, a user may want to define their own file path which could cause issues. We could also add in an optional helper argument for sanitizing the path themselves during single extract if we wanted.

Copy link
Collaborator

@jkorany jkorany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@krippee krippee merged commit 2524bfc into poc Dec 19, 2025
9 checks passed
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.

2 participants

Comments