From ef2fee0ee323be7338822dcdfd560cb719d04856 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Oct 2025 10:00:50 +0000 Subject: [PATCH 1/4] Initial plan From 5392ca9794bfab74f53fa279a7d20a10e6850b40 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Oct 2025 10:16:07 +0000 Subject: [PATCH 2/4] Fix case-sensitive path comparison on Windows for file extraction - Add PathComparison property that uses OrdinalIgnoreCase on Windows and Ordinal on Unix - Update all path comparison checks in ExtractionMethods to use PathComparison - Add comprehensive tests for extraction with case-insensitive paths - Ensure security check for path traversal still works correctly Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com> --- src/SharpCompress/Common/ExtractionMethods.cs | 27 +++-- tests/SharpCompress.Test/ExtractionTests.cs | 98 +++++++++++++++++++ 2 files changed, 111 insertions(+), 14 deletions(-) create mode 100644 tests/SharpCompress.Test/ExtractionTests.cs diff --git a/src/SharpCompress/Common/ExtractionMethods.cs b/src/SharpCompress/Common/ExtractionMethods.cs index f81f82d4c..0e3889bf2 100644 --- a/src/SharpCompress/Common/ExtractionMethods.cs +++ b/src/SharpCompress/Common/ExtractionMethods.cs @@ -7,6 +7,15 @@ namespace SharpCompress.Common; internal static class ExtractionMethods { + /// + /// 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. + /// + private static StringComparison PathComparison => + Environment.OSVersion.Platform == PlatformID.Win32NT + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + /// /// Extract to specific directory, retaining filename /// @@ -48,7 +57,7 @@ public static void WriteEntryToDirectory( 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 +77,7 @@ public static void WriteEntryToDirectory( { 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 +162,7 @@ public static async Task WriteEntryToDirectoryAsync( 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 +182,7 @@ public static async Task WriteEntryToDirectoryAsync( { 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." diff --git a/tests/SharpCompress.Test/ExtractionTests.cs b/tests/SharpCompress.Test/ExtractionTests.cs new file mode 100644 index 000000000..baf12d30f --- /dev/null +++ b/tests/SharpCompress.Test/ExtractionTests.cs @@ -0,0 +1,98 @@ +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 simulates the issue where Path.GetFullPath returns paths with different casing + // than the actual directory on disk (e.g., "system32" vs "System32" on Windows). + // On Windows, file paths are case-insensitive, so the extraction should succeed. + // On Unix-like systems, file paths are case-sensitive, so this test validates the + // platform-specific behavior. + + 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(() => + reader.WriteAllToDirectory( + extractPath, + new ExtractionOptions { ExtractFullPath = true, Overwrite = true } + ) + ); + + Assert.Contains("outside of the destination", exception.Message); + } + } +} From e11198616ef390b0ed73404e12a2eb130d65a55d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Oct 2025 10:20:05 +0000 Subject: [PATCH 3/4] Address code review feedback: use RuntimeInformation for platform detection - Replace Environment.OSVersion.Platform with RuntimeInformation.IsOSPlatform(OSPlatform.Windows) - Clarify test comment about platform-specific behavior - Add using System.Runtime.InteropServices for RuntimeInformation Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com> --- src/SharpCompress/Common/ExtractionMethods.cs | 3 ++- tests/SharpCompress.Test/ExtractionTests.cs | 11 ++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/SharpCompress/Common/ExtractionMethods.cs b/src/SharpCompress/Common/ExtractionMethods.cs index 0e3889bf2..485fdf4d3 100644 --- a/src/SharpCompress/Common/ExtractionMethods.cs +++ b/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; @@ -12,7 +13,7 @@ internal static class ExtractionMethods /// Windows uses case-insensitive file systems, while Unix-like systems use case-sensitive file systems. /// private static StringComparison PathComparison => - Environment.OSVersion.Platform == PlatformID.Win32NT + RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; diff --git a/tests/SharpCompress.Test/ExtractionTests.cs b/tests/SharpCompress.Test/ExtractionTests.cs index baf12d30f..04b5b08f6 100644 --- a/tests/SharpCompress.Test/ExtractionTests.cs +++ b/tests/SharpCompress.Test/ExtractionTests.cs @@ -13,11 +13,12 @@ public class ExtractionTests : TestBase [Fact] public void Extraction_ShouldHandleCaseInsensitivePathsOnWindows() { - // This test simulates the issue where Path.GetFullPath returns paths with different casing - // than the actual directory on disk (e.g., "system32" vs "System32" on Windows). - // On Windows, file paths are case-insensitive, so the extraction should succeed. - // On Unix-like systems, file paths are case-sensitive, so this test validates the - // platform-specific behavior. + // 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; From 150d9c35b755159e67b086edb751e2352751e1e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Oct 2025 10:22:56 +0000 Subject: [PATCH 4/4] Complete fix for case-sensitive path comparison on Windows Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com> --- src/SharpCompress/packages.lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SharpCompress/packages.lock.json b/src/SharpCompress/packages.lock.json index b85a38f73..904860ce8 100644 --- a/src/SharpCompress/packages.lock.json +++ b/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",