fix(packager): reject zip entries that escape the extraction directory - #796
Merged
Merged
Conversation
ZipPluginDecoder.ExtractTo joined each entry's dir/filename onto the destination with path.Join, which cleans but does not contain: an entry named '../../x' resolves outside dst, giving an uploaded .difypkg arbitrary file write (overwriting runtime files or planted code that the daemon then executes) wherever signature verification is off or the package source is otherwise trusted enough to extract. Each entry is now resolved against dst with filepath.Rel and rejected unless the result is strictly contained. Adds regression tests for the traversal case and for normal nested entries. Signed-off-by: Sasha Mitchell <sash.t.mitchell@gmail.com>
dosubot
Bot
added
size:S
Contributor
|
you can use os.OpenRoot |
Adopt the review suggestion: writes now go through a root-scoped handle, so extraction cannot escape dst even if a path slips past the lexical Rel() guard (symlink or platform edge). The explicit check stays for the domain error message; OpenRoot is the enforcement layer.
Contributor
Author
|
Good call, done. Writes now go through |
dosubot
Bot
added
size:M
fatelei
approved these changes
Aug 7, 2026
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
2 participants
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.
Description
ZipPluginDecoder.ExtractTobuilds the output path withpath.Join(dst, dir)/filepath.Join(workingPath, filename)from raw zip entry names.path.Joincleans the result but does not contain it, so an entry named../../evil.pyresolves outside the plugin working directory.Impact: installing an attacker-crafted
.difypkgyields arbitrary file write on the daemon host - overwriting runtime files, or planting code that the daemon subsequently loads/executes - anywhere signature verification is disabled (local/self-hosted installs) or the package otherwise reaches extraction.Type of Change
Essential Checklist
Testing
Each entry is now resolved against the destination with
filepath.Reland rejected unless strictly contained (illegal file path in plugin package). On rejection the partially-written working directory is removed, matching the existing error path.Added
pkg/plugin_packager/decoder/zip_extract_test.go:TestExtractToRejectsPathTraversal: a../../entry errors and nothing is written outsidedstTestExtractToAllowsNormalEntries: nested legitimate entries still extract correctlygo test ./pkg/plugin_packager/decoder/ -run TestExtractTo -count=1 -v- both pass.Additional Information
The read paths (
ReadFile/ReadDir) are unaffected; this hardens only the write sink. Behavior for well-formed packages (including marketplace packages) is unchanged.Made with Cursor