fix(security): prevent Zip Slip path traversal in harUnzip#40623
Open
SebTardif wants to merge 1 commit intomicrosoft:mainfrom
Open
fix(security): prevent Zip Slip path traversal in harUnzip#40623SebTardif wants to merge 1 commit intomicrosoft:mainfrom
SebTardif wants to merge 1 commit intomicrosoft:mainfrom
Conversation
pavelfeldman
approved these changes
May 5, 2026
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
fbad648 to
74cee1d
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Add resolveWithinRoot() validation to harUnzip() to ensure zip entry paths cannot escape the target resourcesDir via path traversal sequences (e.g. ../../etc/evil). This follows the same pattern already used in extractTrace() in traceParser.ts. Fixes microsoft#40622
74cee1d to
7b86f65
Compare
Contributor
Test results for "MCP"2 failed 6941 passed, 1052 skipped Merge workflow run. |
Contributor
Test results for "tests 1"41655 passed, 851 skipped Merge workflow run. |
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.
Summary
Fixes #40622
The
harUnzip()function inpackages/playwright-core/src/server/localUtils.tsextracts zip entries usingpath.join(resourcesDir, entry)without validating that the resolved path stays insideresourcesDir. A crafted.har.zipfile with entries containing path traversal sequences (e.g.../../etc/cron.d/evil) could write files to arbitrary locations on the filesystem.Fix
Added
resolveWithinRoot()validation before writing each extracted zip entry, rejecting any entry whose resolved path escapes the targetresourcesDir. This follows the exact same pattern already used inextractTrace()inpackages/playwright-core/src/tools/trace/traceParser.ts:Changes
packages/playwright-core/src/server/localUtils.ts: ImportresolveWithinRootfrom@utils/fileUtilsand add path containment check inharUnzip().Test plan
The
resolveWithinRoot()utility is already thoroughly tested via the existingextractTrace()codepath. The fix is a 3-line addition that rejects any zip entry with a relative path escaping the output directory (returnsnullfor absolute paths and paths that resolve outside the root after normalization).