Fix path traversal vulnerability in artifact tool zip extraction#46713
Open
ayushhgarg-work wants to merge 1 commit intoAzure:mainfrom
Open
Fix path traversal vulnerability in artifact tool zip extraction#46713ayushhgarg-work wants to merge 1 commit intoAzure:mainfrom
ayushhgarg-work wants to merge 1 commit intoAzure:mainfrom
Conversation
ayushhgarg-work
requested review from
JustinFirsching,
NonStatic2014,
achauhan-scc,
arunsu,
jayesh-tanna,
kingernupur,
nick863,
novaturient95,
raduk,
rtanase,
sharma-riti and
vivram
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR mitigates a ZipSlip/path traversal risk when downloading and extracting the Azure DevOps ArtifactTool zip in ArtifactCache._redirect_artifacts_tool_path() by validating each archive member resolves within the intended temporary extraction directory before calling extractall().
Changes:
- Converts the temporary extraction directory to a
Pathand pre-resolves the destination directory. - Adds per-zip-member path traversal validation prior to extraction, rejecting malicious entries (e.g.,
../, absolute paths). - Sets the override environment variable and cached tool path using the resolved destination path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Adds path traversal (ZipSlip) validation to
_artifact_utils.py::_redirect_artifacts_tool_path()before extracting the artifact tool zip archive. Each zip member's resolved path is checked to ensure it stays within the intended extraction directory, preventing malicious archives from writing files outside the temp directory.This aligns with the existing safe extraction pattern in
_local_job_invoker.py::unzip_to_temporary_file().Motivation
Addresses CWE-22 (Improper Limitation of a Pathname to a Restricted Directory). Without this check, a crafted zip archive with
../entries could write files outside the extraction boundary whenextractall()is called.Verified that:
../../path traversal entries are rejectedsubdir/../../escaped.txt) are rejected