-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Solve symlinks destination #129281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
alinpahontu2912
wants to merge
2
commits into
dotnet:main
Choose a base branch
from
alinpahontu2912:symlink_resolution
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+202
−3
Open
Solve symlinks destination #129281
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -368,7 +368,7 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b | |||||||||
| string? fileDestinationPath = GetFullDestinationPath( | ||||||||||
| destinationDirectoryPath, | ||||||||||
| Path.IsPathFullyQualified(name) ? name : Path.Join(destinationDirectoryPath, name)); | ||||||||||
| if (fileDestinationPath == null) | ||||||||||
| if (fileDestinationPath is null || FilePathEscapesDirectory(destinationDirectoryPath, fileDestinationPath)) | ||||||||||
| { | ||||||||||
| throw new IOException(SR.Format(SR.TarExtractingResultsFileOutside, name, destinationDirectoryPath)); | ||||||||||
| } | ||||||||||
|
|
@@ -389,7 +389,7 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b | |||||||||
| string? linkDestination = GetFullDestinationPath( | ||||||||||
| destinationDirectoryPath, | ||||||||||
| Path.IsPathFullyQualified(linkName) ? linkName : Path.Join(Path.GetDirectoryName(fileDestinationPath), linkName)); | ||||||||||
| if (linkDestination is null) | ||||||||||
| if (linkDestination is null || FilePathEscapesDirectory(destinationDirectoryPath, linkDestination)) | ||||||||||
| { | ||||||||||
| throw new IOException(SR.Format(SR.TarExtractingResultsLinkOutside, linkName, destinationDirectoryPath)); | ||||||||||
| } | ||||||||||
|
|
@@ -404,7 +404,7 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b | |||||||||
| string? linkDestination = GetFullDestinationPath( | ||||||||||
| destinationDirectoryPath, | ||||||||||
| Path.Join(destinationDirectoryPath, linkName)); | ||||||||||
| if (linkDestination is null) | ||||||||||
| if (linkDestination is null || FilePathEscapesDirectory(destinationDirectoryPath, linkDestination)) | ||||||||||
| { | ||||||||||
| throw new IOException(SR.Format(SR.TarExtractingResultsLinkOutside, linkName, destinationDirectoryPath)); | ||||||||||
| } | ||||||||||
|
|
@@ -415,6 +415,115 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b | |||||||||
| return (fileDestinationPath, linkTargetPath); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Check if the file destination path or the link target path escapes the destination directory, by walking through the relative path components and resolving symlinks at each step. | ||||||||||
| private static bool FilePathEscapesDirectory(string destinationDirectoryPath, string fileDestinationPath) | ||||||||||
| { | ||||||||||
| // Windows is case insensitive while Linux is case sensitive | ||||||||||
| // This ensures the comparison is consistent with how the OS would resolve the paths | ||||||||||
| StringComparison pathComparison = OperatingSystem.IsWindows() | ||||||||||
| ? StringComparison.OrdinalIgnoreCase | ||||||||||
| : StringComparison.Ordinal; | ||||||||||
|
|
||||||||||
| string resolvedDest = ResolvePhysicalPath(destinationDirectoryPath); | ||||||||||
|
|
||||||||||
| // Use the logical destination path for computing the relative path | ||||||||||
| string logicalDest = Path.GetFullPath(destinationDirectoryPath); | ||||||||||
| string logicalPrefix = logicalDest.EndsWith(Path.DirectorySeparatorChar) | ||||||||||
| ? logicalDest | ||||||||||
| : logicalDest + Path.DirectorySeparatorChar; | ||||||||||
|
|
||||||||||
| string destPrefix = resolvedDest.EndsWith(Path.DirectorySeparatorChar) | ||||||||||
| ? resolvedDest | ||||||||||
| : resolvedDest + Path.DirectorySeparatorChar; | ||||||||||
|
|
||||||||||
| // Normalize file path (resolves .. and . but not symlinks) | ||||||||||
| string normalizedFile = Path.GetFullPath(fileDestinationPath); | ||||||||||
|
|
||||||||||
| // Guard with StartsWith before computing relative path | ||||||||||
| if (!normalizedFile.StartsWith(logicalPrefix, pathComparison) && | ||||||||||
| !normalizedFile.Equals(logicalDest, pathComparison)) | ||||||||||
| { | ||||||||||
| return true; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Walk relative components, resolving symlinks at each step | ||||||||||
| string relative = normalizedFile.Substring(logicalPrefix.Length) | ||||||||||
| .TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); | ||||||||||
|
|
||||||||||
|
Comment on lines
+449
to
+452
|
||||||||||
| string[] components = relative.Split(new char[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }, | ||||||||||
| StringSplitOptions.RemoveEmptyEntries); | ||||||||||
|
|
||||||||||
| string current = resolvedDest; | ||||||||||
|
|
||||||||||
| foreach (string component in components) | ||||||||||
| { | ||||||||||
| current = Path.Combine(current, component); | ||||||||||
|
|
||||||||||
| if (Path.Exists(current)) | ||||||||||
| { | ||||||||||
| string? resolved = ResolveSymlink(current); | ||||||||||
| if (resolved is null) | ||||||||||
| { | ||||||||||
| return true; | ||||||||||
| } | ||||||||||
| current = resolved; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| string normalizedCurrent = Path.GetFullPath(current); | ||||||||||
| if (!normalizedCurrent.StartsWith(destPrefix, pathComparison) && | ||||||||||
| !normalizedCurrent.Equals(resolvedDest, pathComparison)) | ||||||||||
|
Comment on lines
+473
to
+474
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| { | ||||||||||
| return true; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private static string? ResolveSymlink(string path) | ||||||||||
| { | ||||||||||
| FileSystemInfo? target = new FileInfo(path).ResolveLinkTarget(returnFinalTarget: true); | ||||||||||
|
|
||||||||||
| if (target is null) | ||||||||||
| { | ||||||||||
| return Path.GetFullPath(path); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return target.FullName; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Resolves the full path of the specified path, resolving symlinks at each step. | ||||||||||
| // This is needed to mitigate malicious entries in the archive that could lead to writing files outside of the intended directory. | ||||||||||
| private static string ResolvePhysicalPath(string path) | ||||||||||
| { | ||||||||||
| string fullPath = Path.GetFullPath(path); | ||||||||||
| string? root = Path.GetPathRoot(fullPath); | ||||||||||
|
|
||||||||||
| if (root is null) | ||||||||||
| { | ||||||||||
| return fullPath; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| string[] components = fullPath.Substring(root.Length) | ||||||||||
| .Split(new char[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }, StringSplitOptions.RemoveEmptyEntries); | ||||||||||
| string current = root; | ||||||||||
| foreach (string component in components) | ||||||||||
| { | ||||||||||
| current = Path.Combine(current, component); | ||||||||||
| if (Path.Exists(current)) | ||||||||||
| { | ||||||||||
| string? resolved = ResolveSymlink(current); | ||||||||||
| if (resolved is null) | ||||||||||
| { | ||||||||||
| return current; | ||||||||||
| } | ||||||||||
| current = resolved; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return current; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Returns the full destination path if the path is the destinationDirectory or a subpath. Otherwise, returns null. | ||||||||||
| private static string? GetFullDestinationPath(string destinationDirectoryFullPath, string qualifiedPath) | ||||||||||
| { | ||||||||||
|
|
||||||||||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fsutil.exe file setCaseSensitiveInfo <path> enableto violate the general assumptionscasefoldenabled. Also, we can mount NTFS, FAT or other case-insensitive filesystem on unix and run app from there.It's best to avoid assuming case sensitivity and let the underlying filesystem do its thing.