some refactoring

This commit is contained in:
Adam Hathcock
2026-05-05 17:22:53 +01:00
parent 8d6f1014f0
commit bd0fa73cce
6 changed files with 456 additions and 170 deletions

View File

@@ -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<ProgressReport>? 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<string>();
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(

View File

@@ -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
/// <summary>
/// Extract to specific directory asynchronously with progress reporting and cancellation support
/// </summary>
/// <param name="archive">The archive to extract.</param>
/// <param name="destinationDirectory">The folder to extract into.</param>
/// <param name="options">Extraction options.</param>
/// <param name="progress">Optional progress reporter for tracking extraction progress.</param>
@@ -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<string>();
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;

View File

@@ -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);
}
}

View File

@@ -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<string, CancellationToken, ValueTask> 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);
}
}
}

View File

@@ -15,70 +15,87 @@ internal static partial class ExtractionMethods
Action<string> 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<string> 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,

View File

@@ -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<ExtractionException>(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<ExtractionException>(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<Exception?> 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<Exception?> RecordExceptionAsync(Func<Task> 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