diff --git a/docs/API.md b/docs/API.md index 80143e83..7cc13b7d 100644 --- a/docs/API.md +++ b/docs/API.md @@ -463,6 +463,8 @@ using (var archive = ZipArchive.OpenArchive("file.zip")) `CheckCrc` validates archive-level payload checksums when the format stores reliable metadata, such as ZIP CRC32 values. Formats without payload checksums skip this validation. Decompressor integrity checks that are required to decode a stream may still fail even when `CheckCrc` is disabled. +When using `SymbolicLinkHandler`, directory extraction rejects link targets outside the extraction root and never follows symbolic links or reparse points while extracting later entries. The handler itself remains trusted application code. + ### Options matrix ```text diff --git a/src/SharpCompress/Common/DirectoryManagement.cs b/src/SharpCompress/Common/DirectoryManagement.cs index 4bd4585d..0bf1a013 100644 --- a/src/SharpCompress/Common/DirectoryManagement.cs +++ b/src/SharpCompress/Common/DirectoryManagement.cs @@ -1,3 +1,4 @@ +using System; using System.IO; namespace SharpCompress.Common; @@ -8,6 +9,10 @@ internal static class DirectoryManagement "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 const string LinkTargetOutsideDestinationMessage = + "Entry is trying to create a symbolic link outside of the destination directory."; + internal const string ReparsePointInDestinationMessage = + "Entry is trying to extract through a symbolic link or reparse point."; internal static string GetFullDestinationDirectoryPath(string destinationDirectory) { @@ -58,6 +63,114 @@ internal static class DirectoryManagement throw new ExtractionException(exceptionMessage); } + internal static void EnsureNoReparsePointInDestinationDirectory( + string destinationPath, + string fullDestinationDirectoryPath + ) + { + var destinationDirectoryPath = TrimTrailingDirectorySeparators( + fullDestinationDirectoryPath + ); + EnsurePathIsNotReparsePoint(destinationDirectoryPath); + + if (string.Equals(destinationPath, destinationDirectoryPath, Utility.PathComparison)) + { + return; + } + + var relativeDestinationPath = destinationPath.Substring( + fullDestinationDirectoryPath.Length + ); + var path = destinationDirectoryPath; + + foreach ( + var pathPart in relativeDestinationPath.Split( + new[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }, + StringSplitOptions.RemoveEmptyEntries + ) + ) + { + path = Path.Combine(path, pathPart); + + if (!PathExistsAndIsNotReparsePoint(path)) + { + return; + } + } + } + + internal static void CreateDirectory( + string destinationPath, + string fullDestinationDirectoryPath + ) + { + EnsureNoReparsePointInDestinationDirectory(destinationPath, fullDestinationDirectoryPath); + + if (!Directory.Exists(destinationPath)) + { + Directory.CreateDirectory(destinationPath); + } + + EnsureNoReparsePointInDestinationDirectory(destinationPath, fullDestinationDirectoryPath); + } + + internal static void EnsureLinkTargetInDestinationDirectory( + string destinationFileName, + string linkTarget, + string fullDestinationDirectoryPath + ) + { + var destinationDirectory = Path.GetDirectoryName(destinationFileName) + .NotNull("Destination directory is null"); + var fullLinkTargetPath = Path.GetFullPath(Path.Combine(destinationDirectory, linkTarget)); + + EnsurePathInDestinationDirectory( + fullLinkTargetPath, + fullDestinationDirectoryPath, + LinkTargetOutsideDestinationMessage + ); + } + + internal static void EnsurePathIsNotReparsePoint(string path) + { + PathExistsAndIsNotReparsePoint(path); + } + + private static bool PathExistsAndIsNotReparsePoint(string path) + { + try + { + if ((File.GetAttributes(path) & FileAttributes.ReparsePoint) != 0) + { + throw new ExtractionException(ReparsePointInDestinationMessage); + } + + return true; + } + catch (FileNotFoundException) + { + return false; + } + catch (DirectoryNotFoundException) + { + return false; + } + catch (UnauthorizedAccessException exception) + { + throw new ExtractionException( + "Unable to verify the extraction path for symbolic links or reparse points.", + exception + ); + } + catch (IOException exception) + { + throw new ExtractionException( + "Unable to verify the extraction path for symbolic links or reparse points.", + exception + ); + } + } + private static bool IsDirectorySeparator(char value) => value == Path.DirectorySeparatorChar || value == Path.AltDirectorySeparatorChar; diff --git a/src/SharpCompress/Common/ExtractionOptions.cs b/src/SharpCompress/Common/ExtractionOptions.cs index 04b33416..ce58a529 100644 --- a/src/SharpCompress/Common/ExtractionOptions.cs +++ b/src/SharpCompress/Common/ExtractionOptions.cs @@ -60,6 +60,8 @@ public sealed record ExtractionOptions : IExtractionOptions /// /// Breaking change: Changed from field to property in version 0.40.0. /// If no handler is provided, symbolic links are silently skipped during extraction. + /// Directory extraction rejects link targets outside the destination directory and does not + /// follow symbolic links or reparse points in later entry paths. /// public Action? SymbolicLinkHandler { get; set; } diff --git a/src/SharpCompress/Common/IEntryExtensions.Async.cs b/src/SharpCompress/Common/IEntryExtensions.Async.cs index 4ef697e0..eafddfcf 100644 --- a/src/SharpCompress/Common/IEntryExtensions.Async.cs +++ b/src/SharpCompress/Common/IEntryExtensions.Async.cs @@ -38,6 +38,11 @@ internal static partial class IEntryExtensions CancellationToken cancellationToken = default ) { + if (entry.LinkTarget is not null && options.SymbolicLinkHandler is null) + { + return; + } + var destinationFileName = GetEntryDestinationFileName( entry, fullDestinationDirectoryPath, @@ -54,6 +59,20 @@ internal static partial class IEntryExtensions DirectoryManagement.WriteFileOutsideDestinationMessage ); + DirectoryManagement.EnsureNoReparsePointInDestinationDirectory( + destinationFileName, + fullDestinationDirectoryPath + ); + + if (entry.LinkTarget is not null) + { + DirectoryManagement.EnsureLinkTargetInDestinationDirectory( + destinationFileName, + entry.LinkTarget, + fullDestinationDirectoryPath + ); + } + if (writeAsync != null) { await writeAsync(destinationFileName, cancellationToken).ConfigureAwait(false); @@ -69,10 +88,10 @@ internal static partial class IEntryExtensions DirectoryManagement.CreateDirectoryOutsideDestinationMessage ); - if (!Directory.Exists(destinationFileName)) - { - Directory.CreateDirectory(destinationFileName); - } + DirectoryManagement.CreateDirectory( + destinationFileName, + fullDestinationDirectoryPath + ); } } @@ -90,6 +109,8 @@ internal static partial class IEntryExtensions } else { + DirectoryManagement.EnsurePathIsNotReparsePoint(destinationFileName); + var fm = FileMode.Create; if (!options.Overwrite) diff --git a/src/SharpCompress/Common/IEntryExtensions.cs b/src/SharpCompress/Common/IEntryExtensions.cs index 492ad85d..d7e6f5cb 100644 --- a/src/SharpCompress/Common/IEntryExtensions.cs +++ b/src/SharpCompress/Common/IEntryExtensions.cs @@ -45,6 +45,11 @@ internal static partial class IEntryExtensions Action? write ) { + if (entry.LinkTarget is not null && options.SymbolicLinkHandler is null) + { + return; + } + var destinationFileName = GetEntryDestinationFileName( entry, fullDestinationDirectoryPath, @@ -60,6 +65,21 @@ internal static partial class IEntryExtensions fullDestinationDirectoryPath, DirectoryManagement.WriteFileOutsideDestinationMessage ); + + DirectoryManagement.EnsureNoReparsePointInDestinationDirectory( + destinationFileName, + fullDestinationDirectoryPath + ); + + if (entry.LinkTarget is not null) + { + DirectoryManagement.EnsureLinkTargetInDestinationDirectory( + destinationFileName, + entry.LinkTarget, + fullDestinationDirectoryPath + ); + } + write?.Invoke(destinationFileName); } else if (options.ExtractFullPath) @@ -72,10 +92,10 @@ internal static partial class IEntryExtensions DirectoryManagement.CreateDirectoryOutsideDestinationMessage ); - if (!Directory.Exists(destinationFileName)) - { - Directory.CreateDirectory(destinationFileName); - } + DirectoryManagement.CreateDirectory( + destinationFileName, + fullDestinationDirectoryPath + ); } } @@ -102,10 +122,7 @@ internal static partial class IEntryExtensions : DirectoryManagement.WriteFileOutsideDestinationMessage ); - if (!Directory.Exists(destdir)) - { - Directory.CreateDirectory(destdir); - } + DirectoryManagement.CreateDirectory(destdir, fullDestinationDirectoryPath); return Path.Combine(destdir, file); } @@ -126,6 +143,8 @@ internal static partial class IEntryExtensions } else { + DirectoryManagement.EnsurePathIsNotReparsePoint(destinationFileName); + var fm = FileMode.Create; if (!options.Overwrite) diff --git a/src/SharpCompress/Common/Options/IExtractionOptions.cs b/src/SharpCompress/Common/Options/IExtractionOptions.cs index 767bfb93..a6379f91 100644 --- a/src/SharpCompress/Common/Options/IExtractionOptions.cs +++ b/src/SharpCompress/Common/Options/IExtractionOptions.cs @@ -39,6 +39,8 @@ public interface IExtractionOptions /// Delegate for writing symbolic links to disk. /// The first parameter is the source path (where the symlink is created). /// The second parameter is the target path (what the symlink refers to). + /// Directory extraction rejects link targets outside the destination directory and does not + /// follow symbolic links or reparse points in later entry paths. /// Action? SymbolicLinkHandler { get; set; } } diff --git a/tests/SharpCompress.Test/Security/SymbolicLinkExtractionTests.cs b/tests/SharpCompress.Test/Security/SymbolicLinkExtractionTests.cs new file mode 100644 index 00000000..96ae170c --- /dev/null +++ b/tests/SharpCompress.Test/Security/SymbolicLinkExtractionTests.cs @@ -0,0 +1,304 @@ +#if NET8_0_OR_GREATER +using System; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Common; +using SharpCompress.Readers; +using Xunit; + +namespace SharpCompress.Test.Security; + +public class SymbolicLinkExtractionTests : TestBase +{ + private const int TarBlockSize = 512; + + [Theory] + [InlineData("ReaderAll")] + [InlineData("ReaderEntry")] + [InlineData("Archive")] + [InlineData("ArchiveEntry")] + [InlineData("AsyncReaderAll")] + [InlineData("AsyncReaderEntry")] + [InlineData("AsyncArchive")] + [InlineData("AsyncArchiveEntry")] + public async Task SymbolicLinkTargetOutsideDestination_ShouldThrowBeforeInvokingHandler( + string api + ) + { + var destinationDirectory = GetScratchPath("extract"); + var outsideDirectory = GetScratchPath("outside"); + var archivePath = GetScratch2Path("symbolic-link-outside.tar"); + Directory.CreateDirectory(destinationDirectory); + Directory.CreateDirectory(outsideDirectory); + BuildTar(archivePath, "../outside"); + + var handlerCalls = 0; + var options = new ExtractionOptions { SymbolicLinkHandler = (_, _) => handlerCalls++ }; + + var exception = await ExtractAsync(api, archivePath, destinationDirectory, options); + + var extractionException = Assert.IsType(exception); + Assert.Contains("symbolic link outside", extractionException.Message); + Assert.Equal(0, handlerCalls); + Assert.False(File.Exists(Path.Combine(outsideDirectory, "secret.txt"))); + } + + [Theory] + [InlineData("ReaderAll")] + [InlineData("Archive")] + [InlineData("AsyncReaderAll")] + [InlineData("AsyncArchive")] + public async Task EntriesBeneathSymbolicLink_ShouldNotBeExtracted(string api) + { + var destinationDirectory = GetScratchPath("extract"); + var targetDirectory = Path.Combine(destinationDirectory, "target"); + var archivePath = GetScratch2Path("symbolic-link-inside.tar"); + Directory.CreateDirectory(destinationDirectory); + Directory.CreateDirectory(targetDirectory); + BuildTar(archivePath, "target"); + + var handlerCalls = 0; + var options = new ExtractionOptions + { + SymbolicLinkHandler = (linkPath, linkTarget) => + { + handlerCalls++; + Directory.CreateSymbolicLink(linkPath, linkTarget); + }, + }; + + var exception = await ExtractAsync(api, archivePath, destinationDirectory, options); + + var extractionException = Assert.IsType(exception); + Assert.Contains("symbolic link or reparse point", extractionException.Message); + Assert.Equal(1, handlerCalls); + Assert.False(File.Exists(Path.Combine(targetDirectory, "secret.txt"))); + } + + private static void BuildTar(string path, string linkTarget) + { + using var stream = File.Create(path); + WriteTarEntry(stream, "link", (byte)'2', linkTarget, Array.Empty()); + WriteTarEntry(stream, "link/secret.txt", (byte)'0', null, Encoding.UTF8.GetBytes("secret")); + stream.Write(new byte[TarBlockSize * 2]); + } + + private static void WriteTarEntry( + Stream stream, + string name, + byte entryType, + string? linkTarget, + byte[] data + ) + { + var header = new byte[TarBlockSize]; + WriteString(header, 0, 100, name); + WriteOctal(header, 100, 8, 0b110_100_100); + WriteOctal(header, 108, 8, 0); + WriteOctal(header, 116, 8, 0); + WriteOctal(header, 124, 12, data.Length); + WriteOctal(header, 136, 12, 0); + Array.Fill(header, (byte)' ', 148, 8); + header[156] = entryType; + WriteString(header, 157, 100, linkTarget ?? string.Empty); + WriteString(header, 257, 6, "ustar"); + WriteString(header, 263, 2, "00"); + + var checksum = header.Sum(value => value); + WriteString(header, 148, 6, Convert.ToString(checksum, 8).PadLeft(6, '0')); + header[154] = 0; + header[155] = (byte)' '; + + stream.Write(header); + stream.Write(data); + + var padding = (TarBlockSize - (data.Length % TarBlockSize)) % TarBlockSize; + if (padding > 0) + { + stream.Write(new byte[padding]); + } + } + + private static void WriteString(byte[] buffer, int offset, int length, string value) + { + var bytes = Encoding.ASCII.GetBytes(value); + Array.Copy(bytes, 0, buffer, offset, Math.Min(bytes.Length, length)); + } + + private static void WriteOctal(byte[] buffer, int offset, int length, long value) + { + WriteString( + buffer, + offset, + length - 1, + Convert.ToString(value, 8).PadLeft(length - 1, '0') + ); + buffer[offset + length - 1] = 0; + } + + private static Task ExtractAsync( + string api, + string archivePath, + string destinationDirectory, + ExtractionOptions options + ) => + api switch + { + "ReaderAll" => Task.FromResult( + RecordException(() => + ExtractWithReaderAll(archivePath, destinationDirectory, options) + ) + ), + "ReaderEntry" => Task.FromResult( + RecordException(() => + ExtractWithReaderEntry(archivePath, destinationDirectory, options) + ) + ), + "Archive" => Task.FromResult( + RecordException(() => + ExtractWithArchive(archivePath, destinationDirectory, options) + ) + ), + "ArchiveEntry" => Task.FromResult( + RecordException(() => + ExtractWithArchiveEntry(archivePath, destinationDirectory, options) + ) + ), + "AsyncReaderAll" => RecordExceptionAsync(() => + ExtractWithAsyncReaderAllAsync(archivePath, destinationDirectory, options) + ), + "AsyncReaderEntry" => RecordExceptionAsync(() => + ExtractWithAsyncReaderEntryAsync(archivePath, destinationDirectory, options) + ), + "AsyncArchive" => RecordExceptionAsync(() => + ExtractWithAsyncArchiveAsync(archivePath, destinationDirectory, options) + ), + "AsyncArchiveEntry" => RecordExceptionAsync(() => + ExtractWithAsyncArchiveEntryAsync(archivePath, destinationDirectory, 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 destinationDirectory, + ExtractionOptions options + ) + { + using var stream = File.OpenRead(archivePath); + using var reader = ReaderFactory.OpenReader(stream); + reader.WriteAllToDirectory(destinationDirectory, options); + } + + private static void ExtractWithReaderEntry( + string archivePath, + string destinationDirectory, + ExtractionOptions options + ) + { + using var stream = File.OpenRead(archivePath); + using var reader = ReaderFactory.OpenReader(stream); + Assert.True(reader.MoveToNextEntry()); + reader.WriteEntryToDirectory(destinationDirectory, options); + } + + private static void ExtractWithArchive( + string archivePath, + string destinationDirectory, + ExtractionOptions options + ) + { + using var archive = ArchiveFactory.OpenArchive(archivePath); + archive.WriteToDirectory(destinationDirectory, options); + } + + private static void ExtractWithArchiveEntry( + string archivePath, + string destinationDirectory, + ExtractionOptions options + ) + { + using var archive = ArchiveFactory.OpenArchive(archivePath); + archive.Entries.First().WriteToDirectory(destinationDirectory, options); + } + + private static async Task ExtractWithAsyncReaderAllAsync( + string archivePath, + string destinationDirectory, + ExtractionOptions options + ) + { + using var stream = File.OpenRead(archivePath); + await using var reader = await ReaderFactory.OpenAsyncReader(stream); + await reader.WriteAllToDirectoryAsync(destinationDirectory, options); + } + + private static async Task ExtractWithAsyncReaderEntryAsync( + string archivePath, + string destinationDirectory, + ExtractionOptions options + ) + { + using var stream = File.OpenRead(archivePath); + await using var reader = await ReaderFactory.OpenAsyncReader(stream); + Assert.True(await reader.MoveToNextEntryAsync()); + await reader.WriteEntryToDirectoryAsync(destinationDirectory, options); + } + + private static async Task ExtractWithAsyncArchiveAsync( + string archivePath, + string destinationDirectory, + ExtractionOptions options + ) + { + await using var archive = await ArchiveFactory.OpenAsyncArchive(archivePath); + await archive.WriteToDirectoryAsync(destinationDirectory, options); + } + + private static async Task ExtractWithAsyncArchiveEntryAsync( + string archivePath, + string destinationDirectory, + ExtractionOptions options + ) + { + await using var archive = await ArchiveFactory.OpenAsyncArchive(archivePath); + + await foreach (var entry in archive.EntriesAsync) + { + await entry.WriteToDirectoryAsync(destinationDirectory, options); + return; + } + + throw new InvalidOperationException("Archive did not contain an entry."); + } +} +#endif