Merge pull request #988 from adamhathcock/copilot/fix-file-write-error

Fix extraction failure on Windows due to case-sensitive path comparison

src/SharpCompress/Common/ExtractionMethods.cs

@@ -1,5 +1,6 @@
 using System;
 using System.IO;
+using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -7,6 +8,15 @@ namespace SharpCompress.Common;
 
 internal static class ExtractionMethods
 {
+    /// <summary>
+    /// Gets the appropriate StringComparison for path checks based on the file system.
+    /// Windows uses case-insensitive file systems, while Unix-like systems use case-sensitive file systems.
+    /// </summary>
+    private static StringComparison PathComparison =>
+        RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
+            ? StringComparison.OrdinalIgnoreCase
+            : StringComparison.Ordinal;
+
     /// <summary>
     /// Extract to specific directory, retaining filename
     /// </summary>
@@ -48,7 +58,7 @@ internal static class ExtractionMethods
 
             if (!Directory.Exists(destdir))
             {
-                if (!destdir.StartsWith(fullDestinationDirectoryPath, StringComparison.Ordinal))
+                if (!destdir.StartsWith(fullDestinationDirectoryPath, PathComparison))
                 {
                     throw new ExtractionException(
                         "Entry is trying to create a directory outside of the destination directory."
@@ -68,12 +78,7 @@ internal static class ExtractionMethods
         {
             destinationFileName = Path.GetFullPath(destinationFileName);
 
-            if (
-                !destinationFileName.StartsWith(
-                    fullDestinationDirectoryPath,
-                    StringComparison.Ordinal
-                )
-            )
+            if (!destinationFileName.StartsWith(fullDestinationDirectoryPath, PathComparison))
             {
                 throw new ExtractionException(
                     "Entry is trying to write a file outside of the destination directory."
@@ -158,7 +163,7 @@ internal static class ExtractionMethods
 
             if (!Directory.Exists(destdir))
             {
-                if (!destdir.StartsWith(fullDestinationDirectoryPath, StringComparison.Ordinal))
+                if (!destdir.StartsWith(fullDestinationDirectoryPath, PathComparison))
                 {
                     throw new ExtractionException(
                         "Entry is trying to create a directory outside of the destination directory."
@@ -178,12 +183,7 @@ internal static class ExtractionMethods
         {
             destinationFileName = Path.GetFullPath(destinationFileName);
 
-            if (
-                !destinationFileName.StartsWith(
-                    fullDestinationDirectoryPath,
-                    StringComparison.Ordinal
-                )
-            )
+            if (!destinationFileName.StartsWith(fullDestinationDirectoryPath, PathComparison))
             {
                 throw new ExtractionException(
                     "Entry is trying to write a file outside of the destination directory."

src/SharpCompress/packages.lock.json

@@ -335,9 +335,9 @@
     "net8.0": {
       "Microsoft.NET.ILLink.Tasks": {
         "type": "Direct",
-        "requested": "[8.0.20, )",
-        "resolved": "8.0.20",
-        "contentHash": "Rhcto2AjGvTO62+/VTmBpumBOmqIGp7nYEbTbmEXkCq4yPGxV8whju3/HsIA/bKyo2+DggaYk5+/8sxb1AbPTw=="
+        "requested": "[8.0.0, )",
+        "resolved": "8.0.0",
+        "contentHash": "B3etT5XQ2nlWkZGO2m/ytDYrOmSsQG1XNBaM6ZYlX5Ch/tDrMFadr0/mK6gjZwaQc55g+5+WZMw4Cz3m8VEF7g=="
       },
       "Microsoft.SourceLink.GitHub": {
         "type": "Direct",

tests/SharpCompress.Test/ExtractionTests.cs

@@ -0,0 +1,99 @@
+using System;
+using System.IO;
+using SharpCompress.Common;
+using SharpCompress.Readers;
+using SharpCompress.Writers;
+using SharpCompress.Writers.Zip;
+using Xunit;
+
+namespace SharpCompress.Test;
+
+public class ExtractionTests : TestBase
+{
+    [Fact]
+    public void Extraction_ShouldHandleCaseInsensitivePathsOnWindows()
+    {
+        // This test validates that extraction succeeds when Path.GetFullPath returns paths
+        // with casing that matches the platform's file system behavior. On Windows,
+        // Path.GetFullPath can return different casing than the actual directory on disk
+        // (e.g., "system32" vs "System32"), and the extraction should succeed because
+        // Windows file systems are case-insensitive. On Unix-like systems, this test
+        // verifies that the case-sensitive comparison is used correctly.
+
+        var testArchive = Path.Combine(SCRATCH2_FILES_PATH, "test-extraction.zip");
+        var extractPath = SCRATCH_FILES_PATH;
+
+        // Create a simple test archive with a single file
+        using (var stream = File.Create(testArchive))
+        {
+            using var writer = (ZipWriter)
+                WriterFactory.Open(stream, ArchiveType.Zip, CompressionType.Deflate);
+
+            // Create a test file to add to the archive
+            var testFilePath = Path.Combine(SCRATCH2_FILES_PATH, "testfile.txt");
+            File.WriteAllText(testFilePath, "Test content");
+
+            writer.Write("testfile.txt", testFilePath);
+        }
+
+        // Extract the archive - this should succeed regardless of path casing
+        using (var stream = File.OpenRead(testArchive))
+        {
+            using var reader = ReaderFactory.Open(stream);
+
+            // This should not throw an exception even if Path.GetFullPath returns
+            // a path with different casing than the actual directory
+            var exception = Record.Exception(() =>
+                reader.WriteAllToDirectory(
+                    extractPath,
+                    new ExtractionOptions { ExtractFullPath = false, Overwrite = true }
+                )
+            );
+
+            Assert.Null(exception);
+        }
+
+        // Verify the file was extracted successfully
+        var extractedFile = Path.Combine(extractPath, "testfile.txt");
+        Assert.True(File.Exists(extractedFile));
+        Assert.Equal("Test content", File.ReadAllText(extractedFile));
+    }
+
+    [Fact]
+    public void Extraction_ShouldPreventPathTraversalAttacks()
+    {
+        // This test ensures that the security check still works to prevent
+        // path traversal attacks (e.g., using "../" to escape the destination directory)
+
+        var testArchive = Path.Combine(SCRATCH2_FILES_PATH, "test-traversal.zip");
+        var extractPath = SCRATCH_FILES_PATH;
+
+        // Create a test archive with a path traversal attempt
+        using (var stream = File.Create(testArchive))
+        {
+            using var writer = (ZipWriter)
+                WriterFactory.Open(stream, ArchiveType.Zip, CompressionType.Deflate);
+
+            var testFilePath = Path.Combine(SCRATCH2_FILES_PATH, "testfile2.txt");
+            File.WriteAllText(testFilePath, "Test content");
+
+            // Try to write with a path that attempts to escape the destination directory
+            writer.Write("../../evil.txt", testFilePath);
+        }
+
+        // Extract the archive - this should throw an exception for path traversal
+        using (var stream = File.OpenRead(testArchive))
+        {
+            using var reader = ReaderFactory.Open(stream);
+
+            var exception = Assert.Throws<ExtractionException>(() =>
+                reader.WriteAllToDirectory(
+                    extractPath,
+                    new ExtractionOptions { ExtractFullPath = true, Overwrite = true }
+                )
+            );
+
+            Assert.Contains("outside of the destination", exception.Message);
+        }
+    }
+}