From 2021a06626d0555a4d69471386e763ca5f5d5dfb Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 5 May 2026 16:52:16 +0100 Subject: [PATCH 1/7] add zipslip tests and fixes --- .../Archives/IArchiveExtensions.cs | 52 ++++++-- .../Archives/IAsyncArchiveExtensions.cs | 51 ++++++-- tests/SharpCompress.Test/Security/ZipSlip.cs | 118 ++++++++++++++++++ 3 files changed, 202 insertions(+), 19 deletions(-) create mode 100644 tests/SharpCompress.Test/Security/ZipSlip.cs diff --git a/src/SharpCompress/Archives/IArchiveExtensions.cs b/src/SharpCompress/Archives/IArchiveExtensions.cs index 80857a25..9e80c47f 100644 --- a/src/SharpCompress/Archives/IArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IArchiveExtensions.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Runtime.InteropServices; using SharpCompress.Common; using SharpCompress.Readers; @@ -8,6 +9,15 @@ namespace SharpCompress.Archives; public static class IArchiveExtensions { + + /// + /// 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 => + RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; extension(IArchive archive) { /// @@ -39,6 +49,25 @@ public static class IArchiveExtensions IProgress? progress ) { + var fullDestinationDirectoryPath = Path.GetFullPath(destinationDirectory); + options ??= new ExtractionOptions(); + + //check for trailing slash. + if ( + fullDestinationDirectoryPath[fullDestinationDirectoryPath.Length - 1] + != Path.DirectorySeparatorChar + ) + { + fullDestinationDirectoryPath += Path.DirectorySeparatorChar; + } + + if (!Directory.Exists(fullDestinationDirectoryPath)) + { + throw new ExtractionException( + $"Directory does not exist to extract to: {fullDestinationDirectoryPath}" + ); + } + var totalBytes = archive.TotalUncompressedSize; var bytesRead = 0L; var seenDirectories = new HashSet(); @@ -47,16 +76,21 @@ public static class IArchiveExtensions { if (entry.IsDirectory) { - var dirPath = Path.Combine( - destinationDirectory, - entry.Key.NotNull("Entry Key is null") - ); - if ( - Path.GetDirectoryName(dirPath + "/") is { } parentDirectory - && seenDirectories.Add(dirPath) - ) + + var folder = Path.GetDirectoryName(entry.Key.NotNull("Entry Key is null")) + .NotNull("Directory is null"); + var destdir = Path.GetFullPath(Path.Combine(fullDestinationDirectoryPath, folder)); + + if (!Directory.Exists(destdir) && seenDirectories.Add(destdir)) { - Directory.CreateDirectory(parentDirectory); + if (!destdir.StartsWith(fullDestinationDirectoryPath, PathComparison)) + { + throw new ExtractionException( + "Entry is trying to create a directory outside of the destination directory." + ); + } + + Directory.CreateDirectory(destdir); } continue; } diff --git a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs index 9ba59977..5d9ef261 100644 --- a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using SharpCompress.Common; @@ -10,6 +11,14 @@ namespace SharpCompress.Archives; public static class IAsyncArchiveExtensions { + /// + /// 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 => + RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; extension(IAsyncArchive archive) { /// @@ -58,7 +67,25 @@ public static class IAsyncArchiveExtensions IProgress? progress, CancellationToken cancellationToken ) - { + { var fullDestinationDirectoryPath = Path.GetFullPath(destinationDirectory); + options ??= new ExtractionOptions(); + + //check for trailing slash. + if ( + fullDestinationDirectoryPath[fullDestinationDirectoryPath.Length - 1] + != Path.DirectorySeparatorChar + ) + { + fullDestinationDirectoryPath += Path.DirectorySeparatorChar; + } + + if (!Directory.Exists(fullDestinationDirectoryPath)) + { + throw new ExtractionException( + $"Directory does not exist to extract to: {fullDestinationDirectoryPath}" + ); + } + var totalBytes = await archive.TotalUncompressedSizeAsync().ConfigureAwait(false); var bytesRead = 0L; var seenDirectories = new HashSet(); @@ -69,16 +96,20 @@ public static class IAsyncArchiveExtensions if (entry.IsDirectory) { - var dirPath = Path.Combine( - destinationDirectory, - entry.Key.NotNull("Entry Key is null") - ); - if ( - Path.GetDirectoryName(dirPath + "/") is { } parentDirectory - && seenDirectories.Add(dirPath) - ) + var folder = Path.GetDirectoryName(entry.Key.NotNull("Entry Key is null")) + .NotNull("Directory is null"); + var destdir = Path.GetFullPath(Path.Combine(fullDestinationDirectoryPath, folder)); + + if (!Directory.Exists(destdir) && seenDirectories.Add(destdir)) { - Directory.CreateDirectory(parentDirectory); + if (!destdir.StartsWith(fullDestinationDirectoryPath, PathComparison)) + { + throw new ExtractionException( + "Entry is trying to create a directory outside of the destination directory." + ); + } + + Directory.CreateDirectory(destdir); } continue; } diff --git a/tests/SharpCompress.Test/Security/ZipSlip.cs b/tests/SharpCompress.Test/Security/ZipSlip.cs new file mode 100644 index 00000000..0b4bcd3d --- /dev/null +++ b/tests/SharpCompress.Test/Security/ZipSlip.cs @@ -0,0 +1,118 @@ +#if NET8_0_OR_GREATER +using System; +using System.IO; +using System.Threading.Tasks; +using AwesomeAssertions; +using SharpCompress.Archives; +using SharpCompress.Common; +using Xunit; +using SysZip = System.IO.Compression.ZipArchive; +using SysZipMode = System.IO.Compression.ZipArchiveMode; + +namespace SharpCompress.Test.Security; + +public class ZipSlip: TestBase +{ + + [Fact] +public void RunSync() +{ + Console.WriteLine("--- Sync: archive.WriteToDirectory() ---"); + var (extractDir, parentDir) = SetupDirs("sync"); + Directory.CreateDirectory(extractDir); + var archivePath = Path.Combine(parentDir, "malicious.zip"); + + BuildMaliciousZip(archivePath); + + using var archive = ArchiveFactory.OpenArchive(archivePath); + + var ex = Assert.Throws(() => + archive.WriteToDirectory(extractDir, new ExtractionOptions { ExtractFullPath = true })); + ex.Message.Should().Contain("Entry is trying to create a directory outside of the destination directory"); + + CheckResults(parentDir, extractDir); +} + + [Fact] +public async Task RunAsync() +{ + Console.WriteLine("--- Async: archive.WriteToDirectoryAsync() ---"); + var (extractDir, parentDir) = SetupDirs("async"); + Directory.CreateDirectory(extractDir); + var archivePath = Path.Combine(parentDir, "malicious.zip"); + + BuildMaliciousZip(archivePath); + + var archive = await ArchiveFactory.OpenAsyncArchive(archivePath); + await using (archive) + { + + var ex = await Assert.ThrowsAsync(async () => + + await archive.WriteToDirectoryAsync(extractDir, + new ExtractionOptions + { + ExtractFullPath = true + })); + ex.Message.Should().Contain("Entry is trying to create a directory outside of the destination directory"); + } + + CheckResults(parentDir, extractDir); +} + +// Craft a ZIP with malicious directory entries using System.IO.Compression +// so we bypass any SharpCompress write-side normalisation. +static void BuildMaliciousZip(string path) +{ + using var fs = File.Create(path); + using var zip = new SysZip(fs, SysZipMode.Create); + + // 1. Relative traversal: two levels up, then "escaped_relative/" + zip.CreateEntry("../../escaped_relative/"); + + // 2. Absolute Unix path (Path.Combine discards the base when second arg is rooted) + zip.CreateEntry("/tmp/escaped_absolute/"); + + // 3. A legitimate entry for contrast + zip.CreateEntry("safe_subdir/"); +} + +static (string extractDir, string parentDir) SetupDirs(string label) +{ + var parentDir = Path.Combine(TEST_ARCHIVES_PATH, $"sc_poc_{label}_{Path.GetRandomFileName()}"); + Directory.CreateDirectory(parentDir); + var extractDir = Path.Combine(parentDir, "extract_target"); + + Console.WriteLine($" Parent : {parentDir}"); + Console.WriteLine($" Target : {extractDir}"); + return (extractDir, parentDir); +} + +static void CheckResults(string parentDir, string extractDir) +{ + Console.WriteLine(" Directories created after extraction:"); + foreach (var d in Directory.GetDirectories(parentDir, "*", SearchOption.AllDirectories)) + { + var relative = Path.GetRelativePath(parentDir, d); + var escaped = !d.StartsWith(extractDir, StringComparison.Ordinal); + Console.WriteLine($" {(escaped ? "[ESCAPED]" : "[ ok ]")} {relative}"); + } + + // Relative traversal "../../escaped_relative/" escapes two levels above extractDir + // (which is parentDir/extract_target), landing in Path.GetTempPath() + var relTarget = Path.GetFullPath(Path.Combine(extractDir, "../../escaped_relative")); + if (Directory.Exists(relTarget)) + { + Console.WriteLine($" [ESCAPED] relative traversal created: {relTarget}"); + Directory.Delete(relTarget); + } + + var absTarget = "/tmp/escaped_absolute"; + if (Directory.Exists(absTarget)) + { + Console.WriteLine($" [ESCAPED] absolute path created: {absTarget}"); + Directory.Delete(absTarget); + } +} +} +#endif From 3b29de3954a7e1e21e48bb2380da9bf0bab70535 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 5 May 2026 16:57:15 +0100 Subject: [PATCH 2/7] test clean up and formatting --- .../Archives/IArchiveExtensions.cs | 10 +- .../Archives/IAsyncArchiveExtensions.cs | 11 +- tests/SharpCompress.Test/Security/ZipSlip.cs | 184 +++++++++--------- 3 files changed, 109 insertions(+), 96 deletions(-) diff --git a/src/SharpCompress/Archives/IArchiveExtensions.cs b/src/SharpCompress/Archives/IArchiveExtensions.cs index 9e80c47f..a1ac2226 100644 --- a/src/SharpCompress/Archives/IArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IArchiveExtensions.cs @@ -9,7 +9,6 @@ namespace SharpCompress.Archives; public static class IArchiveExtensions { - /// /// 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. @@ -76,12 +75,13 @@ public static class IArchiveExtensions { if (entry.IsDirectory) { - var folder = Path.GetDirectoryName(entry.Key.NotNull("Entry Key is null")) - .NotNull("Directory is null"); - var destdir = Path.GetFullPath(Path.Combine(fullDestinationDirectoryPath, folder)); + .NotNull("Directory is null"); + var destdir = Path.GetFullPath( + Path.Combine(fullDestinationDirectoryPath, folder) + ); - if (!Directory.Exists(destdir) && seenDirectories.Add(destdir)) + if (!Directory.Exists(destdir) && seenDirectories.Add(destdir)) { if (!destdir.StartsWith(fullDestinationDirectoryPath, PathComparison)) { diff --git a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs index 5d9ef261..51e8f3f2 100644 --- a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs @@ -67,7 +67,8 @@ public static class IAsyncArchiveExtensions IProgress? progress, CancellationToken cancellationToken ) - { var fullDestinationDirectoryPath = Path.GetFullPath(destinationDirectory); + { + var fullDestinationDirectoryPath = Path.GetFullPath(destinationDirectory); options ??= new ExtractionOptions(); //check for trailing slash. @@ -97,10 +98,12 @@ public static class IAsyncArchiveExtensions if (entry.IsDirectory) { var folder = Path.GetDirectoryName(entry.Key.NotNull("Entry Key is null")) - .NotNull("Directory is null"); - var destdir = Path.GetFullPath(Path.Combine(fullDestinationDirectoryPath, folder)); + .NotNull("Directory is null"); + var destdir = Path.GetFullPath( + Path.Combine(fullDestinationDirectoryPath, folder) + ); - if (!Directory.Exists(destdir) && seenDirectories.Add(destdir)) + if (!Directory.Exists(destdir) && seenDirectories.Add(destdir)) { if (!destdir.StartsWith(fullDestinationDirectoryPath, PathComparison)) { diff --git a/tests/SharpCompress.Test/Security/ZipSlip.cs b/tests/SharpCompress.Test/Security/ZipSlip.cs index 0b4bcd3d..251b48d4 100644 --- a/tests/SharpCompress.Test/Security/ZipSlip.cs +++ b/tests/SharpCompress.Test/Security/ZipSlip.cs @@ -11,108 +11,118 @@ using SysZipMode = System.IO.Compression.ZipArchiveMode; namespace SharpCompress.Test.Security; -public class ZipSlip: TestBase +public class ZipSlip : TestBase { + [Fact] + public void RunSync() + { + Console.WriteLine("--- Sync: archive.WriteToDirectory() ---"); + var (extractDir, parentDir) = SetupDirs("sync"); + Directory.CreateDirectory(extractDir); + var archivePath = Path.Combine(parentDir, "malicious.zip"); + + BuildMaliciousZip(archivePath); + + using var archive = ArchiveFactory.OpenArchive(archivePath); + + var ex = Assert.Throws(() => + archive.WriteToDirectory(extractDir, new ExtractionOptions { ExtractFullPath = true }) + ); + ex.Message.Should() + .Contain("Entry is trying to create a directory outside of the destination directory"); + + CheckResults(archivePath, parentDir, extractDir); + } [Fact] -public void RunSync() -{ - Console.WriteLine("--- Sync: archive.WriteToDirectory() ---"); - var (extractDir, parentDir) = SetupDirs("sync"); - Directory.CreateDirectory(extractDir); - var archivePath = Path.Combine(parentDir, "malicious.zip"); - - BuildMaliciousZip(archivePath); - - using var archive = ArchiveFactory.OpenArchive(archivePath); - - var ex = Assert.Throws(() => - archive.WriteToDirectory(extractDir, new ExtractionOptions { ExtractFullPath = true })); - ex.Message.Should().Contain("Entry is trying to create a directory outside of the destination directory"); - - CheckResults(parentDir, extractDir); -} - - [Fact] -public async Task RunAsync() -{ - Console.WriteLine("--- Async: archive.WriteToDirectoryAsync() ---"); - var (extractDir, parentDir) = SetupDirs("async"); - Directory.CreateDirectory(extractDir); - var archivePath = Path.Combine(parentDir, "malicious.zip"); - - BuildMaliciousZip(archivePath); - - var archive = await ArchiveFactory.OpenAsyncArchive(archivePath); - await using (archive) + public async Task RunAsync() { + Console.WriteLine("--- Async: archive.WriteToDirectoryAsync() ---"); + var (extractDir, parentDir) = SetupDirs("async"); + Directory.CreateDirectory(extractDir); + var archivePath = Path.Combine(parentDir, "malicious.zip"); - var ex = await Assert.ThrowsAsync(async () => + BuildMaliciousZip(archivePath); - await archive.WriteToDirectoryAsync(extractDir, - new ExtractionOptions - { - ExtractFullPath = true - })); - ex.Message.Should().Contain("Entry is trying to create a directory outside of the destination directory"); + var archive = await ArchiveFactory.OpenAsyncArchive(archivePath); + await using (archive) + { + var ex = await Assert.ThrowsAsync(async () => + await archive.WriteToDirectoryAsync( + extractDir, + new ExtractionOptions { ExtractFullPath = true } + ) + ); + ex.Message.Should() + .Contain( + "Entry is trying to create a directory outside of the destination directory" + ); + } + + CheckResults(archivePath, parentDir, extractDir); } - CheckResults(parentDir, extractDir); -} - -// Craft a ZIP with malicious directory entries using System.IO.Compression -// so we bypass any SharpCompress write-side normalisation. -static void BuildMaliciousZip(string path) -{ - using var fs = File.Create(path); - using var zip = new SysZip(fs, SysZipMode.Create); - - // 1. Relative traversal: two levels up, then "escaped_relative/" - zip.CreateEntry("../../escaped_relative/"); - - // 2. Absolute Unix path (Path.Combine discards the base when second arg is rooted) - zip.CreateEntry("/tmp/escaped_absolute/"); - - // 3. A legitimate entry for contrast - zip.CreateEntry("safe_subdir/"); -} - -static (string extractDir, string parentDir) SetupDirs(string label) -{ - var parentDir = Path.Combine(TEST_ARCHIVES_PATH, $"sc_poc_{label}_{Path.GetRandomFileName()}"); - Directory.CreateDirectory(parentDir); - var extractDir = Path.Combine(parentDir, "extract_target"); - - Console.WriteLine($" Parent : {parentDir}"); - Console.WriteLine($" Target : {extractDir}"); - return (extractDir, parentDir); -} - -static void CheckResults(string parentDir, string extractDir) -{ - Console.WriteLine(" Directories created after extraction:"); - foreach (var d in Directory.GetDirectories(parentDir, "*", SearchOption.AllDirectories)) + // Craft a ZIP with malicious directory entries using System.IO.Compression + // so we bypass any SharpCompress write-side normalisation. + static void BuildMaliciousZip(string path) { - var relative = Path.GetRelativePath(parentDir, d); - var escaped = !d.StartsWith(extractDir, StringComparison.Ordinal); - Console.WriteLine($" {(escaped ? "[ESCAPED]" : "[ ok ]")} {relative}"); + using var fs = File.Create(path); + using var zip = new SysZip(fs, SysZipMode.Create); + + // 1. Relative traversal: two levels up, then "escaped_relative/" + zip.CreateEntry("../../escaped_relative/"); + + // 2. Absolute Unix path (Path.Combine discards the base when second arg is rooted) + zip.CreateEntry("/tmp/escaped_absolute/"); + + // 3. A legitimate entry for contrast + zip.CreateEntry("safe_subdir/"); } - // Relative traversal "../../escaped_relative/" escapes two levels above extractDir - // (which is parentDir/extract_target), landing in Path.GetTempPath() - var relTarget = Path.GetFullPath(Path.Combine(extractDir, "../../escaped_relative")); - if (Directory.Exists(relTarget)) + static (string extractDir, string parentDir) SetupDirs(string label) { - Console.WriteLine($" [ESCAPED] relative traversal created: {relTarget}"); - Directory.Delete(relTarget); + var parentDir = Path.Combine( + TEST_ARCHIVES_PATH, + $"sc_poc_{label}_{Path.GetRandomFileName()}" + ); + Directory.CreateDirectory(parentDir); + var extractDir = Path.Combine(parentDir, "extract_target"); + + Console.WriteLine($" Parent : {parentDir}"); + Console.WriteLine($" Target : {extractDir}"); + return (extractDir, parentDir); } - var absTarget = "/tmp/escaped_absolute"; - if (Directory.Exists(absTarget)) + static void CheckResults(string archivePath, string parentDir, string extractDir) { - Console.WriteLine($" [ESCAPED] absolute path created: {absTarget}"); - Directory.Delete(absTarget); + Console.WriteLine(" Directories created after extraction:"); + foreach (var d in Directory.GetDirectories(parentDir, "*", SearchOption.AllDirectories)) + { + var relative = Path.GetRelativePath(parentDir, d); + var escaped = !d.StartsWith(extractDir, StringComparison.Ordinal); + Console.WriteLine($" {(escaped ? "[ESCAPED]" : "[ ok ]")} {relative}"); + } + + // Relative traversal "../../escaped_relative/" escapes two levels above extractDir + // (which is parentDir/extract_target), landing in Path.GetTempPath() + var relTarget = Path.GetFullPath(Path.Combine(extractDir, "../../escaped_relative")); + if (Directory.Exists(relTarget)) + { + Console.WriteLine($" [ESCAPED] relative traversal created: {relTarget}"); + Directory.Delete(relTarget); + } + File.Delete(archivePath); + if (Directory.Exists(extractDir)) + { + Directory.Delete(extractDir); + } + + var absTarget = "/tmp/escaped_absolute"; + if (Directory.Exists(absTarget)) + { + Console.WriteLine($" [ESCAPED] absolute path created: {absTarget}"); + Directory.Delete(absTarget); + } } } -} #endif From 8d6f1014f08bf7e1d28510ae92c0c253fabedb73 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 5 May 2026 17:02:28 +0100 Subject: [PATCH 3/7] consolidate path comparsion --- src/SharpCompress/Archives/IArchiveExtensions.cs | 11 +---------- .../Archives/IAsyncArchiveExtensions.cs | 11 +---------- .../Common/ExtractionMethods.Async.cs | 4 ++-- src/SharpCompress/Common/ExtractionMethods.cs | 14 ++------------ src/SharpCompress/Utility.cs | 10 ++++++++++ 5 files changed, 16 insertions(+), 34 deletions(-) diff --git a/src/SharpCompress/Archives/IArchiveExtensions.cs b/src/SharpCompress/Archives/IArchiveExtensions.cs index a1ac2226..17a87962 100644 --- a/src/SharpCompress/Archives/IArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IArchiveExtensions.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.Runtime.InteropServices; using SharpCompress.Common; using SharpCompress.Readers; @@ -9,14 +8,6 @@ namespace SharpCompress.Archives; public static class IArchiveExtensions { - /// - /// 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 => - RuntimeInformation.IsOSPlatform(OSPlatform.Windows) - ? StringComparison.OrdinalIgnoreCase - : StringComparison.Ordinal; extension(IArchive archive) { /// @@ -83,7 +74,7 @@ public static class IArchiveExtensions if (!Directory.Exists(destdir) && seenDirectories.Add(destdir)) { - if (!destdir.StartsWith(fullDestinationDirectoryPath, PathComparison)) + if (!destdir.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) { throw new ExtractionException( "Entry is trying to create a directory outside of the destination directory." diff --git a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs index 51e8f3f2..26facb6b 100644 --- a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using SharpCompress.Common; @@ -11,14 +10,6 @@ namespace SharpCompress.Archives; public static class IAsyncArchiveExtensions { - /// - /// 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 => - RuntimeInformation.IsOSPlatform(OSPlatform.Windows) - ? StringComparison.OrdinalIgnoreCase - : StringComparison.Ordinal; extension(IAsyncArchive archive) { /// @@ -105,7 +96,7 @@ public static class IAsyncArchiveExtensions if (!Directory.Exists(destdir) && seenDirectories.Add(destdir)) { - if (!destdir.StartsWith(fullDestinationDirectoryPath, PathComparison)) + if (!destdir.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) { throw new ExtractionException( "Entry is trying to create a directory outside of the destination directory." diff --git a/src/SharpCompress/Common/ExtractionMethods.Async.cs b/src/SharpCompress/Common/ExtractionMethods.Async.cs index 8792e06b..86347310 100644 --- a/src/SharpCompress/Common/ExtractionMethods.Async.cs +++ b/src/SharpCompress/Common/ExtractionMethods.Async.cs @@ -45,7 +45,7 @@ internal static partial class ExtractionMethods if (!Directory.Exists(destdir)) { - if (!destdir.StartsWith(fullDestinationDirectoryPath, PathComparison)) + if (!destdir.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) { throw new ExtractionException( "Entry is trying to create a directory outside of the destination directory." @@ -65,7 +65,7 @@ internal static partial class ExtractionMethods { destinationFileName = Path.GetFullPath(destinationFileName); - if (!destinationFileName.StartsWith(fullDestinationDirectoryPath, PathComparison)) + if (!destinationFileName.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) { throw new ExtractionException( "Entry is trying to write a file outside of the destination directory." diff --git a/src/SharpCompress/Common/ExtractionMethods.cs b/src/SharpCompress/Common/ExtractionMethods.cs index 526c6e26..bde397d4 100644 --- a/src/SharpCompress/Common/ExtractionMethods.cs +++ b/src/SharpCompress/Common/ExtractionMethods.cs @@ -1,20 +1,10 @@ using System; using System.IO; -using System.Runtime.InteropServices; namespace SharpCompress.Common; internal static partial 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 => - RuntimeInformation.IsOSPlatform(OSPlatform.Windows) - ? StringComparison.OrdinalIgnoreCase - : StringComparison.Ordinal; - /// /// Extract to specific directory, retaining filename /// @@ -55,7 +45,7 @@ internal static partial class ExtractionMethods if (!Directory.Exists(destdir)) { - if (!destdir.StartsWith(fullDestinationDirectoryPath, PathComparison)) + if (!destdir.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) { throw new ExtractionException( "Entry is trying to create a directory outside of the destination directory." @@ -75,7 +65,7 @@ internal static partial class ExtractionMethods { destinationFileName = Path.GetFullPath(destinationFileName); - if (!destinationFileName.StartsWith(fullDestinationDirectoryPath, PathComparison)) + if (!destinationFileName.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) { throw new ExtractionException( "Entry is trying to write a file outside of the destination directory." diff --git a/src/SharpCompress/Utility.cs b/src/SharpCompress/Utility.cs index af8572d7..017bc965 100644 --- a/src/SharpCompress/Utility.cs +++ b/src/SharpCompress/Utility.cs @@ -3,6 +3,7 @@ using System.Buffers; using System.Collections.Generic; using System.Collections.ObjectModel; using System.IO; +using System.Runtime.InteropServices; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -13,6 +14,15 @@ namespace SharpCompress; internal static partial class Utility { + /// + /// 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. + /// + internal static StringComparison PathComparison => + RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + public static bool UseSyncOverAsyncDispose() { var useSyncOverAsync = false; From bd0fa73cce7521874eec88b861f177735d66decc Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 5 May 2026 17:22:53 +0100 Subject: [PATCH 4/7] some refactoring --- .../Archives/IArchiveExtensions.cs | 51 ++-- .../Archives/IAsyncArchiveExtensions.cs | 61 ++--- .../Common/DirectoryManagement.cs | 81 ++++++ .../Common/ExtractionMethods.Async.cs | 92 ++++--- src/SharpCompress/Common/ExtractionMethods.cs | 103 ++++---- .../Security/ExtractionPathTraversalTests.cs | 238 ++++++++++++++++++ 6 files changed, 456 insertions(+), 170 deletions(-) create mode 100644 src/SharpCompress/Common/DirectoryManagement.cs create mode 100644 tests/SharpCompress.Test/Security/ExtractionPathTraversalTests.cs diff --git a/src/SharpCompress/Archives/IArchiveExtensions.cs b/src/SharpCompress/Archives/IArchiveExtensions.cs index 17a87962..0417cdd7 100644 --- a/src/SharpCompress/Archives/IArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IArchiveExtensions.cs @@ -1,6 +1,4 @@ using System; -using System.Collections.Generic; -using System.IO; using SharpCompress.Common; using SharpCompress.Readers; @@ -39,54 +37,33 @@ public static class IArchiveExtensions IProgress? progress ) { - var fullDestinationDirectoryPath = Path.GetFullPath(destinationDirectory); options ??= new ExtractionOptions(); - - //check for trailing slash. - if ( - fullDestinationDirectoryPath[fullDestinationDirectoryPath.Length - 1] - != Path.DirectorySeparatorChar - ) - { - fullDestinationDirectoryPath += Path.DirectorySeparatorChar; - } - - if (!Directory.Exists(fullDestinationDirectoryPath)) - { - throw new ExtractionException( - $"Directory does not exist to extract to: {fullDestinationDirectoryPath}" - ); - } + var fullDestinationDirectoryPath = DirectoryManagement.GetFullDestinationDirectoryPath( + destinationDirectory + ); var totalBytes = archive.TotalUncompressedSize; var bytesRead = 0L; - var seenDirectories = new HashSet(); foreach (var entry in archive.Entries) { if (entry.IsDirectory) { - var folder = Path.GetDirectoryName(entry.Key.NotNull("Entry Key is null")) - .NotNull("Directory is null"); - var destdir = Path.GetFullPath( - Path.Combine(fullDestinationDirectoryPath, folder) + ExtractionMethods.WriteEntryToDirectoryCore( + entry, + fullDestinationDirectoryPath, + options, + _ => { } ); - - if (!Directory.Exists(destdir) && seenDirectories.Add(destdir)) - { - if (!destdir.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) - { - throw new ExtractionException( - "Entry is trying to create a directory outside of the destination directory." - ); - } - - Directory.CreateDirectory(destdir); - } continue; } - entry.WriteToDirectory(destinationDirectory, options); + ExtractionMethods.WriteEntryToDirectoryCore( + entry, + fullDestinationDirectoryPath, + options, + path => entry.WriteToFile(path, options) + ); bytesRead += entry.Size; progress?.Report( diff --git a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs index 26facb6b..3d9db3f2 100644 --- a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs @@ -1,6 +1,4 @@ using System; -using System.Collections.Generic; -using System.IO; using System.Threading; using System.Threading.Tasks; using SharpCompress.Common; @@ -15,7 +13,6 @@ public static class IAsyncArchiveExtensions /// /// Extract to specific directory asynchronously with progress reporting and cancellation support /// - /// The archive to extract. /// The folder to extract into. /// Extraction options. /// Optional progress reporter for tracking extraction progress. @@ -59,28 +56,13 @@ public static class IAsyncArchiveExtensions CancellationToken cancellationToken ) { - var fullDestinationDirectoryPath = Path.GetFullPath(destinationDirectory); options ??= new ExtractionOptions(); - - //check for trailing slash. - if ( - fullDestinationDirectoryPath[fullDestinationDirectoryPath.Length - 1] - != Path.DirectorySeparatorChar - ) - { - fullDestinationDirectoryPath += Path.DirectorySeparatorChar; - } - - if (!Directory.Exists(fullDestinationDirectoryPath)) - { - throw new ExtractionException( - $"Directory does not exist to extract to: {fullDestinationDirectoryPath}" - ); - } + var fullDestinationDirectoryPath = DirectoryManagement.GetFullDestinationDirectoryPath( + destinationDirectory + ); var totalBytes = await archive.TotalUncompressedSizeAsync().ConfigureAwait(false); var bytesRead = 0L; - var seenDirectories = new HashSet(); await foreach (var entry in archive.EntriesAsync.WithCancellation(cancellationToken)) { @@ -88,28 +70,27 @@ public static class IAsyncArchiveExtensions if (entry.IsDirectory) { - var folder = Path.GetDirectoryName(entry.Key.NotNull("Entry Key is null")) - .NotNull("Directory is null"); - var destdir = Path.GetFullPath( - Path.Combine(fullDestinationDirectoryPath, folder) - ); - - if (!Directory.Exists(destdir) && seenDirectories.Add(destdir)) - { - if (!destdir.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) - { - throw new ExtractionException( - "Entry is trying to create a directory outside of the destination directory." - ); - } - - Directory.CreateDirectory(destdir); - } + await ExtractionMethods + .WriteEntryToDirectoryAsyncCore( + entry, + fullDestinationDirectoryPath, + options, + (_, _) => new ValueTask(), + cancellationToken + ) + .ConfigureAwait(false); continue; } - await entry - .WriteToDirectoryAsync(destinationDirectory, options, cancellationToken) + await ExtractionMethods + .WriteEntryToDirectoryAsyncCore( + entry, + fullDestinationDirectoryPath, + options, + async (path, ct) => + await entry.WriteToFileAsync(path, options, ct).ConfigureAwait(false), + cancellationToken + ) .ConfigureAwait(false); bytesRead += entry.Size; diff --git a/src/SharpCompress/Common/DirectoryManagement.cs b/src/SharpCompress/Common/DirectoryManagement.cs new file mode 100644 index 00000000..b4574018 --- /dev/null +++ b/src/SharpCompress/Common/DirectoryManagement.cs @@ -0,0 +1,81 @@ +using System.IO; + +namespace SharpCompress.Common; + +internal static class DirectoryManagement +{ + + internal const string CreateDirectoryOutsideDestinationMessage = + "Entry is trying to create a directory outside of the destination directory."; + internal const string WriteFileOutsideDestinationMessage = + "Entry is trying to write a file outside of the destination directory."; + + internal static string GetFullDestinationDirectoryPath(string destinationDirectory) + { + var fullDestinationDirectoryPath = Path.GetFullPath(destinationDirectory); + + // Keep the trailing separator so prefix checks cannot match sibling directories. + if ( + !IsDirectorySeparator( + fullDestinationDirectoryPath[fullDestinationDirectoryPath.Length - 1] + ) + ) + { + fullDestinationDirectoryPath += Path.DirectorySeparatorChar; + } + + if (!Directory.Exists(fullDestinationDirectoryPath)) + { + throw new ExtractionException( + $"Directory does not exist to extract to: {fullDestinationDirectoryPath}" + ); + } + + return fullDestinationDirectoryPath; + } + + internal static void EnsurePathInDestinationDirectory( + string destinationPath, + string fullDestinationDirectoryPath, + string exceptionMessage + ) + { + if (destinationPath.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) + { + return; + } + + if ( + string.Equals( + destinationPath, + TrimTrailingDirectorySeparators(fullDestinationDirectoryPath), + Utility.PathComparison + ) + ) + { + return; + } + + throw new ExtractionException(exceptionMessage); + } + + + + private static bool IsDirectorySeparator(char value) => + value == Path.DirectorySeparatorChar || value == Path.AltDirectorySeparatorChar; + + private static string TrimTrailingDirectorySeparators(string path) + { + var root = Path.GetPathRoot(path); + var rootLength = root?.Length ?? 0; + var end = path.Length; + + while (end > rootLength && IsDirectorySeparator(path[end - 1])) + { + end--; + } + + return end == path.Length ? path : path.Substring(0, end); + } + +} diff --git a/src/SharpCompress/Common/ExtractionMethods.Async.cs b/src/SharpCompress/Common/ExtractionMethods.Async.cs index 86347310..b9c913cd 100644 --- a/src/SharpCompress/Common/ExtractionMethods.Async.cs +++ b/src/SharpCompress/Common/ExtractionMethods.Async.cs @@ -15,67 +15,59 @@ internal static partial class ExtractionMethods CancellationToken cancellationToken = default ) { - string destinationFileName; - var fullDestinationDirectoryPath = Path.GetFullPath(destinationDirectory); options ??= new ExtractionOptions(); + var fullDestinationDirectoryPath = DirectoryManagement.GetFullDestinationDirectoryPath(destinationDirectory); - //check for trailing slash. - if ( - fullDestinationDirectoryPath[fullDestinationDirectoryPath.Length - 1] - != Path.DirectorySeparatorChar - ) - { - fullDestinationDirectoryPath += Path.DirectorySeparatorChar; - } + await WriteEntryToDirectoryAsyncCore( + entry, + fullDestinationDirectoryPath, + options, + writeAsync, + cancellationToken + ) + .ConfigureAwait(false); + } - if (!Directory.Exists(fullDestinationDirectoryPath)) - { - throw new ExtractionException( - $"Directory does not exist to extract to: {fullDestinationDirectoryPath}" - ); - } - - var file = Path.GetFileName(entry.Key.NotNull("Entry Key is null")).NotNull("File is null"); - file = Utility.ReplaceInvalidFileNameChars(file); - if (options.ExtractFullPath) - { - var folder = Path.GetDirectoryName(entry.Key.NotNull("Entry Key is null")) - .NotNull("Directory is null"); - var destdir = Path.GetFullPath(Path.Combine(fullDestinationDirectoryPath, folder)); - - if (!Directory.Exists(destdir)) - { - if (!destdir.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) - { - throw new ExtractionException( - "Entry is trying to create a directory outside of the destination directory." - ); - } - - Directory.CreateDirectory(destdir); - } - destinationFileName = Path.Combine(destdir, file); - } - else - { - destinationFileName = Path.Combine(fullDestinationDirectoryPath, file); - } + internal static async ValueTask WriteEntryToDirectoryAsyncCore( + IEntry entry, + string fullDestinationDirectoryPath, + ExtractionOptions options, + Func writeAsync, + CancellationToken cancellationToken = default + ) + { + var destinationFileName = GetEntryDestinationFileName( + entry, + fullDestinationDirectoryPath, + options + ); if (!entry.IsDirectory) { destinationFileName = Path.GetFullPath(destinationFileName); - if (!destinationFileName.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) - { - throw new ExtractionException( - "Entry is trying to write a file outside of the destination directory." - ); - } + DirectoryManagement.EnsurePathInDestinationDirectory( + destinationFileName, + fullDestinationDirectoryPath, + DirectoryManagement.WriteFileOutsideDestinationMessage + ); + await writeAsync(destinationFileName, cancellationToken).ConfigureAwait(false); } - else if (options.ExtractFullPath && !Directory.Exists(destinationFileName)) + else if (options.ExtractFullPath) { - Directory.CreateDirectory(destinationFileName); + destinationFileName = Path.GetFullPath(destinationFileName); + + DirectoryManagement.EnsurePathInDestinationDirectory( + destinationFileName, + fullDestinationDirectoryPath, + DirectoryManagement.CreateDirectoryOutsideDestinationMessage + ); + + if (!Directory.Exists(destinationFileName)) + { + Directory.CreateDirectory(destinationFileName); + } } } diff --git a/src/SharpCompress/Common/ExtractionMethods.cs b/src/SharpCompress/Common/ExtractionMethods.cs index bde397d4..66172b36 100644 --- a/src/SharpCompress/Common/ExtractionMethods.cs +++ b/src/SharpCompress/Common/ExtractionMethods.cs @@ -15,70 +15,87 @@ internal static partial class ExtractionMethods Action write ) { - string destinationFileName; - var fullDestinationDirectoryPath = Path.GetFullPath(destinationDirectory); options ??= new ExtractionOptions(); + var fullDestinationDirectoryPath = DirectoryManagement.GetFullDestinationDirectoryPath(destinationDirectory); - //check for trailing slash. - if ( - fullDestinationDirectoryPath[fullDestinationDirectoryPath.Length - 1] - != Path.DirectorySeparatorChar - ) - { - fullDestinationDirectoryPath += Path.DirectorySeparatorChar; - } + WriteEntryToDirectoryCore(entry, fullDestinationDirectoryPath, options, write); + } - if (!Directory.Exists(fullDestinationDirectoryPath)) + internal static void WriteEntryToDirectoryCore( + IEntry entry, + string fullDestinationDirectoryPath, + ExtractionOptions options, + Action write + ) + { + var destinationFileName = GetEntryDestinationFileName( + entry, + fullDestinationDirectoryPath, + options + ); + + if (!entry.IsDirectory) { - throw new ExtractionException( - $"Directory does not exist to extract to: {fullDestinationDirectoryPath}" + destinationFileName = Path.GetFullPath(destinationFileName); + + DirectoryManagement.EnsurePathInDestinationDirectory( + destinationFileName, + fullDestinationDirectoryPath, + DirectoryManagement.WriteFileOutsideDestinationMessage ); - } + write(destinationFileName); + } + else if (options.ExtractFullPath) + { + destinationFileName = Path.GetFullPath(destinationFileName); + + DirectoryManagement.EnsurePathInDestinationDirectory( + destinationFileName, + fullDestinationDirectoryPath, + DirectoryManagement.CreateDirectoryOutsideDestinationMessage + ); + + if (!Directory.Exists(destinationFileName)) + { + Directory.CreateDirectory(destinationFileName); + } + } + } + + private static string GetEntryDestinationFileName( + IEntry entry, + string fullDestinationDirectoryPath, + ExtractionOptions options + ) + { var file = Path.GetFileName(entry.Key.NotNull("Entry Key is null")).NotNull("File is null"); file = Utility.ReplaceInvalidFileNameChars(file); + if (options.ExtractFullPath) { var folder = Path.GetDirectoryName(entry.Key.NotNull("Entry Key is null")) .NotNull("Directory is null"); var destdir = Path.GetFullPath(Path.Combine(fullDestinationDirectoryPath, folder)); + DirectoryManagement.EnsurePathInDestinationDirectory( + destdir, + fullDestinationDirectoryPath, + entry.IsDirectory + ? DirectoryManagement.CreateDirectoryOutsideDestinationMessage + : DirectoryManagement.WriteFileOutsideDestinationMessage + ); + if (!Directory.Exists(destdir)) { - if (!destdir.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) - { - throw new ExtractionException( - "Entry is trying to create a directory outside of the destination directory." - ); - } - Directory.CreateDirectory(destdir); } - destinationFileName = Path.Combine(destdir, file); - } - else - { - destinationFileName = Path.Combine(fullDestinationDirectoryPath, file); + + return Path.Combine(destdir, file); } - if (!entry.IsDirectory) - { - destinationFileName = Path.GetFullPath(destinationFileName); - - if (!destinationFileName.StartsWith(fullDestinationDirectoryPath, Utility.PathComparison)) - { - throw new ExtractionException( - "Entry is trying to write a file outside of the destination directory." - ); - } - write(destinationFileName); - } - else if (options.ExtractFullPath && !Directory.Exists(destinationFileName)) - { - Directory.CreateDirectory(destinationFileName); - } + return Path.Combine(fullDestinationDirectoryPath, file); } - public static void WriteEntryToFile( IEntry entry, string destinationFileName, diff --git a/tests/SharpCompress.Test/Security/ExtractionPathTraversalTests.cs b/tests/SharpCompress.Test/Security/ExtractionPathTraversalTests.cs new file mode 100644 index 00000000..69ebddd8 --- /dev/null +++ b/tests/SharpCompress.Test/Security/ExtractionPathTraversalTests.cs @@ -0,0 +1,238 @@ +#if NET8_0_OR_GREATER +using System; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Common; +using SharpCompress.Readers; +using Xunit; +using SysZip = System.IO.Compression.ZipArchive; +using SysZipMode = System.IO.Compression.ZipArchiveMode; + +namespace SharpCompress.Test.Security; + +public class ExtractionPathTraversalTests : TestBase +{ + [Theory] + [InlineData("ReaderAll")] + [InlineData("ReaderEntry")] + [InlineData("Archive")] + [InlineData("ArchiveEntry")] + [InlineData("AsyncReaderAll")] + [InlineData("AsyncReaderEntry")] + [InlineData("AsyncArchive")] + [InlineData("AsyncArchiveEntry")] + public async Task DirectoryTraversalToExistingOutsideDirectory_ShouldThrow(string api) + { + var extractDir = Path.Combine(SCRATCH_FILES_PATH, "extract"); + Directory.CreateDirectory(extractDir); + var escapedDirectory = Path.GetFullPath(Path.Combine(extractDir, "../../escaped_existing")); + Directory.CreateDirectory(escapedDirectory); + var archivePath = Path.Combine(SCRATCH2_FILES_PATH, $"{api}.zip"); + BuildZip(archivePath, "../../escaped_existing/"); + + var exception = await RecordExtractionExceptionAsync(api, archivePath, extractDir); + + var extractionException = Assert.IsType(exception); + Assert.Contains("outside of the destination", extractionException.Message); + } + + [Theory] + [InlineData("ReaderAll")] + [InlineData("ReaderEntry")] + [InlineData("Archive")] + [InlineData("ArchiveEntry")] + [InlineData("AsyncReaderAll")] + [InlineData("AsyncReaderEntry")] + [InlineData("AsyncArchive")] + [InlineData("AsyncArchiveEntry")] + public async Task FileTraversalToSiblingDirectory_ShouldThrow(string api) + { + var extractDir = Path.Combine(SCRATCH_FILES_PATH, "extract"); + Directory.CreateDirectory(extractDir); + var siblingDirectory = Path.Combine(SCRATCH_FILES_PATH, "extract2"); + Directory.CreateDirectory(siblingDirectory); + var archivePath = Path.Combine(SCRATCH2_FILES_PATH, $"{api}.zip"); + BuildZip(archivePath, "../extract2/evil.txt"); + + var exception = await RecordExtractionExceptionAsync(api, archivePath, extractDir); + + var extractionException = Assert.IsType(exception); + Assert.Contains("outside of the destination", extractionException.Message); + Assert.False(File.Exists(Path.Combine(siblingDirectory, "evil.txt"))); + } + + private static void BuildZip(string path, string entryName) + { + using var fs = File.Create(path); + using var zip = new SysZip(fs, SysZipMode.Create); + var entry = zip.CreateEntry(entryName); + + if (entryName.EndsWith('/')) + { + return; + } + + using var writer = new StreamWriter(entry.Open()); + writer.Write("evil"); + } + + private static async Task RecordExtractionExceptionAsync( + string api, + string archivePath, + string extractDir + ) + { + var options = new ExtractionOptions { ExtractFullPath = true, Overwrite = true }; + + return api switch + { + "ReaderAll" => RecordException(() => + ExtractWithReaderAll(archivePath, extractDir, options) + ), + "ReaderEntry" => RecordException(() => + ExtractWithReaderEntry(archivePath, extractDir, options) + ), + "Archive" => RecordException(() => + ExtractWithArchive(archivePath, extractDir, options) + ), + "ArchiveEntry" => RecordException(() => + ExtractWithArchiveEntry(archivePath, extractDir, options) + ), + "AsyncReaderAll" => await RecordExceptionAsync(() => + ExtractWithAsyncReaderAllAsync(archivePath, extractDir, options) + ), + "AsyncReaderEntry" => await RecordExceptionAsync(() => + ExtractWithAsyncReaderEntryAsync(archivePath, extractDir, options) + ), + "AsyncArchive" => await RecordExceptionAsync(() => + ExtractWithAsyncArchiveAsync(archivePath, extractDir, options) + ), + "AsyncArchiveEntry" => await RecordExceptionAsync(() => + ExtractWithAsyncArchiveEntryAsync(archivePath, extractDir, options) + ), + _ => throw new ArgumentOutOfRangeException(nameof(api), api, null), + }; + } + + private static Exception? RecordException(Action action) + { + try + { + action(); + return null; + } + catch (Exception exception) + { + return exception; + } + } + + private static async Task RecordExceptionAsync(Func action) + { + try + { + await action(); + return null; + } + catch (Exception exception) + { + return exception; + } + } + + private static void ExtractWithReaderAll( + string archivePath, + string extractDir, + ExtractionOptions options + ) + { + using var stream = File.OpenRead(archivePath); + using var reader = ReaderFactory.OpenReader(stream); + reader.WriteAllToDirectory(extractDir, options); + } + + private static void ExtractWithReaderEntry( + string archivePath, + string extractDir, + ExtractionOptions options + ) + { + using var stream = File.OpenRead(archivePath); + using var reader = ReaderFactory.OpenReader(stream); + Assert.True(reader.MoveToNextEntry()); + reader.WriteEntryToDirectory(extractDir, options); + } + + private static void ExtractWithArchive( + string archivePath, + string extractDir, + ExtractionOptions options + ) + { + using var archive = ArchiveFactory.OpenArchive(archivePath); + archive.WriteToDirectory(extractDir, options); + } + + private static void ExtractWithArchiveEntry( + string archivePath, + string extractDir, + ExtractionOptions options + ) + { + using var archive = ArchiveFactory.OpenArchive(archivePath); + archive.Entries.Single().WriteToDirectory(extractDir, options); + } + + private static async Task ExtractWithAsyncReaderAllAsync( + string archivePath, + string extractDir, + ExtractionOptions options + ) + { + using var stream = File.OpenRead(archivePath); + await using var reader = await ReaderFactory.OpenAsyncReader(stream); + await reader.WriteAllToDirectoryAsync(extractDir, options); + } + + private static async Task ExtractWithAsyncReaderEntryAsync( + string archivePath, + string extractDir, + ExtractionOptions options + ) + { + using var stream = File.OpenRead(archivePath); + await using var reader = await ReaderFactory.OpenAsyncReader(stream); + Assert.True(await reader.MoveToNextEntryAsync()); + await reader.WriteEntryToDirectoryAsync(extractDir, options); + } + + private static async Task ExtractWithAsyncArchiveAsync( + string archivePath, + string extractDir, + ExtractionOptions options + ) + { + await using var archive = await ArchiveFactory.OpenAsyncArchive(archivePath); + await archive.WriteToDirectoryAsync(extractDir, options); + } + + private static async Task ExtractWithAsyncArchiveEntryAsync( + string archivePath, + string extractDir, + ExtractionOptions options + ) + { + await using var archive = await ArchiveFactory.OpenAsyncArchive(archivePath); + + await foreach (var entry in archive.EntriesAsync) + { + await entry.WriteToDirectoryAsync(extractDir, options); + return; + } + + throw new InvalidOperationException("Archive did not contain an entry."); + } +} +#endif From 2c35d2d242ff4cc8b8c80353ac09c5bfa58d9dd2 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 5 May 2026 17:30:02 +0100 Subject: [PATCH 5/7] made methods extension methods for IEntry --- .../Archives/IArchiveEntryExtensions.cs | 12 +- .../Archives/IArchiveExtensions.cs | 6 +- .../Archives/IAsyncArchiveExtensions.cs | 6 +- .../Common/DirectoryManagement.cs | 4 - .../Common/ExtractionMethods.Async.cs | 109 -------------- src/SharpCompress/Common/ExtractionMethods.cs | 132 ----------------- .../Common/IEntryExtensions.Async.cs | 114 ++++++++++++++ src/SharpCompress/Common/IEntryExtensions.cs | 139 ++++++++++++++++++ .../Readers/IAsyncReaderExtensions.cs | 15 +- .../Readers/IReaderExtensions.cs | 6 +- 10 files changed, 269 insertions(+), 274 deletions(-) delete mode 100644 src/SharpCompress/Common/ExtractionMethods.Async.cs delete mode 100644 src/SharpCompress/Common/ExtractionMethods.cs create mode 100644 src/SharpCompress/Common/IEntryExtensions.Async.cs create mode 100644 src/SharpCompress/Common/IEntryExtensions.cs diff --git a/src/SharpCompress/Archives/IArchiveEntryExtensions.cs b/src/SharpCompress/Archives/IArchiveEntryExtensions.cs index f60105a5..f89ba345 100644 --- a/src/SharpCompress/Archives/IArchiveEntryExtensions.cs +++ b/src/SharpCompress/Archives/IArchiveEntryExtensions.cs @@ -106,8 +106,7 @@ public static class IArchiveEntryExtensions string destinationDirectory, ExtractionOptions? options = null ) => - ExtractionMethods.WriteEntryToDirectory( - entry, + entry.WriteEntryToDirectory( destinationDirectory, options, (path) => entry.WriteToFile(path, options) @@ -121,9 +120,8 @@ public static class IArchiveEntryExtensions ExtractionOptions? options = null, CancellationToken cancellationToken = default ) => - await ExtractionMethods + await entry .WriteEntryToDirectoryAsync( - entry, destinationDirectory, options, async (path, ct) => @@ -136,8 +134,7 @@ public static class IArchiveEntryExtensions /// Extract to specific file /// public void WriteToFile(string destinationFileName, ExtractionOptions? options = null) => - ExtractionMethods.WriteEntryToFile( - entry, + entry.WriteEntryToFile( destinationFileName, options, (x, fm) => @@ -155,9 +152,8 @@ public static class IArchiveEntryExtensions ExtractionOptions? options = null, CancellationToken cancellationToken = default ) => - await ExtractionMethods + await entry .WriteEntryToFileAsync( - entry, destinationFileName, options, async (x, fm, ct) => diff --git a/src/SharpCompress/Archives/IArchiveExtensions.cs b/src/SharpCompress/Archives/IArchiveExtensions.cs index 0417cdd7..2ae6e4ab 100644 --- a/src/SharpCompress/Archives/IArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IArchiveExtensions.cs @@ -49,8 +49,7 @@ public static class IArchiveExtensions { if (entry.IsDirectory) { - ExtractionMethods.WriteEntryToDirectoryCore( - entry, + entry.WriteEntryToDirectoryCore( fullDestinationDirectoryPath, options, _ => { } @@ -58,8 +57,7 @@ public static class IArchiveExtensions continue; } - ExtractionMethods.WriteEntryToDirectoryCore( - entry, + entry.WriteEntryToDirectoryCore( fullDestinationDirectoryPath, options, path => entry.WriteToFile(path, options) diff --git a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs index 3d9db3f2..a3e9948e 100644 --- a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs @@ -70,9 +70,8 @@ public static class IAsyncArchiveExtensions if (entry.IsDirectory) { - await ExtractionMethods + await entry .WriteEntryToDirectoryAsyncCore( - entry, fullDestinationDirectoryPath, options, (_, _) => new ValueTask(), @@ -82,9 +81,8 @@ public static class IAsyncArchiveExtensions continue; } - await ExtractionMethods + await entry .WriteEntryToDirectoryAsyncCore( - entry, fullDestinationDirectoryPath, options, async (path, ct) => diff --git a/src/SharpCompress/Common/DirectoryManagement.cs b/src/SharpCompress/Common/DirectoryManagement.cs index b4574018..4bd4585d 100644 --- a/src/SharpCompress/Common/DirectoryManagement.cs +++ b/src/SharpCompress/Common/DirectoryManagement.cs @@ -4,7 +4,6 @@ namespace SharpCompress.Common; internal static class DirectoryManagement { - internal const string CreateDirectoryOutsideDestinationMessage = "Entry is trying to create a directory outside of the destination directory."; internal const string WriteFileOutsideDestinationMessage = @@ -59,8 +58,6 @@ internal static class DirectoryManagement throw new ExtractionException(exceptionMessage); } - - private static bool IsDirectorySeparator(char value) => value == Path.DirectorySeparatorChar || value == Path.AltDirectorySeparatorChar; @@ -77,5 +74,4 @@ internal static class DirectoryManagement return end == path.Length ? path : path.Substring(0, end); } - } diff --git a/src/SharpCompress/Common/ExtractionMethods.Async.cs b/src/SharpCompress/Common/ExtractionMethods.Async.cs deleted file mode 100644 index b9c913cd..00000000 --- a/src/SharpCompress/Common/ExtractionMethods.Async.cs +++ /dev/null @@ -1,109 +0,0 @@ -using System; -using System.IO; -using System.Threading; -using System.Threading.Tasks; - -namespace SharpCompress.Common; - -internal static partial class ExtractionMethods -{ - public static async ValueTask WriteEntryToDirectoryAsync( - IEntry entry, - string destinationDirectory, - ExtractionOptions? options, - Func writeAsync, - CancellationToken cancellationToken = default - ) - { - options ??= new ExtractionOptions(); - var fullDestinationDirectoryPath = DirectoryManagement.GetFullDestinationDirectoryPath(destinationDirectory); - - await WriteEntryToDirectoryAsyncCore( - entry, - fullDestinationDirectoryPath, - options, - writeAsync, - cancellationToken - ) - .ConfigureAwait(false); - } - - internal static async ValueTask WriteEntryToDirectoryAsyncCore( - IEntry entry, - string fullDestinationDirectoryPath, - ExtractionOptions options, - Func writeAsync, - CancellationToken cancellationToken = default - ) - { - var destinationFileName = GetEntryDestinationFileName( - entry, - fullDestinationDirectoryPath, - options - ); - - if (!entry.IsDirectory) - { - destinationFileName = Path.GetFullPath(destinationFileName); - - DirectoryManagement.EnsurePathInDestinationDirectory( - destinationFileName, - fullDestinationDirectoryPath, - DirectoryManagement.WriteFileOutsideDestinationMessage - ); - - await writeAsync(destinationFileName, cancellationToken).ConfigureAwait(false); - } - else if (options.ExtractFullPath) - { - destinationFileName = Path.GetFullPath(destinationFileName); - - DirectoryManagement.EnsurePathInDestinationDirectory( - destinationFileName, - fullDestinationDirectoryPath, - DirectoryManagement.CreateDirectoryOutsideDestinationMessage - ); - - if (!Directory.Exists(destinationFileName)) - { - Directory.CreateDirectory(destinationFileName); - } - } - } - - public static async ValueTask WriteEntryToFileAsync( - IEntry entry, - string destinationFileName, - ExtractionOptions? options, - Func openAndWriteAsync, - CancellationToken cancellationToken = default - ) - { - options ??= new ExtractionOptions(); - if (entry.LinkTarget != null) - { - if (options.SymbolicLinkHandler is not null) - { - options.SymbolicLinkHandler(destinationFileName, entry.LinkTarget); - } - else - { - ExtractionOptions.DefaultSymbolicLinkHandler(destinationFileName, entry.LinkTarget); - } - return; - } - else - { - var fm = FileMode.Create; - - if (!options.Overwrite) - { - fm = FileMode.CreateNew; - } - - await openAndWriteAsync(destinationFileName, fm, cancellationToken) - .ConfigureAwait(false); - entry.PreserveExtractionOptions(destinationFileName, options); - } - } -} diff --git a/src/SharpCompress/Common/ExtractionMethods.cs b/src/SharpCompress/Common/ExtractionMethods.cs deleted file mode 100644 index 66172b36..00000000 --- a/src/SharpCompress/Common/ExtractionMethods.cs +++ /dev/null @@ -1,132 +0,0 @@ -using System; -using System.IO; - -namespace SharpCompress.Common; - -internal static partial class ExtractionMethods -{ - /// - /// Extract to specific directory, retaining filename - /// - public static void WriteEntryToDirectory( - IEntry entry, - string destinationDirectory, - ExtractionOptions? options, - Action write - ) - { - options ??= new ExtractionOptions(); - var fullDestinationDirectoryPath = DirectoryManagement.GetFullDestinationDirectoryPath(destinationDirectory); - - WriteEntryToDirectoryCore(entry, fullDestinationDirectoryPath, options, write); - } - - internal static void WriteEntryToDirectoryCore( - IEntry entry, - string fullDestinationDirectoryPath, - ExtractionOptions options, - Action write - ) - { - var destinationFileName = GetEntryDestinationFileName( - entry, - fullDestinationDirectoryPath, - options - ); - - if (!entry.IsDirectory) - { - destinationFileName = Path.GetFullPath(destinationFileName); - - DirectoryManagement.EnsurePathInDestinationDirectory( - destinationFileName, - fullDestinationDirectoryPath, - DirectoryManagement.WriteFileOutsideDestinationMessage - ); - - write(destinationFileName); - } - else if (options.ExtractFullPath) - { - destinationFileName = Path.GetFullPath(destinationFileName); - - DirectoryManagement.EnsurePathInDestinationDirectory( - destinationFileName, - fullDestinationDirectoryPath, - DirectoryManagement.CreateDirectoryOutsideDestinationMessage - ); - - if (!Directory.Exists(destinationFileName)) - { - Directory.CreateDirectory(destinationFileName); - } - } - } - - private static string GetEntryDestinationFileName( - IEntry entry, - string fullDestinationDirectoryPath, - ExtractionOptions options - ) - { - var file = Path.GetFileName(entry.Key.NotNull("Entry Key is null")).NotNull("File is null"); - file = Utility.ReplaceInvalidFileNameChars(file); - - if (options.ExtractFullPath) - { - var folder = Path.GetDirectoryName(entry.Key.NotNull("Entry Key is null")) - .NotNull("Directory is null"); - var destdir = Path.GetFullPath(Path.Combine(fullDestinationDirectoryPath, folder)); - - DirectoryManagement.EnsurePathInDestinationDirectory( - destdir, - fullDestinationDirectoryPath, - entry.IsDirectory - ? DirectoryManagement.CreateDirectoryOutsideDestinationMessage - : DirectoryManagement.WriteFileOutsideDestinationMessage - ); - - if (!Directory.Exists(destdir)) - { - Directory.CreateDirectory(destdir); - } - - return Path.Combine(destdir, file); - } - - return Path.Combine(fullDestinationDirectoryPath, file); - } - public static void WriteEntryToFile( - IEntry entry, - string destinationFileName, - ExtractionOptions? options, - Action openAndWrite - ) - { - options ??= new ExtractionOptions(); - if (entry.LinkTarget != null) - { - if (options.SymbolicLinkHandler is not null) - { - options.SymbolicLinkHandler(destinationFileName, entry.LinkTarget); - } - else - { - ExtractionOptions.DefaultSymbolicLinkHandler(destinationFileName, entry.LinkTarget); - } - return; - } - else - { - var fm = FileMode.Create; - - if (!options.Overwrite) - { - fm = FileMode.CreateNew; - } - - openAndWrite(destinationFileName, fm); - entry.PreserveExtractionOptions(destinationFileName, options); - } - } -} diff --git a/src/SharpCompress/Common/IEntryExtensions.Async.cs b/src/SharpCompress/Common/IEntryExtensions.Async.cs new file mode 100644 index 00000000..234a1d3c --- /dev/null +++ b/src/SharpCompress/Common/IEntryExtensions.Async.cs @@ -0,0 +1,114 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +namespace SharpCompress.Common; + +internal static partial class IEntryExtensions +{ + extension(IEntry entry) + { + public async ValueTask WriteEntryToDirectoryAsync( + string destinationDirectory, + ExtractionOptions? options, + Func writeAsync, + CancellationToken cancellationToken = default + ) + { + options ??= new ExtractionOptions(); + var fullDestinationDirectoryPath = DirectoryManagement.GetFullDestinationDirectoryPath( + destinationDirectory + ); + + await WriteEntryToDirectoryAsyncCore( + entry, + fullDestinationDirectoryPath, + options, + writeAsync, + cancellationToken + ) + .ConfigureAwait(false); + } + + internal async ValueTask WriteEntryToDirectoryAsyncCore( + string fullDestinationDirectoryPath, + ExtractionOptions options, + Func writeAsync, + CancellationToken cancellationToken = default + ) + { + var destinationFileName = GetEntryDestinationFileName( + entry, + fullDestinationDirectoryPath, + options + ); + + if (!entry.IsDirectory) + { + destinationFileName = Path.GetFullPath(destinationFileName); + + DirectoryManagement.EnsurePathInDestinationDirectory( + destinationFileName, + fullDestinationDirectoryPath, + DirectoryManagement.WriteFileOutsideDestinationMessage + ); + + await writeAsync(destinationFileName, cancellationToken).ConfigureAwait(false); + } + else if (options.ExtractFullPath) + { + destinationFileName = Path.GetFullPath(destinationFileName); + + DirectoryManagement.EnsurePathInDestinationDirectory( + destinationFileName, + fullDestinationDirectoryPath, + DirectoryManagement.CreateDirectoryOutsideDestinationMessage + ); + + if (!Directory.Exists(destinationFileName)) + { + Directory.CreateDirectory(destinationFileName); + } + } + } + + public async ValueTask WriteEntryToFileAsync( + string destinationFileName, + ExtractionOptions? options, + Func openAndWriteAsync, + CancellationToken cancellationToken = default + ) + { + options ??= new ExtractionOptions(); + if (entry.LinkTarget != null) + { + if (options.SymbolicLinkHandler is not null) + { + options.SymbolicLinkHandler(destinationFileName, entry.LinkTarget); + } + else + { + ExtractionOptions.DefaultSymbolicLinkHandler( + destinationFileName, + entry.LinkTarget + ); + } + return; + } + else + { + var fm = FileMode.Create; + + if (!options.Overwrite) + { + fm = FileMode.CreateNew; + } + + await openAndWriteAsync(destinationFileName, fm, cancellationToken) + .ConfigureAwait(false); + entry.PreserveExtractionOptions(destinationFileName, options); + } + } + } +} diff --git a/src/SharpCompress/Common/IEntryExtensions.cs b/src/SharpCompress/Common/IEntryExtensions.cs new file mode 100644 index 00000000..a0b0d5bc --- /dev/null +++ b/src/SharpCompress/Common/IEntryExtensions.cs @@ -0,0 +1,139 @@ +using System; +using System.IO; + +namespace SharpCompress.Common; + +internal static partial class IEntryExtensions +{ + extension(IEntry entry) + { + /// + /// Extract to specific directory, retaining filename + /// + public void WriteEntryToDirectory( + string destinationDirectory, + ExtractionOptions? options, + Action write + ) + { + options ??= new ExtractionOptions(); + var fullDestinationDirectoryPath = DirectoryManagement.GetFullDestinationDirectoryPath( + destinationDirectory + ); + + WriteEntryToDirectoryCore(entry, fullDestinationDirectoryPath, options, write); + } + + internal void WriteEntryToDirectoryCore( + string fullDestinationDirectoryPath, + ExtractionOptions options, + Action write + ) + { + var destinationFileName = GetEntryDestinationFileName( + entry, + fullDestinationDirectoryPath, + options + ); + + if (!entry.IsDirectory) + { + destinationFileName = Path.GetFullPath(destinationFileName); + + DirectoryManagement.EnsurePathInDestinationDirectory( + destinationFileName, + fullDestinationDirectoryPath, + DirectoryManagement.WriteFileOutsideDestinationMessage + ); + + write(destinationFileName); + } + else if (options.ExtractFullPath) + { + destinationFileName = Path.GetFullPath(destinationFileName); + + DirectoryManagement.EnsurePathInDestinationDirectory( + destinationFileName, + fullDestinationDirectoryPath, + DirectoryManagement.CreateDirectoryOutsideDestinationMessage + ); + + if (!Directory.Exists(destinationFileName)) + { + Directory.CreateDirectory(destinationFileName); + } + } + } + + private string GetEntryDestinationFileName( + string fullDestinationDirectoryPath, + ExtractionOptions options + ) + { + var file = Path.GetFileName(entry.Key.NotNull("Entry Key is null")) + .NotNull("File is null"); + file = Utility.ReplaceInvalidFileNameChars(file); + + if (options.ExtractFullPath) + { + var folder = Path.GetDirectoryName(entry.Key.NotNull("Entry Key is null")) + .NotNull("Directory is null"); + var destdir = Path.GetFullPath(Path.Combine(fullDestinationDirectoryPath, folder)); + + DirectoryManagement.EnsurePathInDestinationDirectory( + destdir, + fullDestinationDirectoryPath, + entry.IsDirectory + ? DirectoryManagement.CreateDirectoryOutsideDestinationMessage + : DirectoryManagement.WriteFileOutsideDestinationMessage + ); + + if (!Directory.Exists(destdir)) + { + Directory.CreateDirectory(destdir); + } + + return Path.Combine(destdir, file); + } + + return Path.Combine(fullDestinationDirectoryPath, file); + } + + public void WriteEntryToFile( + string destinationFileName, + ExtractionOptions? options, + Action openAndWrite + ) + { + options ??= new ExtractionOptions(); + if (entry.LinkTarget != null) + { + if (options.SymbolicLinkHandler is not null) + { + options.SymbolicLinkHandler(destinationFileName, entry.LinkTarget); + } + else + { + ExtractionOptions.DefaultSymbolicLinkHandler( + destinationFileName, + entry.LinkTarget + ); + } + + return; + } + else + { + var fm = FileMode.Create; + + if (!options.Overwrite) + { + fm = FileMode.CreateNew; + } + + openAndWrite(destinationFileName, fm); + entry.PreserveExtractionOptions(destinationFileName, options); + } + } + } +} diff --git a/src/SharpCompress/Readers/IAsyncReaderExtensions.cs b/src/SharpCompress/Readers/IAsyncReaderExtensions.cs index 120e2f69..a2c01e52 100644 --- a/src/SharpCompress/Readers/IAsyncReaderExtensions.cs +++ b/src/SharpCompress/Readers/IAsyncReaderExtensions.cs @@ -17,9 +17,8 @@ public static class IAsyncReaderExtensions ExtractionOptions? options = null, CancellationToken cancellationToken = default ) => - await ExtractionMethods - .WriteEntryToDirectoryAsync( - reader.Entry, + await reader + .Entry.WriteEntryToDirectoryAsync( destinationDirectory, options, async (path, ct) => @@ -36,9 +35,8 @@ public static class IAsyncReaderExtensions ExtractionOptions? options = null, CancellationToken cancellationToken = default ) => - await ExtractionMethods - .WriteEntryToFileAsync( - reader.Entry, + await reader + .Entry.WriteEntryToFileAsync( destinationFileName, options, async (x, fm, ct) => @@ -72,9 +70,8 @@ public static class IAsyncReaderExtensions ExtractionOptions? options = null, CancellationToken cancellationToken = default ) => - await ExtractionMethods - .WriteEntryToFileAsync( - reader.Entry, + await reader + .Entry.WriteEntryToFileAsync( destinationFileName, options, async (x, fm, ct) => diff --git a/src/SharpCompress/Readers/IReaderExtensions.cs b/src/SharpCompress/Readers/IReaderExtensions.cs index 54dfa14f..74c708a5 100644 --- a/src/SharpCompress/Readers/IReaderExtensions.cs +++ b/src/SharpCompress/Readers/IReaderExtensions.cs @@ -40,8 +40,7 @@ public static class IReaderExtensions string destinationDirectory, ExtractionOptions? options = null ) => - ExtractionMethods.WriteEntryToDirectory( - reader.Entry, + reader.Entry.WriteEntryToDirectory( destinationDirectory, options, (path) => reader.WriteEntryToFile(path, options) @@ -54,8 +53,7 @@ public static class IReaderExtensions string destinationFileName, ExtractionOptions? options = null ) => - ExtractionMethods.WriteEntryToFile( - reader.Entry, + reader.Entry.WriteEntryToFile( destinationFileName, options, (x, fm) => From 5f16216a3f38308bb3dc3d450ec9269e1842307a Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 5 May 2026 17:48:39 +0100 Subject: [PATCH 6/7] review and test fixes --- .../Archives/IArchiveExtensions.cs | 6 +---- .../Archives/IAsyncArchiveExtensions.cs | 2 +- .../Common/IEntryExtensions.Async.cs | 9 ++++--- src/SharpCompress/Common/IEntryExtensions.cs | 5 ++-- tests/SharpCompress.Test/Security/ZipSlip.cs | 24 ++++++++++++------- 5 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/SharpCompress/Archives/IArchiveExtensions.cs b/src/SharpCompress/Archives/IArchiveExtensions.cs index 2ae6e4ab..d99a7afd 100644 --- a/src/SharpCompress/Archives/IArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IArchiveExtensions.cs @@ -49,11 +49,7 @@ public static class IArchiveExtensions { if (entry.IsDirectory) { - entry.WriteEntryToDirectoryCore( - fullDestinationDirectoryPath, - options, - _ => { } - ); + entry.WriteEntryToDirectoryCore(fullDestinationDirectoryPath, options, null); continue; } diff --git a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs index a3e9948e..21d2730c 100644 --- a/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs +++ b/src/SharpCompress/Archives/IAsyncArchiveExtensions.cs @@ -74,7 +74,7 @@ public static class IAsyncArchiveExtensions .WriteEntryToDirectoryAsyncCore( fullDestinationDirectoryPath, options, - (_, _) => new ValueTask(), + null, cancellationToken ) .ConfigureAwait(false); diff --git a/src/SharpCompress/Common/IEntryExtensions.Async.cs b/src/SharpCompress/Common/IEntryExtensions.Async.cs index 234a1d3c..c1bc0dea 100644 --- a/src/SharpCompress/Common/IEntryExtensions.Async.cs +++ b/src/SharpCompress/Common/IEntryExtensions.Async.cs @@ -34,7 +34,7 @@ internal static partial class IEntryExtensions internal async ValueTask WriteEntryToDirectoryAsyncCore( string fullDestinationDirectoryPath, ExtractionOptions options, - Func writeAsync, + Func? writeAsync, CancellationToken cancellationToken = default ) { @@ -53,8 +53,6 @@ internal static partial class IEntryExtensions fullDestinationDirectoryPath, DirectoryManagement.WriteFileOutsideDestinationMessage ); - - await writeAsync(destinationFileName, cancellationToken).ConfigureAwait(false); } else if (options.ExtractFullPath) { @@ -71,6 +69,11 @@ internal static partial class IEntryExtensions Directory.CreateDirectory(destinationFileName); } } + + if (writeAsync != null) + { + await writeAsync(destinationFileName, cancellationToken).ConfigureAwait(false); + } } public async ValueTask WriteEntryToFileAsync( diff --git a/src/SharpCompress/Common/IEntryExtensions.cs b/src/SharpCompress/Common/IEntryExtensions.cs index a0b0d5bc..094d997b 100644 --- a/src/SharpCompress/Common/IEntryExtensions.cs +++ b/src/SharpCompress/Common/IEntryExtensions.cs @@ -27,7 +27,7 @@ internal static partial class IEntryExtensions internal void WriteEntryToDirectoryCore( string fullDestinationDirectoryPath, ExtractionOptions options, - Action write + Action? write ) { var destinationFileName = GetEntryDestinationFileName( @@ -45,8 +45,6 @@ internal static partial class IEntryExtensions fullDestinationDirectoryPath, DirectoryManagement.WriteFileOutsideDestinationMessage ); - - write(destinationFileName); } else if (options.ExtractFullPath) { @@ -63,6 +61,7 @@ internal static partial class IEntryExtensions Directory.CreateDirectory(destinationFileName); } } + write?.Invoke(destinationFileName); } private string GetEntryDestinationFileName( diff --git a/tests/SharpCompress.Test/Security/ZipSlip.cs b/tests/SharpCompress.Test/Security/ZipSlip.cs index 251b48d4..208c3192 100644 --- a/tests/SharpCompress.Test/Security/ZipSlip.cs +++ b/tests/SharpCompress.Test/Security/ZipSlip.cs @@ -23,13 +23,19 @@ public class ZipSlip : TestBase BuildMaliciousZip(archivePath); - using var archive = ArchiveFactory.OpenArchive(archivePath); - - var ex = Assert.Throws(() => - archive.WriteToDirectory(extractDir, new ExtractionOptions { ExtractFullPath = true }) - ); - ex.Message.Should() - .Contain("Entry is trying to create a directory outside of the destination directory"); + using (var archive = ArchiveFactory.OpenArchive(archivePath)) + { + var ex = Assert.Throws(() => + archive.WriteToDirectory( + extractDir, + new ExtractionOptions { ExtractFullPath = true } + ) + ); + ex.Message.Should() + .Contain( + "Entry is trying to create a directory outside of the destination directory" + ); + } CheckResults(archivePath, parentDir, extractDir); } @@ -79,10 +85,10 @@ public class ZipSlip : TestBase zip.CreateEntry("safe_subdir/"); } - static (string extractDir, string parentDir) SetupDirs(string label) + private (string extractDir, string parentDir) SetupDirs(string label) { var parentDir = Path.Combine( - TEST_ARCHIVES_PATH, + SCRATCH_FILES_PATH, $"sc_poc_{label}_{Path.GetRandomFileName()}" ); Directory.CreateDirectory(parentDir); From 7de1f07597115586b37b3ac6bafd5add018cff27 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Tue, 5 May 2026 17:58:36 +0100 Subject: [PATCH 7/7] revert change that made no sense --- src/SharpCompress/Common/IEntryExtensions.Async.cs | 10 +++++----- src/SharpCompress/Common/IEntryExtensions.cs | 2 +- src/SharpCompress/Readers/IAsyncReaderExtensions.cs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/SharpCompress/Common/IEntryExtensions.Async.cs b/src/SharpCompress/Common/IEntryExtensions.Async.cs index c1bc0dea..fabf4334 100644 --- a/src/SharpCompress/Common/IEntryExtensions.Async.cs +++ b/src/SharpCompress/Common/IEntryExtensions.Async.cs @@ -53,6 +53,11 @@ internal static partial class IEntryExtensions fullDestinationDirectoryPath, DirectoryManagement.WriteFileOutsideDestinationMessage ); + + if (writeAsync != null) + { + await writeAsync(destinationFileName, cancellationToken).ConfigureAwait(false); + } } else if (options.ExtractFullPath) { @@ -69,11 +74,6 @@ internal static partial class IEntryExtensions Directory.CreateDirectory(destinationFileName); } } - - if (writeAsync != null) - { - await writeAsync(destinationFileName, cancellationToken).ConfigureAwait(false); - } } public async ValueTask WriteEntryToFileAsync( diff --git a/src/SharpCompress/Common/IEntryExtensions.cs b/src/SharpCompress/Common/IEntryExtensions.cs index 094d997b..813ebfe3 100644 --- a/src/SharpCompress/Common/IEntryExtensions.cs +++ b/src/SharpCompress/Common/IEntryExtensions.cs @@ -45,6 +45,7 @@ internal static partial class IEntryExtensions fullDestinationDirectoryPath, DirectoryManagement.WriteFileOutsideDestinationMessage ); + write?.Invoke(destinationFileName); } else if (options.ExtractFullPath) { @@ -61,7 +62,6 @@ internal static partial class IEntryExtensions Directory.CreateDirectory(destinationFileName); } } - write?.Invoke(destinationFileName); } private string GetEntryDestinationFileName( diff --git a/src/SharpCompress/Readers/IAsyncReaderExtensions.cs b/src/SharpCompress/Readers/IAsyncReaderExtensions.cs index a2c01e52..3c1d0408 100644 --- a/src/SharpCompress/Readers/IAsyncReaderExtensions.cs +++ b/src/SharpCompress/Readers/IAsyncReaderExtensions.cs @@ -41,7 +41,7 @@ public static class IAsyncReaderExtensions options, async (x, fm, ct) => { - using var fs = File.Open(destinationFileName, fm); + using var fs = File.Open(x, fm); await reader.WriteEntryToAsync(fs, ct).ConfigureAwait(false); }, cancellationToken @@ -76,7 +76,7 @@ public static class IAsyncReaderExtensions options, async (x, fm, ct) => { - using var fs = File.Open(destinationFileName, fm); + using var fs = File.Open(x, fm); await reader.WriteEntryToAsync(fs, ct).ConfigureAwait(false); }, cancellationToken