Skip to content

Fix failing symlink tests #130342

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.

Sign up for GitHub

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

Merged
alinpahontu2912 merged 4 commits into from
Jul 15, 2026
Merged

Fix failing symlink tests #130342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b7e8c7b
fix tests
alinpahontu2912 Jul 8, 2026
afd4329
Guard destination-root path in FilePathEscapesDirectory and re-enable…
alinpahontu2912 Jul 9, 2026
ac6974c
Potential fix for pull request finding
alinpahontu2912 Jul 9, 2026
198927c
Merge branch 'release/8.0-staging' into tar_fix_8
alinpahontu2912 Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 35 additions & 27 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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.
Comment thread
alinpahontu2912 marked this conversation as resolved.
if (OperatingSystem.IsWindows() && Path.IsPathRooted(linkName) && !Path.IsPathFullyQualified(linkName))
{
throw new IOException(SR.Format(SR.TarExtractingResultsLinkOutside, linkName, destinationDirectoryPath));
Expand Down Expand Up @@ -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.
Comment thread
alinpahontu2912 marked this conversation as resolved.
private static bool FilePathEscapesDirectory(string destinationDirectoryPath, string fileDestinationPath)
{
// Windows is case insensitive while Linux is case sensitive
Expand All @@ -394,16 +395,34 @@ 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;

// 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);
Expand All @@ -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) &&
Expand All @@ -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;
}
Comment thread
alinpahontu2912 marked this conversation as resolved.

Expand All @@ -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);
}
}

Expand Down
8 changes: 0 additions & 8 deletions src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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/
Expand Down
7 changes: 0 additions & 7 deletions src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Stream.Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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();
Expand Down
6 changes: 0 additions & 6 deletions src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
7 changes: 0 additions & 7 deletions ...ibraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.Stream.Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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();
Expand Down
Loading