mirror of
https://github.com/adamhathcock/sharpcompress.git
synced 2026-09-25 08:24:58 +00:00
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>
This commit is contained in:
@@ -7,6 +7,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 =>
|
||||
Environment.OSVersion.Platform == PlatformID.Win32NT
|
||||
? StringComparison.OrdinalIgnoreCase
|
||||
: StringComparison.Ordinal;
|
||||
|
||||
/// <summary>
|
||||
/// Extract to specific directory, retaining filename
|
||||
/// </summary>
|
||||
@@ -48,7 +57,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 +77,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 +162,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 +182,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."
|
||||
|
||||
98
tests/SharpCompress.Test/ExtractionTests.cs
Normal file
98
tests/SharpCompress.Test/ExtractionTests.cs
Normal file
@@ -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<ExtractionException>(() =>
|
||||
reader.WriteAllToDirectory(
|
||||
extractPath,
|
||||
new ExtractionOptions { ExtractFullPath = true, Overwrite = true }
|
||||
)
|
||||
);
|
||||
|
||||
Assert.Contains("outside of the destination", exception.Message);
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user