.Net: Reject mixed-separator UNC paths in file plugins#14166
Open
jstar0 wants to merge 1 commit into
Open
Conversation
Signed-off-by: King Star <mcxin.y@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens the .NET file-related plugins to consistently reject UNC / extended-path prefixes (including mixed /\ and \/ forms) before path canonicalization can trigger filesystem probes, and adds regression coverage for the newly blocked path-prefix variants.
Changes:
- Update
FileIOPluginandWebFileDownloadPluginto treat any two leading directory-separator characters (/or\, in any combination) as a UNC/extended-path prefix and reject early. - Add unit tests covering all four leading-separator combinations for read/write/download scenarios.
- Align the Web plugin’s per-path allow-list check with the shared UNC/extended prefix detection helper.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| dotnet/src/Plugins/Plugins.Web/WebFileDownloadPlugin.cs | Updates UNC/extended-path detection to reject mixed-separator two-character prefixes before path checks proceed. |
| dotnet/src/Plugins/Plugins.Core/FileIOPlugin.cs | Applies the same UNC/extended-path prefix rejection to FileIOPlugin path validation. |
| dotnet/src/Plugins/Plugins.UnitTests/Web/WebFileDownloadPluginTests.cs | Adds download regressions for the newly rejected mixed-separator UNC/extended prefixes. |
| dotnet/src/Plugins/Plugins.UnitTests/Core/FileIOPluginTests.cs | Adds read/write regressions for the newly rejected mixed-separator UNC/extended prefixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+116
to
119
| if (IsUncOrExtendedPath(path)) | ||
| { | ||
| throw new ArgumentException("Invalid file path, UNC paths are not supported.", nameof(path)); | ||
| } |
Comment on lines
218
to
224
| var expanded = Environment.ExpandEnvironmentVariables(path); | ||
| if (IsUncOrExtendedPath(expanded)) | ||
| { | ||
| throw new ArgumentException("Invalid file path, UNC paths are not supported.", nameof(path)); | ||
| } | ||
|
|
||
| return PathUtilities.GetSafeFullPath(expanded); |
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.
Motivation and Context
FileIOPluginandWebFileDownloadPluginrejected only some UNC path prefixes before path canonicalization. On Windows, mixed/\and\/prefixes resolve to UNC paths too, so those forms could reach filesystem path resolution before the allow-list rejected them.Fixes #14157.
Description
/or\characters as a UNC or extended-path prefix in both plugins.File.Exists, orFile.GetAttributescan probe the filesystem.The change is limited to the two plugins named in the issue. Single-separator rooted paths, normal local paths, allow-list behavior, symlink handling, overwrite checks, and download limits are unchanged.
Verification
The focused regressions pass 12/12, all
FileIOPluginTestspass 21/21, and the deterministic Plugins unit-test suite passes 409/409. The two excluded Web tests use a live HTTPS request and fail on unmodifiedmainin this macOS environment because certificate revocation status is unavailable.Contribution Checklist