-
Notifications
You must be signed in to change notification settings - Fork 5k
fix: sanitize filesystem paths derived from ZIP entries #48205
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
base: main
Are you sure you want to change the base?
Conversation
🤖 GitHub commentsJust comment with:
|
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
/backport-active-all |
|
Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services) |
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
Co-authored-by: Chris Berkhout <chrisberkhout@gmail.com>
Checklist
stresstest.shscript to run them under stress conditions and race detector to verify their stability../changelog/fragmentsusing the changelog tool.Summary
fixes an archive extraction vulnerability in
unzipwhere filesystem paths derived from ZIP archive entries were not consistently validated. Under specific conditions, a crafted archive could exploit directory traversal sequences (e.g...) to write files outside the intended extraction directory.Background / Problem Statement
When extracting ZIP archives, each archive entry contains a file path that is fully controlled by the archive author. These paths are not inherently restricted and may include traversal components such as
../or platform-specific equivalents. If such paths are used directly to construct filesystem paths, extraction may occur outside the expected destination directory. if a ZIP entry contains the path..\elastic-pwnedand the archive is extracted intoC:\output, naïvely joining these paths This behavior can lead to unauthorized file creation or modification outside the extraction root, potentially resulting in sensitive data exposure, file deletion, or unintended behavior changes.The file already contains a helper function,
sanitizeFilePath, which safely constructs and validates extraction paths by:workdir) with the archive entry pathHowever, this protection is only applied when
folder == "".When
folder != "", the destination path is currently computed as:In this case, no validation or sanitization is performed after path construction, allowing crafted ZIP entries to bypass the existing safety checks.
Fix Description
This change ensures that all filesystem paths derived from ZIP archive entries are consistently sanitized, regardless of whether a folder prefix is used.
The fix follows a minimal and safe approach:
sansFolder), join them into a relative path string.sanitizeFilePath, usingworkdiras the extraction root.sanitizeFilePath.Implementation Details
Specifically, in
unzipFile:filepath.Join(workdir, filepath.Join(sansFolder...))usage.filepath.Join(sansFolder...).sanitizeFilePath(workdir, relativePath)and assign the returned value todestPath.This ensures that any traversal attempts are reliably detected and blocked before file extraction occurs. By enforcing consistent path validation for all ZIP entries, this change prevents directory traversal during archive extraction and ensures that files cannot be written outside the intended extraction root. This aligns the behavior of all code paths with the existing security expectations already implemented in
sanitizeFilePath.