diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs index 64eea60618b83b..d9ab85dd160ec3 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs @@ -336,7 +336,6 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b string? fileDestinationPath = GetFullDestinationPath( destinationDirectoryPath, Path.IsPathFullyQualified(name) ? name : Path.Join(destinationDirectoryPath, name)); - if (fileDestinationPath is null || FilePathEscapesDirectory(destinationDirectoryPath, fileDestinationPath)) { throw new IOException(SR.Format(SR.TarExtractingResultsFileOutside, name, destinationDirectoryPath)); @@ -348,9 +347,9 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b // LinkName is an absolute path, or path relative to the fileDestinationPath directory. // We don't check if the LinkName is empty. In that case, creation of the link will fail because link targets can't be empty. string linkName = ArchivingUtils.SanitizeEntryFilePath(LinkName, preserveDriveRoot: true); - // On Windows, reject rooted but not fully qualified paths ("\Windows\win.ini", "C:foo"). - // They resolve ambiguously based on the current drive or working directory. - // Symlinks are resolved at access time, so Path.GetFullPath cannot reliably determine which drive will be used. + // On Windows, reject rooted-but-not-fully-qualified symlink targets (e.g., "\Windows\win.ini"). + // Unlike files, symlink targets are resolved at access time, not extraction time, + // so Path.GetFullPath here cannot reliably predict what drive the OS will resolve them against. if (OperatingSystem.IsWindows() && Path.IsPathRooted(linkName) && !Path.IsPathFullyQualified(linkName)) { throw new IOException(SR.Format(SR.TarExtractingResultsLinkOutside, linkName, destinationDirectoryPath)); @@ -384,7 +383,9 @@ 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. + // Prevent an archive from escaping the extraction root through symlinks that were created by earlier entries in the same archive. + // This protection applies only to links introduced by the archive itself. It is not intended to defend against preexisting symlinks + // already present on disk before extraction. private static bool FilePathEscapesDirectory(string destinationDirectoryPath, string fileDestinationPath) { // Windows is case insensitive while Linux is case sensitive @@ -394,6 +395,13 @@ private static bool FilePathEscapesDirectory(string destinationDirectoryPath, st : 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; @@ -401,9 +409,20 @@ private static bool FilePathEscapesDirectory(string destinationDirectoryPath, st // Normalize file path (resolves .. and . but not symlinks) string normalizedFile = Path.GetFullPath(fileDestinationPath); - // Walk relative components, resolving symlinks at each step - string relative = normalizedFile.Substring(resolvedDest.Length) - .TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + // 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. + // When the file resolves to the destination directory itself, there are no components to walk, + // so the relative path is empty. Guarding here avoids an out-of-range Substring on the prefix length. + string relative = normalizedFile.Equals(logicalDest, pathComparison) + ? string.Empty + : normalizedFile.Substring(logicalPrefix.Length) + .TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); string[] components = relative.Split(new char[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }, StringSplitOptions.RemoveEmptyEntries); @@ -413,16 +432,7 @@ private static bool FilePathEscapesDirectory(string destinationDirectoryPath, st 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; - } + current = ResolveSymlink(current); string normalizedCurrent = Path.GetFullPath(current); if (!normalizedCurrent.StartsWith(destPrefix, pathComparison) && @@ -435,15 +445,18 @@ private static bool FilePathEscapesDirectory(string destinationDirectoryPath, st return false; } - private static string? ResolveSymlink(string path) + private static string ResolveSymlink(string path) { - FileSystemInfo? target = new FileInfo(path).ResolveLinkTarget(returnFinalTarget: true); + var info = new FileInfo(path); - if (target is null) + // Check LinkTarget first so dangling symlinks/junctions (whose final target doesn't exist yet) + // are still resolved to their raw target, rather than being treated as a non-link. + if (info.LinkTarget is null) { return Path.GetFullPath(path); } + FileSystemInfo target = info.ResolveLinkTarget(returnFinalTarget: true) ?? info; return target.FullName; } @@ -467,12 +480,7 @@ private static string ResolvePhysicalPath(string path) current = Path.Combine(current, component); if (Path.Exists(current)) { - string? resolved = ResolveSymlink(current); - if (resolved is null) - { - return current; - } - current = resolved; + current = ResolveSymlink(current); } } diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs index 4e8a5b8e89a811..d6483b8444b1c6 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs @@ -46,7 +46,6 @@ public void NonExistentDirectory_Throws() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void SetsLastModifiedTimeOnExtractedFiles() { using TempDirectory root = new TempDirectory(); @@ -74,7 +73,6 @@ public void SetsLastModifiedTimeOnExtractedFiles() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void SetsLastModifiedTimeOnExtractedDirectories() { using TempDirectory root = new TempDirectory(); @@ -211,7 +209,6 @@ public void Extract_AllSegmentsOfPath() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void ExtractArchiveWithEntriesThatStartWithSlashDotPrefix() { using TempDirectory root = new TempDirectory(); @@ -237,7 +234,6 @@ public void ExtractArchiveWithEntriesThatStartWithSlashDotPrefix() [Theory] [InlineData(true)] [InlineData(false)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void UnixFileModes(bool overwrite) { using TempDirectory source = new TempDirectory(); @@ -306,7 +302,6 @@ public void UnixFileModes(bool overwrite) [Theory] [InlineData(true)] [InlineData(false)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void UnixFileModes_RestrictiveParentDir(bool overwrite) { using TempDirectory source = new TempDirectory(); @@ -347,7 +342,6 @@ public void UnixFileModes_RestrictiveParentDir(bool overwrite) } [ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void LinkBeforeTarget() { using TempDirectory source = new TempDirectory(); @@ -377,7 +371,6 @@ public void LinkBeforeTarget() } [ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void ExtractToDirectory_RejectsSymlinkDirectoryTraversal_WithNestedFile() { using TempDirectory root = new TempDirectory(); @@ -419,7 +412,6 @@ public void ExtractToDirectory_RejectsSymlinkDirectoryTraversal_WithNestedFile() [ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX | TestPlatforms.Windows)] public void ExtractToDirectory_RejectsChainedSymlinkDirectoryTraversal_WithNestedFile() { // dir a/ diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Stream.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Stream.Tests.cs index 03ec5658b0caa6..8bd60b6f3b4a53 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Stream.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Stream.Tests.cs @@ -76,7 +76,6 @@ public void ExtractEntry_ManySubfolderSegments_NoPrecedingDirectoryEntries() [Theory] [InlineData(TarEntryType.SymbolicLink)] [InlineData(TarEntryType.HardLink)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void Extract_LinkEntry_TargetOutsideDirectory(TarEntryType entryType) { using MemoryStream archive = new MemoryStream(); @@ -99,25 +98,21 @@ public void Extract_LinkEntry_TargetOutsideDirectory(TarEntryType entryType) [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))] [InlineData(TarEntryFormat.Pax)] [InlineData(TarEntryFormat.Gnu)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void Extract_SymbolicLinkEntry_TargetInsideDirectory(TarEntryFormat format) => Extract_LinkEntry_TargetInsideDirectory_Internal(TarEntryType.SymbolicLink, format, null); [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.SupportsHardLinkCreation))] [InlineData(TarEntryFormat.Pax)] [InlineData(TarEntryFormat.Gnu)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void Extract_HardLinkEntry_TargetInsideDirectory(TarEntryFormat format) => Extract_LinkEntry_TargetInsideDirectory_Internal(TarEntryType.HardLink, format, null); [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))] [InlineData(TarEntryFormat.Pax)] [InlineData(TarEntryFormat.Gnu)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void Extract_SymbolicLinkEntry_TargetInsideDirectory_LongBaseDir(TarEntryFormat format) => Extract_LinkEntry_TargetInsideDirectory_Internal(TarEntryType.SymbolicLink, format, new string('a', 99)); [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.SupportsHardLinkCreation))] [InlineData(TarEntryFormat.Pax)] [InlineData(TarEntryFormat.Gnu)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void Extract_HardLinkEntry_TargetInsideDirectory_LongBaseDir(TarEntryFormat format) => Extract_LinkEntry_TargetInsideDirectory_Internal(TarEntryType.HardLink, format, new string('a', 99)); // This test would not pass for the V7 and Ustar formats in some OSs like MacCatalyst, tvOSSimulator and OSX, because the TempDirectory gets created in @@ -157,7 +152,6 @@ private void Extract_LinkEntry_TargetInsideDirectory_Internal(TarEntryType entry [InlineData(512)] [InlineData(512 + 1)] [InlineData(512 + 512 - 1)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void Extract_UnseekableStream_BlockAlignmentPadding_DoesNotAffectNextEntries(int contentSize) { byte[] fileContents = new byte[contentSize]; @@ -214,7 +208,6 @@ public void PaxNameCollision_DedupInExtendedAttributes() [Theory] [MemberData(nameof(GetTestTarFormats))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public void UnseekableStreams_RoundTrip(TestTarFormat testFormat) { using TempDirectory root = new(); diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs index efc8bbea35c5d9..42b0eef65eef87 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs @@ -57,7 +57,6 @@ public async Task NonExistentDirectory_Throws_Async() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public async Task SetsLastModifiedTimeOnExtractedFiles() { using TempDirectory root = new TempDirectory(); @@ -85,7 +84,6 @@ public async Task SetsLastModifiedTimeOnExtractedFiles() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public async Task SetsLastModifiedTimeOnExtractedDirectories() { using TempDirectory root = new TempDirectory(); @@ -238,7 +236,6 @@ public async Task Extract_AllSegmentsOfPath_Async() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public async Task ExtractArchiveWithEntriesThatStartWithSlashDotPrefix_Async() { using (TempDirectory root = new TempDirectory()) @@ -267,7 +264,6 @@ public async Task ExtractArchiveWithEntriesThatStartWithSlashDotPrefix_Async() [Theory] [InlineData(true)] [InlineData(false)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public async Task UnixFileModes_Async(bool overwrite) { using TempDirectory source = new TempDirectory(); @@ -334,7 +330,6 @@ public async Task UnixFileModes_Async(bool overwrite) } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public async Task UnixFileModes_RestrictiveParentDir_Async() { using TempDirectory source = new TempDirectory(); @@ -368,7 +363,6 @@ public async Task UnixFileModes_RestrictiveParentDir_Async() } [ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public async Task LinkBeforeTargetAsync() { using TempDirectory source = new TempDirectory(); diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.Stream.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.Stream.Tests.cs index 3801481ca111d1..4a163767e94043 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.Stream.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.Stream.Tests.cs @@ -136,7 +136,6 @@ public async Task ExtractEntry_PodmanImageTarWithRelativeSymlinksPointingInExtra [Theory] [InlineData(TarEntryType.SymbolicLink)] [InlineData(TarEntryType.HardLink)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public async Task Extract_LinkEntry_TargetOutsideDirectory_Async(TarEntryType entryType) { await using (MemoryStream archive = new MemoryStream()) @@ -161,25 +160,21 @@ public async Task Extract_LinkEntry_TargetOutsideDirectory_Async(TarEntryType en [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))] [InlineData(TarEntryFormat.Pax)] [InlineData(TarEntryFormat.Gnu)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public Task Extract_SymbolicLinkEntry_TargetInsideDirectory_Async(TarEntryFormat format) => Extract_LinkEntry_TargetInsideDirectory_Internal_Async(TarEntryType.SymbolicLink, format, null); [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.SupportsHardLinkCreation))] [InlineData(TarEntryFormat.Pax)] [InlineData(TarEntryFormat.Gnu)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public Task Extract_HardLinkEntry_TargetInsideDirectory_Async(TarEntryFormat format) => Extract_LinkEntry_TargetInsideDirectory_Internal_Async(TarEntryType.HardLink, format, null); [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))] [InlineData(TarEntryFormat.Pax)] [InlineData(TarEntryFormat.Gnu)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public Task Extract_SymbolicLinkEntry_TargetInsideDirectory_LongBaseDir_Async(TarEntryFormat format) => Extract_LinkEntry_TargetInsideDirectory_Internal_Async(TarEntryType.SymbolicLink, format, new string('a', 99)); [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.SupportsHardLinkCreation))] [InlineData(TarEntryFormat.Pax)] [InlineData(TarEntryFormat.Gnu)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public Task Extract_HardLinkEntry_TargetInsideDirectory_LongBaseDir_Async(TarEntryFormat format) => Extract_LinkEntry_TargetInsideDirectory_Internal_Async(TarEntryType.HardLink, format, new string('a', 99)); // This test would not pass for the V7 and Ustar formats in some OSs like MacCatalyst, tvOSSimulator and OSX, because the TempDirectory gets created in @@ -222,7 +217,6 @@ private async Task Extract_LinkEntry_TargetInsideDirectory_Internal_Async(TarEnt [InlineData(512)] [InlineData(512 + 1)] [InlineData(512 + 512 - 1)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public async Task Extract_UnseekableStream_BlockAlignmentPadding_DoesNotAffectNextEntries_Async(int contentSize) { byte[] fileContents = new byte[contentSize]; @@ -279,7 +273,6 @@ public async Task PaxNameCollision_DedupInExtendedAttributesAsync() [Theory] [MemberData(nameof(GetTestTarFormats))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] public async Task UnseekableStreams_RoundTrip_Async(TestTarFormat testFormat) { using TempDirectory root = new();