[msbuild] Validate extracted resource paths in UnpackLibraryResources#25911
[msbuild] Validate extracted resource paths in UnpackLibraryResources#25911rolfbjarne wants to merge 7 commits into
Conversation
Ensure that the resolved path for extracted resources stays within the intended output directory. If an embedded resource name contains path traversal sequences (e.g. '..'), the task now logs an error and skips that resource. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the UnpackLibraryResources MSBuild task against path traversal by validating that embedded resources can only be extracted within the intended intermediate output subdirectory.
Changes:
- Added a containment check for computed extraction paths in
UnpackLibraryResourcesand logs an error when the resource would extract out-of-bounds. - Added a new localized error string (
E7183) describing the out-of-bounds extraction attempt.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| msbuild/Xamarin.MacDev.Tasks/Tasks/UnpackLibraryResources.cs | Adds extraction path containment validation and error logging before writing embedded resources to disk. |
| msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx | Introduces E7183 for the new extraction path validation error message. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Switch from manual GetFullPath+StartsWith check to the existing PathUtils.IsPathContained helper which also handles symlinks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Add tests verifying that: - Resources with path traversal sequences are rejected with E7183. - Valid resources are still extracted correctly. Also update E7183 to include the computed path and target directory in the error message for easier diagnosis. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Use pre-compiled test assemblies instead of System.Reflection.Metadata APIs to avoid type conflicts between the framework and package versions of System.Reflection.Metadata referenced by the test project. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ests These were compiled with: csc -target:library -out:Traversal.dll -resource:r.txt,__monotouch_content_.._sEvil.txt empty.cs csc -target:library -out:Valid.dll -resource:r.txt,__monotouch_content_sub_sfile.txt empty.cs Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Replace pre-compiled binary test DLLs with runtime assembly creation using Mono.Cecil. This avoids checking in binary files and makes the test self-contained. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #5da45b0] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [PR Build #5da45b0] Build passed (Build packages) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [PR Build #5da45b0] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #5da45b0] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 199 tests passed 🎉 Tests counts✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Ensure that the resolved path for extracted resources stays within the intended output directory. If an embedded resource name contains path traversal sequences (e.g. '..'), the task now logs an error and skips that resource.