test clean up and formatting

This commit is contained in:
Adam Hathcock
2026-05-05 16:57:15 +01:00
parent 2021a06626
commit 3b29de3954
3 changed files with 109 additions and 96 deletions

View File

@@ -9,7 +9,6 @@ namespace SharpCompress.Archives;
public static class IArchiveExtensions
{
/// <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.
@@ -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))
{

View File

@@ -67,7 +67,8 @@ public static class IAsyncArchiveExtensions
IProgress<ProgressReport>? 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))
{

View File

@@ -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<ExtractionException>(() =>
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<ExtractionException>(() =>
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<ExtractionException>(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<ExtractionException>(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