Fix bug in Tar preventing extraction of hardlinks or entries starting…

… with `.\` (#70853)

* Add PlatformDetection.SupportsHardLinkCreation property.

* Fix how paths are combined/joined and sanitized on extraction, to ensure paths with redundant segments get properly handled.

* Add tests that verify archives with entries whose paths start with .\, including the root folder itself.

* Re-enable the hardlink test, condition it to not run if platform does not support extraction of hardlinks.

* Remove unnecessary test - This same code is already being tested by TarReader_ExtractToFile_Tests.ExtractEntriesWithSlashDotPrefix

* Reuse test code that retrieves memory stream.

* Bump test data package version

* Add missing typeof(PlatformDetection) in ConditionalFact

Co-authored-by: carlossanlop <[email protected]>

eng/Version.Details.xml

@@ -138,9 +138,9 @@
       <Uri>https://github.com/dotnet/runtime-assets</Uri>
       <Sha>0920468fa7db4ee8ea8bbcba186421cb92713adf</Sha>
     </Dependency>
-    <Dependency Name="System.Formats.Tar.TestData" Version="7.0.0-beta.22281.1">
+    <Dependency Name="System.Formats.Tar.TestData" Version="7.0.0-beta.22313.1">
       <Uri>https://github.com/dotnet/runtime-assets</Uri>
-      <Sha>0920468fa7db4ee8ea8bbcba186421cb92713adf</Sha>
+      <Sha>371af1f99788b76eae14b96aad4ab7ac9b373938</Sha>
     </Dependency>
     <Dependency Name="System.IO.Compression.TestData" Version="7.0.0-beta.22281.1">
       <Uri>https://github.com/dotnet/runtime-assets</Uri>

eng/Versions.props

@@ -120,7 +120,7 @@
     <SystemRuntimeNumericsTestDataVersion>7.0.0-beta.22281.1</SystemRuntimeNumericsTestDataVersion>
     <SystemComponentModelTypeConverterTestDataVersion>7.0.0-beta.22281.1</SystemComponentModelTypeConverterTestDataVersion>
     <SystemDrawingCommonTestDataVersion>7.0.0-beta.22281.1</SystemDrawingCommonTestDataVersion>
-    <SystemFormatsTarTestDataVersion>7.0.0-beta.22281.1</SystemFormatsTarTestDataVersion>
+    <SystemFormatsTarTestDataVersion>7.0.0-beta.22313.1</SystemFormatsTarTestDataVersion>
     <SystemIOCompressionTestDataVersion>7.0.0-beta.22281.1</SystemIOCompressionTestDataVersion>
     <SystemIOPackagingTestDataVersion>7.0.0-beta.22281.1</SystemIOPackagingTestDataVersion>
     <SystemNetTestDataVersion>7.0.0-beta.22281.1</SystemNetTestDataVersion>

src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs

@@ -238,6 +238,7 @@ private static bool GetAlpnSupport()
 
         public static bool SupportsAlpn => s_supportsAlpn.Value;
         public static bool SupportsClientAlpn => SupportsAlpn || IsOSX || IsMacCatalyst || IsiOS || IstvOS;
+        public static bool SupportsHardLinkCreation => !IsAndroid;
 
         private static readonly Lazy<bool> s_supportsTls10 = new Lazy<bool>(GetTls10Support);
         private static readonly Lazy<bool> s_supportsTls11 = new Lazy<bool>(GetTls11Support);

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs

@@ -261,9 +261,13 @@ internal void ExtractRelativeToDirectory(string destinationDirectoryPath, bool o
             Debug.Assert(!string.IsNullOrEmpty(destinationDirectoryPath));
             Debug.Assert(Path.IsPathFullyQualified(destinationDirectoryPath));
 
-            string destinationDirectoryFullPath = destinationDirectoryPath.EndsWith(Path.DirectorySeparatorChar) ? destinationDirectoryPath : destinationDirectoryPath + Path.DirectorySeparatorChar;
+            destinationDirectoryPath = Path.TrimEndingDirectorySeparator(destinationDirectoryPath);
 
-            string fileDestinationPath = GetSanitizedFullPath(destinationDirectoryFullPath, Name, SR.TarExtractingResultsFileOutside);
+            string? fileDestinationPath = GetSanitizedFullPath(destinationDirectoryPath, Name);
+            if (fileDestinationPath == null)
+            {
+                throw new IOException(string.Format(SR.TarExtractingResultsFileOutside, Name, destinationDirectoryPath));
+            }
 
             string? linkTargetPath = null;
             if (EntryType is TarEntryType.SymbolicLink or TarEntryType.HardLink)
@@ -273,7 +277,11 @@ internal void ExtractRelativeToDirectory(string destinationDirectoryPath, bool o
                     throw new FormatException(SR.TarEntryHardLinkOrSymlinkLinkNameEmpty);
                 }
 
-                linkTargetPath = GetSanitizedFullPath(destinationDirectoryFullPath, LinkName, SR.TarExtractingResultsLinkOutside);
+                linkTargetPath = GetSanitizedFullPath(destinationDirectoryPath, LinkName);
+                if (linkTargetPath == null)
+                {
+                    throw new IOException(string.Format(SR.TarExtractingResultsLinkOutside, LinkName, destinationDirectoryPath));
+                }
             }
 
             if (EntryType == TarEntryType.Directory)
@@ -286,26 +294,15 @@ internal void ExtractRelativeToDirectory(string destinationDirectoryPath, bool o
                 Directory.CreateDirectory(Path.GetDirectoryName(fileDestinationPath)!);
                 ExtractToFileInternal(fileDestinationPath, linkTargetPath, overwrite);
             }
+        }
 
-            // If the path can be extracted in the specified destination directory, returns the full path with sanitized file name. Otherwise, throws.
-            static string GetSanitizedFullPath(string destinationDirectoryFullPath, string path, string exceptionMessage)
-            {
-                string actualPath = Path.Join(Path.GetDirectoryName(path), ArchivingUtils.SanitizeEntryFilePath(Path.GetFileName(path)));
-
-                if (!Path.IsPathFullyQualified(actualPath))
-                {
-                    actualPath = Path.Combine(destinationDirectoryFullPath, actualPath);
-                }
-
-                actualPath = Path.GetFullPath(actualPath);
-
-                if (!actualPath.StartsWith(destinationDirectoryFullPath, PathInternal.StringComparison))
-                {
-                    throw new IOException(string.Format(exceptionMessage, path, destinationDirectoryFullPath));
-                }
-
-                return actualPath;
-            }
+        // If the path can be extracted in the specified destination directory, returns the full path with sanitized file name. Otherwise, returns null.
+        private static string? GetSanitizedFullPath(string destinationDirectoryFullPath, string path)
+        {
+            string fullyQualifiedPath = Path.IsPathFullyQualified(path) ? path : Path.Combine(destinationDirectoryFullPath, path);
+            string normalizedPath = Path.GetFullPath(fullyQualifiedPath); // Removes relative segments
+            string sanitizedPath = Path.Join(Path.GetDirectoryName(normalizedPath), ArchivingUtils.SanitizeEntryFilePath(Path.GetFileName(normalizedPath)));
+            return sanitizedPath.StartsWith(destinationDirectoryFullPath, PathInternal.StringComparison) ? sanitizedPath : null;
         }
 
         // Extracts the current entry into the filesystem, regardless of the entry type.

src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj

@@ -17,6 +17,7 @@
     <Compile Include="TarFile\TarFile.CreateFromDirectory.Stream.Tests.cs" />
     <Compile Include="TarFile\TarFile.ExtractToDirectory.File.Tests.cs" />
     <Compile Include="TarFile\TarFile.ExtractToDirectory.Stream.Tests.cs" />
+    <Compile Include="TarReader\TarReader.ExtractToFile.Tests.cs" />
     <Compile Include="TarReader\TarReader.File.Tests.cs" />
     <Compile Include="TarReader\TarReader.GetNextEntry.Tests.cs" />
     <Compile Include="TarTestsBase.cs" />
@@ -52,6 +53,7 @@
   <!-- Unix specific files -->
   <ItemGroup Condition="'$(TargetPlatformIdentifier)' != 'windows'">
     <Compile Include="TarFile\TarFile.ExtractToDirectory.File.Tests.Unix.cs" />
+    <Compile Include="TarReader\TarReader.ExtractToFile.Tests.Unix.cs" />
     <Compile Include="TarWriter\TarWriter.WriteEntry.File.Tests.Unix.cs" />
     <Compile Include="$(CommonPath)Interop\Unix\Interop.Errors.cs" Link="Common\Interop\Unix\Interop.Errors.cs" />
     <Compile Include="$(CommonPath)Interop\Unix\Interop.IOErrors.cs" Link="Common\Interop\Unix\Interop.IOErrors.cs" />

src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs

@@ -136,5 +136,28 @@ public void Extract_AllSegmentsOfPath()
             string filePath = Path.Join(segment2Path, "file.txt");
             Assert.True(File.Exists(filePath), $"{filePath}' does not exist.");
         }
+
+        [Fact]
+        public void ExtractArchiveWithEntriesThatStartWithSlashDotPrefix()
+        {
+            using TempDirectory root = new TempDirectory();
+
+            using MemoryStream archiveStream = GetStrangeTarMemoryStream("prefixDotSlashAndCurrentFolderEntry");
+
+            TarFile.ExtractToDirectory(archiveStream, root.Path, overwriteFiles: true);
+
+            archiveStream.Position = 0;
+
+            using TarReader reader = new TarReader(archiveStream, leaveOpen: false);
+
+            TarEntry entry;
+            while ((entry = reader.GetNextEntry()) != null)
+            {
+                // Normalize the path (remove redundant segments), remove trailing separators
+                // this is so the first entry can be skipped if it's the same as the root directory
+                string entryPath = Path.TrimEndingDirectorySeparator(Path.GetFullPath(Path.Join(root.Path, entry.Name)));
+                Assert.True(Path.Exists(entryPath), $"Entry was not extracted: {entryPath}");
+            }
+        }
     }
 }

src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Stream.Tests.cs

@@ -97,8 +97,7 @@ public void Extract_LinkEntry_TargetOutsideDirectory(TarEntryType entryType)
         [ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
         public void Extract_SymbolicLinkEntry_TargetInsideDirectory() => Extract_LinkEntry_TargetInsideDirectory_Internal(TarEntryType.SymbolicLink);
 
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/68360", TestPlatforms.Android | TestPlatforms.LinuxBionic | TestPlatforms.iOS | TestPlatforms.tvOS)]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsHardLinkCreation))]
         public void Extract_HardLinkEntry_TargetInsideDirectory() => Extract_LinkEntry_TargetInsideDirectory_Internal(TarEntryType.HardLink);
 
         private void Extract_LinkEntry_TargetInsideDirectory_Internal(TarEntryType entryType)

src/libraries/System.Formats.Tar/tests/TarReader/TarReader.ExtractToFile.Tests.Unix.cs

@@ -0,0 +1,45 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Reflection;
+using Xunit;
+
+namespace System.Formats.Tar.Tests
+{
+    public partial class TarReader_ExtractToFile_Tests : TarTestsBase
+    {
+        [Fact]
+        public void ExtractToFile_SpecialFile_Unelevated_Throws()
+        {
+            using TempDirectory root = new TempDirectory();
+            using MemoryStream ms = GetTarMemoryStream(CompressionMethod.Uncompressed, TestTarFormat.ustar, "specialfiles");
+
+            using TarReader reader = new TarReader(ms);
+
+            string path = Path.Join(root.Path, "output");
+
+            // Block device requires elevation for writing
+            PosixTarEntry blockDevice = reader.GetNextEntry() as PosixTarEntry;
+            Assert.NotNull(blockDevice);
+            Assert.Throws<UnauthorizedAccessException>(() => blockDevice.ExtractToFile(path, overwrite: false));
+            Assert.False(File.Exists(path));
+
+            // Character device requires elevation for writing
+            PosixTarEntry characterDevice = reader.GetNextEntry() as PosixTarEntry;
+            Assert.NotNull(characterDevice);
+            Assert.Throws<UnauthorizedAccessException>(() => characterDevice.ExtractToFile(path, overwrite: false));
+            Assert.False(File.Exists(path));
+
+            // Fifo does not require elevation, should succeed
+            PosixTarEntry fifo = reader.GetNextEntry() as PosixTarEntry;
+            Assert.NotNull(fifo);
+            fifo.ExtractToFile(path, overwrite: false);
+            Assert.True(File.Exists(path));
+
+            Assert.Null(reader.GetNextEntry());
+        }
+    }
+}

src/libraries/System.Formats.Tar/tests/TarReader/TarReader.ExtractToFile.Tests.cs

@@ -1,44 +1,38 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Collections.Generic;
 using System.IO;
-using System.Linq;
 using Xunit;
 
 namespace System.Formats.Tar.Tests
 {
-    public class TarReader_ExtractToFile_Tests : TarTestsBase
+    public partial class TarReader_ExtractToFile_Tests : TarTestsBase
     {
         [Fact]
-        public void ExtractToFile_SpecialFile_Unelevated_Throws()
+        public void ExtractEntriesWithSlashDotPrefix()
         {
             using TempDirectory root = new TempDirectory();
-            using MemoryStream ms = GetTarMemoryStream(CompressionMethod.Uncompressed, TestTarFormat.ustar, "specialfiles");
 
-            using TarReader reader = new TarReader(ms);
-
-            string path = Path.Join(root.Path, "output");
-
-            // Block device requires elevation for writing
-            PosixTarEntry blockDevice = reader.GetNextEntry() as PosixTarEntry;
-            Assert.NotNull(blockDevice);
-            Assert.Throws<UnauthorizedAccessException>(() => blockDevice.ExtractToFile(path, overwrite: false));
-            Assert.False(File.Exists(path));
-
-            // Character device requires elevation for writing
-            PosixTarEntry characterDevice = reader.GetNextEntry() as PosixTarEntry;
-            Assert.NotNull(characterDevice);
-            Assert.Throws<UnauthorizedAccessException>(() => characterDevice.ExtractToFile(path, overwrite: false));
-            Assert.False(File.Exists(path));
-
-            // Fifo does not require elevation, should succeed
-            PosixTarEntry fifo = reader.GetNextEntry() as PosixTarEntry;
-            Assert.NotNull(fifo);
-            fifo.ExtractToFile(path, overwrite: false);
-            Assert.True(File.Exists(path));
-
-            Assert.Null(reader.GetNextEntry());
+            using MemoryStream archiveStream = GetStrangeTarMemoryStream("prefixDotSlashAndCurrentFolderEntry");
+            using (TarReader reader = new TarReader(archiveStream, leaveOpen: false))
+            {
+                string rootPath = Path.TrimEndingDirectorySeparator(root.Path);
+                TarEntry entry;
+                while ((entry = reader.GetNextEntry()) != null)
+                {
+                    Assert.NotNull(entry);
+                    Assert.StartsWith("./", entry.Name);
+                    // Normalize the path (remove redundant segments), remove trailing separators
+                    // this is so the first entry can be skipped if it's the same as the root directory
+                    string entryPath = Path.TrimEndingDirectorySeparator(Path.GetFullPath(Path.Join(rootPath, entry.Name)));
+                    if (entryPath != rootPath)
+                    {
+                        entry.ExtractToFile(entryPath, overwrite: true);
+                        Assert.True(Path.Exists(entryPath), $"Entry was not extracted: {entryPath}");
+                    }
+                }
+            }
         }
+
     }
 }

src/libraries/System.Formats.Tar/tests/TarTestsBase.cs

@@ -96,9 +96,17 @@ protected static string GetTarFilePath(CompressionMethod compressionMethod, Test
         }
 
         // MemoryStream containing the copied contents of the specified file. Meant for reading and writing.
-        protected static MemoryStream GetTarMemoryStream(CompressionMethod compressionMethod, TestTarFormat format, string testCaseName)
+        protected static MemoryStream GetTarMemoryStream(CompressionMethod compressionMethod, TestTarFormat format, string testCaseName) =>
+            GetMemoryStream(GetTarFilePath(compressionMethod, format, testCaseName));
+
+        protected static string GetStrangeTarFilePath(string testCaseName) =>
+            Path.Join(Directory.GetCurrentDirectory(), "strange", testCaseName + ".tar");
+
+        protected static MemoryStream GetStrangeTarMemoryStream(string testCaseName) =>
+            GetMemoryStream(GetStrangeTarFilePath(testCaseName));
+
+        private static MemoryStream GetMemoryStream(string path)
         {
-            string path = GetTarFilePath(compressionMethod, format, testCaseName);
             MemoryStream ms = new();
             using (FileStream fs = File.OpenRead(path))
             {