diff --git a/README.md b/README.md index 65cccf52..176b9979 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,49 @@ public string? ParentPath { get; } public DateTime CreateTime { get; } public DateTime ModifyTime { get; } public DateTime AccessTime { get; } +public FileEntryMetadata? Metadata { get; set; } +``` + + +
+File Metadata +
+When the source archive or disc image records it, `FileEntry.Metadata` holds additional attributes for the entry. Every property is nullable, and `Metadata` itself is `null` when the format carries no metadata at all, so you can distinguish "not recorded" from a real value. + +```csharp +public long? Mode { get; set; } // Unix permission bits +public bool? IsExecutable { get; } // Derived from Mode +public bool? IsSetUid { get; } // Derived from Mode +public bool? IsSetGid { get; } // Derived from Mode +public long? Uid { get; set; } // Unix owner id +public long? Gid { get; set; } // Unix group id +public FileAttributes? FileAttributes { get; set; } // Windows/DOS file attributes +public string? SecurityDescriptorSddl { get; set; } // Windows security descriptor, SDDL form +``` + +Which properties are populated depends on the format: + +| Source | Populated | +| --- | --- | +| TAR, AR/DEB | `Mode`, `Uid`, `Gid` | +| ZIP | `Mode`, when the entry records Unix permissions in its external attributes | +| RAR, 7z | `Mode`, taken from the raw attribute field the archive records: a Unix mode for archives created on Unix, DOS attributes otherwise | +| Ext, XFS, Btrfs, HFS+ (inside VHD/VHDX/VMDK/DMG) | `Mode`, `Uid`, `Gid` | +| ISO 9660 with RockRidge extensions | `Mode`, `Uid`, `Gid`, `FileAttributes` | +| ISO 9660 without RockRidge extensions | `FileAttributes` | +| FAT (inside a disc image) | `FileAttributes` | +| NTFS (inside a disc image) | `FileAttributes`, `SecurityDescriptorSddl` | +| WIM | `FileAttributes`, and `SecurityDescriptorSddl` when the image records one | +| UDF | None; `Metadata` is `null` | + +```csharp +foreach (var file in extractor.Extract("path/to/image.vhdx")) +{ + if (file.Metadata?.SecurityDescriptorSddl is { } sddl) + { + Console.WriteLine($"{file.FullPath}: {sddl}"); + } +} ```
diff --git a/RecursiveExtractor.Cli.Tests/CliTests/CliTests.cs b/RecursiveExtractor.Cli.Tests/CliTests/CliTests.cs index 7c9eb773..71a35820 100644 --- a/RecursiveExtractor.Cli.Tests/CliTests/CliTests.cs +++ b/RecursiveExtractor.Cli.Tests/CliTests/CliTests.cs @@ -27,11 +27,11 @@ public static TheoryData CliArchiveData { { "TestData.zip", 5 }, { "TestData.7z", 3 }, - { "TestData.tar", 6 }, + { "TestData.tar", 5 }, { "TestData.rar", 3 }, { "TestData.rar4", 3 }, - { "TestData.tar.bz2", 6 }, - { "TestData.tar.gz", 6 }, + { "TestData.tar.bz2", 5 }, + { "TestData.tar.gz", 5 }, { "TestData.tar.xz", 3 }, { "sysvbanner_1.0-17fakesync1_amd64.deb", 8 }, { "TestData.a", 3 }, @@ -39,7 +39,7 @@ public static TheoryData CliArchiveData { "TestData.iso", 3 }, { "TestData.vhdx", 3 }, { "EmptyFile.txt", 1 }, - { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 54 : 52 }, + { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 51 : 49 }, }; if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { diff --git a/RecursiveExtractor.Cli.Tests/RecursiveExtractor.Cli.Tests.csproj b/RecursiveExtractor.Cli.Tests/RecursiveExtractor.Cli.Tests.csproj index 41d07b76..89ddffce 100644 --- a/RecursiveExtractor.Cli.Tests/RecursiveExtractor.Cli.Tests.csproj +++ b/RecursiveExtractor.Cli.Tests/RecursiveExtractor.Cli.Tests.csproj @@ -10,9 +10,9 @@ - - - + + + diff --git a/RecursiveExtractor.Cli/RecursiveExtractor.Cli.csproj b/RecursiveExtractor.Cli/RecursiveExtractor.Cli.csproj index df2aa1a5..1a24e848 100644 --- a/RecursiveExtractor.Cli/RecursiveExtractor.Cli.csproj +++ b/RecursiveExtractor.Cli/RecursiveExtractor.Cli.csproj @@ -29,7 +29,7 @@ - + diff --git a/RecursiveExtractor.Tests/ExtractorTests/DisposeBehaviorTests.cs b/RecursiveExtractor.Tests/ExtractorTests/DisposeBehaviorTests.cs index da7883bc..af9d136b 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/DisposeBehaviorTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/DisposeBehaviorTests.cs @@ -23,11 +23,11 @@ public static TheoryData DisposeData var data = new TheoryData { { "TestData.7z", 3, false }, - { "TestData.tar", 6, false }, + { "TestData.tar", 5, false }, { "TestData.rar", 3, false }, { "TestData.rar4", 3, false }, - { "TestData.tar.bz2", 6, false }, - { "TestData.tar.gz", 6, false }, + { "TestData.tar.bz2", 5, false }, + { "TestData.tar.gz", 5, false }, { "TestData.tar.xz", 3, false }, { "sysvbanner_1.0-17fakesync1_amd64.deb", 8, false }, { "TestData.a", 3, false }, @@ -37,11 +37,11 @@ public static TheoryData DisposeData { "EmptyFile.txt", 1, false }, { "TestData.zip", 5, true }, { "TestData.7z", 3, true }, - { "TestData.tar", 6, true }, + { "TestData.tar", 5, true }, { "TestData.rar", 3, true }, { "TestData.rar4", 3, true }, - { "TestData.tar.bz2", 6, true }, - { "TestData.tar.gz", 6, true }, + { "TestData.tar.bz2", 5, true }, + { "TestData.tar.gz", 5, true }, { "TestData.tar.xz", 3, true }, { "sysvbanner_1.0-17fakesync1_amd64.deb", 8, true }, { "TestData.a", 3, true }, @@ -49,8 +49,8 @@ public static TheoryData DisposeData { "TestData.iso", 3, true }, { "TestData.vhdx", 3, true }, { "EmptyFile.txt", 1, true }, - { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 54 : 52, true }, - { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 54 : 52, false }, + { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 51 : 49, true }, + { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 51 : 49, false }, }; if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { diff --git a/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs b/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs index 92dd3c24..f3409ff8 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/ExpectedNumFilesTests.cs @@ -28,19 +28,21 @@ public static TheoryData ArchiveData { "100trees.7z", 101 }, { "TestData.zip", 5 }, { "TestData.7z",3 }, - { "TestData.tar", 6 }, + { "TestData.tar", 5 }, { "TestData.rar",3 }, { "TestData.rar4",3 }, - { "TestData.tar.bz2", 6 }, - { "TestData.tar.gz", 6 }, + { "TestData.tar.bz2", 5 }, + { "TestData.tar.gz", 5 }, { "TestData.tar.xz",3 }, { "sysvbanner_1.0-17fakesync1_amd64.deb", 8 }, { "TestData.a",3 }, { "TestData.bsd.ar",3 }, { "TestData.iso",3 }, + { "TestDataRockRidge.iso",2 }, + { "TestDataJolietRockRidge.iso",2 }, { "TestData.vhdx",3 }, { "EmptyFile.txt", 1 }, - { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 54 : 52 }, + { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 51 : 49 }, { "UdfTest.iso", 3 }, { "UdfTestWithMultiSystem.iso", 3 }, { "TestData.arj", 1 }, @@ -69,7 +71,7 @@ public static TheoryData NoRecursionData { "100trees.7z", 101 }, { "TestData.zip", 5 }, { "TestData.7z", 3 }, - { "TestData.tar", 6 }, + { "TestData.tar", 5 }, { "TestData.rar", 3 }, { "TestData.rar4", 3 }, { "TestData.tar.bz2", 1 }, @@ -79,6 +81,8 @@ public static TheoryData NoRecursionData { "TestData.a", 3 }, { "TestData.bsd.ar", 3 }, { "TestData.iso", 3 }, + { "TestDataRockRidge.iso", 2 }, + { "TestDataJolietRockRidge.iso", 2 }, { "TestData.vhdx", 3 }, { "EmptyFile.txt", 1 }, { "TestDataArchivesNested.zip", 14 }, diff --git a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs index 1700217f..6f1a52a9 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/FileMetadataTests.cs @@ -1,8 +1,10 @@ -// Copyright (c) Microsoft Corporation. Licensed under the MIT License. +// Copyright (c) Microsoft Corporation. Licensed under the MIT License. using Microsoft.CST.RecursiveExtractor; +using System.Collections.Generic; using System.IO; using System.Linq; +using System.Runtime.InteropServices; using System.Threading.Tasks; using Xunit; @@ -101,6 +103,8 @@ public void MetadataDefaults_AreNull() Assert.Null(metadata.IsExecutable); Assert.Null(metadata.IsSetUid); Assert.Null(metadata.IsSetGid); + Assert.Null(metadata.FileAttributes); + Assert.Null(metadata.SecurityDescriptorSddl); } [Fact] @@ -140,4 +144,248 @@ public void FileEntry_MetadataDefaultsToNull() var entry = new FileEntry("test.txt", stream); Assert.Null(entry.Metadata); } + + [Fact] + public async Task IsoEntries_HaveDosAttributesButNoUnixMetadataWithoutRockRidge() + { + // TestData.iso has no RockRidge extensions, so Unix metadata is unavailable. + // CDReader still implements IDosFileSystem, so file attributes are reported. + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.iso"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.NotNull(entry.Metadata); + Assert.Equal(FileAttributes.ReadOnly, entry.Metadata!.FileAttributes); + Assert.Null(entry.Metadata.Mode); + Assert.Null(entry.Metadata.Uid); + Assert.Null(entry.Metadata.Gid); + Assert.Null(entry.Metadata.SecurityDescriptorSddl); + } + } + + [Fact] + public void IsoEntries_HaveDosAttributesButNoUnixMetadataWithoutRockRidge_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.iso"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.NotNull(entry.Metadata); + Assert.Equal(FileAttributes.ReadOnly, entry.Metadata!.FileAttributes); + Assert.Null(entry.Metadata.Mode); + Assert.Null(entry.Metadata.Uid); + Assert.Null(entry.Metadata.Gid); + Assert.Null(entry.Metadata.SecurityDescriptorSddl); + } + } + + [Fact] + public async Task IsoRockRidgeEntries_HaveMetadata() + { + // TestDataRockRidge.iso has RockRidge extensions with Unix permissions + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestDataRockRidge.iso"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + AssertRockRidgeMetadata(results); + } + + [Fact] + public void IsoRockRidgeEntries_HaveMetadata_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestDataRockRidge.iso"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + AssertRockRidgeMetadata(results); + } + + private static void AssertRockRidgeMetadata(IList results) + { + Assert.Equal(2, results.Count); + foreach (var entry in results) + { + Assert.NotNull(entry.Metadata); + Assert.Equal(1001, entry.Metadata!.Uid); + Assert.Equal(1001, entry.Metadata.Gid); + Assert.NotNull(entry.Metadata.FileAttributes); + } + + // testfile.txt is 0755 (493 decimal), subdir/nested.txt is 0644 (420 decimal) + var topLevel = results.Single(x => x.Name == "testfile.txt"); + Assert.Equal(493, topLevel.Metadata!.Mode); + Assert.True(topLevel.Metadata.IsExecutable); + + var nested = results.Single(x => x.Name == "nested.txt"); + Assert.Equal(420, nested.Metadata!.Mode); + Assert.False(nested.Metadata.IsExecutable); + } + + [Fact] + public async Task IsoJolietRockRidgeEntries_HaveUnixMetadata() + { + // TestDataJolietRockRidge.iso carries both a Joliet supplementary volume descriptor and RockRidge + // SUSP records. DiscUtils gives Joliet priority and only parses SUSP records for the variant it + // activates, so the Unix metadata is only reachable through a second reader that skips Joliet. + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestDataJolietRockRidge.iso"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + AssertJolietRockRidgeMetadata(results); + } + + [Fact] + public void IsoJolietRockRidgeEntries_HaveUnixMetadata_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestDataJolietRockRidge.iso"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + AssertJolietRockRidgeMetadata(results); + } + + private static void AssertJolietRockRidgeMetadata(IList results) + { + Assert.Equal(2, results.Count); + foreach (var entry in results) + { + Assert.NotNull(entry.Metadata); + Assert.Equal(0, entry.Metadata!.Uid); + Assert.Equal(0, entry.Metadata.Gid); + // File attributes still come from the Joliet tree the entries were enumerated from + Assert.Equal(FileAttributes.ReadOnly, entry.Metadata.FileAttributes); + Assert.Null(entry.Metadata.SecurityDescriptorSddl); + } + + // testfile.txt is 0755 (493 decimal), subdir/nested.txt is 0644 (420 decimal) + var topLevel = results.Single(x => x.Name == "testfile.txt"); + Assert.Equal(493, topLevel.Metadata!.Mode); + Assert.True(topLevel.Metadata.IsExecutable); + + var nested = results.Single(x => x.Name == "nested.txt"); + Assert.Equal(420, nested.Metadata!.Mode); + Assert.False(nested.Metadata.IsExecutable); + } + + [Fact] + public async Task UdfEntries_HaveNoMetadata() + { + // UdfReader implements none of the Unix/DOS/Windows file system interfaces, + // so no metadata is available for pure UDF images. + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "UdfTest.iso"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.Null(entry.Metadata); + } + } + + [Fact] + public void UdfEntries_HaveNoMetadata_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "UdfTest.iso"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.Null(entry.Metadata); + } + } + + [Fact] + public async Task VhdxNtfsEntries_HaveWindowsMetadata() + { + // TestData.vhdx contains an NTFS file system that implements IDosFileSystem and IWindowsFileSystem + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.vhdx"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + AssertNtfsMetadata(results); + } + + [Fact] + public void VhdxNtfsEntries_HaveWindowsMetadata_Sync() + { + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.vhdx"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + AssertNtfsMetadata(results); + } + + private static void AssertNtfsMetadata(IList results) + { + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.NotNull(entry.Metadata); + // NTFS provides Windows file attributes + Assert.Equal(FileAttributes.Archive, entry.Metadata!.FileAttributes); + // NTFS provides security descriptors + Assert.NotNull(entry.Metadata.SecurityDescriptorSddl); + Assert.Contains("O:", entry.Metadata.SecurityDescriptorSddl); // Owner present + Assert.Contains("D:", entry.Metadata.SecurityDescriptorSddl); // DACL present + // NTFS does not provide Unix metadata + Assert.Null(entry.Metadata.Mode); + Assert.Null(entry.Metadata.Uid); + Assert.Null(entry.Metadata.Gid); + } + } + + [Fact] + public async Task WimEntries_HaveWindowsFileAttributes() + { + // WIM extraction is only supported on Windows + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return; + } + + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.wim"); + var results = await extractor.ExtractAsync(path, new ExtractorOptions() { Recurse = false }).ToListAsync(); + + AssertWimMetadata(results); + } + + [Fact] + public void WimEntries_HaveWindowsFileAttributes_Sync() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return; + } + + var extractor = new Extractor(); + var path = Path.Combine(Directory.GetCurrentDirectory(), "TestData", "TestDataArchives", "TestData.wim"); + var results = extractor.Extract(path, new ExtractorOptions() { Recurse = false }).ToList(); + + AssertWimMetadata(results); + } + + private static void AssertWimMetadata(IList results) + { + Assert.NotEmpty(results); + foreach (var entry in results) + { + Assert.NotNull(entry.Metadata); + Assert.Equal(FileAttributes.Archive, entry.Metadata!.FileAttributes); + Assert.Null(entry.Metadata.Mode); + Assert.Null(entry.Metadata.Uid); + Assert.Null(entry.Metadata.Gid); + // This WIM records no security descriptors, so an empty SDDL is normalized to null + Assert.Null(entry.Metadata.SecurityDescriptorSddl); + } + } } diff --git a/RecursiveExtractor.Tests/ExtractorTests/FilterTests.cs b/RecursiveExtractor.Tests/ExtractorTests/FilterTests.cs index 3ed7a32c..50ee5c3e 100644 --- a/RecursiveExtractor.Tests/ExtractorTests/FilterTests.cs +++ b/RecursiveExtractor.Tests/ExtractorTests/FilterTests.cs @@ -56,11 +56,11 @@ public static TheoryData DenyFilterData { { "TestData.zip", 4 }, { "TestData.7z", 2 }, - { "TestData.tar", 5 }, + { "TestData.tar", 4 }, { "TestData.rar", 2 }, { "TestData.rar4", 2 }, - { "TestData.tar.bz2", 5 }, - { "TestData.tar.gz", 5 }, + { "TestData.tar.bz2", 4 }, + { "TestData.tar.gz", 4 }, { "TestData.tar.xz", 2 }, { "sysvbanner_1.0-17fakesync1_amd64.deb", 8 }, { "TestData.a", 3 }, @@ -68,7 +68,7 @@ public static TheoryData DenyFilterData { "TestData.iso", 2 }, { "TestData.vhdx", 2 }, { "EmptyFile.txt", 1 }, - { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 45 : 44 }, + { "TestDataArchivesNested.zip", RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 42 : 41 }, }; if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { diff --git a/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj b/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj index 248021b2..67b13b0b 100644 --- a/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj +++ b/RecursiveExtractor.Tests/RecursiveExtractor.Tests.csproj @@ -10,12 +10,12 @@ - + - - - - + + + + @@ -165,6 +165,12 @@ PreserveNewest + + PreserveNewest + + + PreserveNewest + PreserveNewest diff --git a/RecursiveExtractor.Tests/SanitizePathTests.cs b/RecursiveExtractor.Tests/SanitizePathTests.cs index 3303238e..06c9261c 100644 --- a/RecursiveExtractor.Tests/SanitizePathTests.cs +++ b/RecursiveExtractor.Tests/SanitizePathTests.cs @@ -35,6 +35,166 @@ public void TestSanitizePathLinux(string linuxInputPath, string expectedLinuxPat } } + /// + /// ZipSlipSanitize should strip leading slashes to prevent absolute path traversal. + /// On any OS, a Unix-style absolute path like "/etc/passwd" must become relative. + /// + [Theory] + [InlineData("/etc/passwd", "etc/passwd")] + [InlineData("/tmp/evil.txt", "tmp/evil.txt")] + [InlineData("///triple/leading", "triple/leading")] + [InlineData("/", "")] + public void TestZipSlipSanitize_AbsoluteUnixPaths(string input, string expected) + { + // Normalize expected to the current OS separator + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// ZipSlipSanitize should strip Windows drive letter roots. + /// + [Theory] + [InlineData("C:\\Windows\\System32\\evil.dll", "Windows/System32/evil.dll")] + [InlineData("D:/data/file.txt", "data/file.txt")] + [InlineData("C:\\", "")] + [InlineData("C:/", "")] + [InlineData("a:file.txt", "a:file.txt")] + public void TestZipSlipSanitize_AbsoluteWindowsPaths(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// ZipSlipSanitize should recursively strip nested roots that are exposed after each strip. + /// A single pass is insufficient for crafted paths like "D:\C:\" (stripping D: exposes \C:\), + /// or paths starting with multiple separators like "\\C:\" (stripping both leading separators + /// exposes C:\), or "../C:\file" where removing ".." exposes C:\file. + /// + [Theory] + [InlineData("D:\\C:\\file.txt", "file.txt")] + [InlineData("\\\\C:\\file.txt", "file.txt")] + [InlineData("..\\C:\\file.txt", "file.txt")] + [InlineData("D:/C:/file.txt", "file.txt")] + [InlineData("D:\\C:\\D:\\file.txt", "file.txt")] + public void TestZipSlipSanitize_NestedRoots(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// ZipSlipSanitize should strip ".." directory traversal components. + /// + [Theory] + [InlineData("foo/../../../etc/passwd", "foo/etc/passwd")] + [InlineData("../secret.txt", "secret.txt")] + [InlineData("a/b/../../c", "a/b/c")] + public void TestZipSlipSanitize_DotDotTraversal(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// ZipSlipSanitize should handle combined absolute + traversal attacks. + /// After ".." is stripped, exposed leading separators are also stripped. + /// + [Theory] + [InlineData("/../../etc/passwd", "etc/passwd")] + [InlineData("C:\\..\\..\\Windows\\evil.dll", "Windows/evil.dll")] + [InlineData("/../../../tmp/evil", "tmp/evil")] + public void TestZipSlipSanitize_CombinedAbsoluteAndTraversal(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// ZipSlipSanitize should collapse double separators that result from stripping. + /// + [Theory] + [InlineData("a/../b", "a/b")] + [InlineData("a/../../b", "a/b")] + public void TestZipSlipSanitize_CollapsesDoubleSeparators(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// Safe relative paths should pass through unmodified. + /// Filenames containing ".." as a substring (not a path segment) must be preserved. + /// + [Theory] + [InlineData("normal/path/file.txt", "normal/path/file.txt")] + [InlineData("file.txt", "file.txt")] + [InlineData("a/b/c/d.txt", "a/b/c/d.txt")] + [InlineData("file..txt", "file..txt")] + [InlineData("my..archive/data..bin", "my..archive/data..bin")] + [InlineData("a/./b", "a/b")] + [InlineData("a:file.txt", "a:file.txt")] + [InlineData("a:folder/file.txt", "a:folder/file.txt")] + public void TestZipSlipSanitize_SafePathsUnchanged(string input, string expected) + { + expected = expected.Replace('/', Path.DirectorySeparatorChar); + var result = FileEntry.ZipSlipSanitize(input); + Assert.Equal(expected, result); + } + + /// + /// FileEntry.FullPath should never be absolute when constructed with a parent, + /// even if the entry name is an absolute path. + /// + [Fact] + public void TestFileEntry_AbsoluteEntryName_BecomesRelative() + { + var parent = new FileEntry("archive.zip", Stream.Null); + var child = new FileEntry("/etc/cron.d/evil", Stream.Null, parent); + Assert.False(Path.IsPathRooted(child.FullPath), + $"FullPath should be relative but was: {child.FullPath}"); + AssertNoTraversalSegments(child.FullPath); + } + + /// + /// FileEntry.FullPath should not contain ".." path segments even when the entry name has traversal. + /// + [Fact] + public void TestFileEntry_TraversalEntryName_Sanitized() + { + var parent = new FileEntry("archive.tar", Stream.Null); + var child = new FileEntry("../../../etc/passwd", Stream.Null, parent); + AssertNoTraversalSegments(child.FullPath); + } + + /// + /// Backslash-rooted paths should also be made relative. + /// + [Fact] + public void TestFileEntry_BackslashRooted_BecomesRelative() + { + var parent = new FileEntry("archive.zip", Stream.Null); + var child = new FileEntry("\\Windows\\System32\\evil.dll", Stream.Null, parent); + Assert.False(Path.IsPathRooted(child.FullPath), + $"FullPath should be relative but was: {child.FullPath}"); + } + + /// + /// Assert that no path segment is exactly ".." (ignoring ".." as a substring in filenames like "file..txt"). + /// + private static void AssertNoTraversalSegments(string path) + { + var segments = path.Split(Path.DirectorySeparatorChar); + Assert.DoesNotContain("..", segments); + } + protected static readonly NLog.Logger Logger = NLog.LogManager.GetCurrentClassLogger(); } } \ No newline at end of file diff --git a/RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataJolietRockRidge.iso b/RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataJolietRockRidge.iso new file mode 100644 index 00000000..28861c61 Binary files /dev/null and b/RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataJolietRockRidge.iso differ diff --git a/RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataRockRidge.iso b/RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataRockRidge.iso new file mode 100644 index 00000000..f7a9291b Binary files /dev/null and b/RecursiveExtractor.Tests/TestData/TestDataArchives/TestDataRockRidge.iso differ diff --git a/RecursiveExtractor/Extractor.cs b/RecursiveExtractor/Extractor.cs index 85d53e8a..ec5015df 100644 --- a/RecursiveExtractor/Extractor.cs +++ b/RecursiveExtractor/Extractor.cs @@ -495,9 +495,16 @@ public ExtractionStatusCode ExtractToDirectory(string outputDirectory, FileEntry opts ??= new ExtractorOptions(); if (!opts.Parallel) { + var fullOutputDir = Path.GetFullPath(outputDirectory).TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar; foreach (var entry in Extract(fileEntry, opts)) { var targetPath = Path.Combine(outputDirectory, entry.GetSanitizedPath()); + var fullTargetPath = Path.GetFullPath(targetPath); + if (!fullTargetPath.StartsWith(fullOutputDir, StringComparison.Ordinal)) + { + Logger.Warn("Skipping entry that would escape output directory: {0}", entry.FullPath); + continue; + } if (Path.GetDirectoryName(targetPath) is { } directoryPathNotNull && targetPath is { } targetPathNotNull) { try @@ -537,6 +544,7 @@ public ExtractionStatusCode ExtractToDirectory(string outputDirectory, FileEntry using var enumerator = extractedEnumeration.GetEnumerator(); // Move to the first element to prepare ConcurrentBag entryBatch = new(); + var parallelFullOutputDir = Path.GetFullPath(outputDirectory).TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar; bool moreAvailable = enumerator.MoveNext(); while (moreAvailable) { @@ -559,6 +567,12 @@ public ExtractionStatusCode ExtractToDirectory(string outputDirectory, FileEntry Parallel.ForEach(entryBatch, new ParallelOptions() { CancellationToken = cts.Token }, entry => { var targetPath = Path.Combine(outputDirectory, entry.GetSanitizedPath()); + var fullTargetPath = Path.GetFullPath(targetPath); + if (!fullTargetPath.StartsWith(parallelFullOutputDir, StringComparison.Ordinal)) + { + Logger.Warn("Skipping entry that would escape output directory: {0}", entry.FullPath); + return; + } if (Path.GetDirectoryName(targetPath) is { } directoryPathNotNull && targetPath is { } targetPathNotNull) { @@ -645,11 +659,18 @@ public async Task ExtractToDirectoryAsync(string outputDir /// If we should print the filename when writing it out to disc. public async Task ExtractToDirectoryAsync(string outputDirectory, FileEntry fileEntry, ExtractorOptions? opts = null, IEnumerable? acceptFilters = null, IEnumerable? denyFilters = null, bool printNames = false) { + var asyncFullOutputDir = Path.GetFullPath(outputDirectory).TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar; await foreach (var entry in ExtractAsync(fileEntry, opts)) { if (opts?.FileNamePasses(entry.FullPath) ?? true) { var targetPath = Path.Combine(outputDirectory, entry.GetSanitizedPath()); + var fullTargetPath = Path.GetFullPath(targetPath); + if (!fullTargetPath.StartsWith(asyncFullOutputDir, StringComparison.Ordinal)) + { + Logger.Warn("Skipping entry that would escape output directory: {0}", entry.FullPath); + continue; + } if (Path.GetDirectoryName(targetPath) is { } directoryPathNotNull && targetPath is { } targetPathNotNull) { diff --git a/RecursiveExtractor/Extractors/AceExtractor.cs b/RecursiveExtractor/Extractors/AceExtractor.cs index ae0429f8..54c0b2ff 100644 --- a/RecursiveExtractor/Extractors/AceExtractor.cs +++ b/RecursiveExtractor/Extractors/AceExtractor.cs @@ -29,11 +29,11 @@ public AceExtractor(Extractor context) /// public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - AceReader? aceReader = null; + IReader? aceReader = null; try { fileEntry.Content.Position = 0; - aceReader = AceReader.Open(fileEntry.Content, new ReaderOptions() + aceReader = AceReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); @@ -100,11 +100,11 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra /// public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - AceReader? aceReader = null; + IReader? aceReader = null; try { fileEntry.Content.Position = 0; - aceReader = AceReader.Open(fileEntry.Content, new ReaderOptions() + aceReader = AceReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); diff --git a/RecursiveExtractor/Extractors/ArcExtractor.cs b/RecursiveExtractor/Extractors/ArcExtractor.cs index 5b33bd12..8247b15e 100644 --- a/RecursiveExtractor/Extractors/ArcExtractor.cs +++ b/RecursiveExtractor/Extractors/ArcExtractor.cs @@ -29,11 +29,11 @@ public ArcExtractor(Extractor context) /// public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - ArcReader? arcReader = null; + IReader? arcReader = null; try { fileEntry.Content.Position = 0; - arcReader = ArcReader.Open(fileEntry.Content, new ReaderOptions() + arcReader = ArcReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); @@ -103,11 +103,11 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra /// public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - ArcReader? arcReader = null; + IReader? arcReader = null; try { fileEntry.Content.Position = 0; - arcReader = ArcReader.Open(fileEntry.Content, new ReaderOptions() + arcReader = ArcReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); diff --git a/RecursiveExtractor/Extractors/ArjExtractor.cs b/RecursiveExtractor/Extractors/ArjExtractor.cs index 4c1f324a..bb836879 100644 --- a/RecursiveExtractor/Extractors/ArjExtractor.cs +++ b/RecursiveExtractor/Extractors/ArjExtractor.cs @@ -29,11 +29,11 @@ public ArjExtractor(Extractor context) /// public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - ArjReader? arjReader = null; + IReader? arjReader = null; try { fileEntry.Content.Position = 0; - arjReader = ArjReader.Open(fileEntry.Content, new ReaderOptions() + arjReader = ArjReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); @@ -100,11 +100,11 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra /// public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - ArjReader? arjReader = null; + IReader? arjReader = null; try { fileEntry.Content.Position = 0; - arjReader = ArjReader.Open(fileEntry.Content, new ReaderOptions() + arjReader = ArjReader.OpenReader(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); diff --git a/RecursiveExtractor/Extractors/BZip2Extractor.cs b/RecursiveExtractor/Extractors/BZip2Extractor.cs index 60ff5242..0276b403 100644 --- a/RecursiveExtractor/Extractors/BZip2Extractor.cs +++ b/RecursiveExtractor/Extractors/BZip2Extractor.cs @@ -40,7 +40,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra var failed = false; try { - using var bzipStream = new BZip2Stream(fileEntry.Content, CompressionMode.Decompress, false); + using var bzipStream = BZip2Stream.Create(fileEntry.Content, CompressionMode.Decompress, false, leaveOpen: false); await bzipStream.CopyToAsync(fs); } catch (Exception e) @@ -99,7 +99,7 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti try { - using var bzipStream = new BZip2Stream(fileEntry.Content, CompressionMode.Decompress, false); + using var bzipStream = BZip2Stream.Create(fileEntry.Content, CompressionMode.Decompress, false, leaveOpen: false); bzipStream.CopyTo(fs); } catch (Exception e) @@ -139,4 +139,4 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti } } } -} \ No newline at end of file +} diff --git a/RecursiveExtractor/Extractors/DiscCommon.cs b/RecursiveExtractor/Extractors/DiscCommon.cs index d15c8fcf..8662566e 100644 --- a/RecursiveExtractor/Extractors/DiscCommon.cs +++ b/RecursiveExtractor/Extractors/DiscCommon.cs @@ -1,4 +1,5 @@ using DiscUtils; +using DiscUtils.Iso9660; using System; using System.Collections.Generic; using System.Collections.ObjectModel; @@ -14,6 +15,199 @@ public static class DiscCommon { private static readonly NLog.Logger Logger = NLog.LogManager.GetCurrentClassLogger(); + /// + /// Tries to extract file metadata from a DiscUtils file system entry. + /// For file systems implementing (such as Ext, Xfs, Btrfs, HfsPlus, + /// and ISO 9660 images via CDReader when RockRidge is the active variant; see + /// for images that also carry Joliet), + /// returns permissions, UID, and GID. + /// For file systems implementing (such as NTFS, FAT, WIM, and ISO 9660), + /// returns Windows file attributes. + /// For file systems implementing (such as NTFS and WIM), + /// also returns the security descriptor in SDDL format when the file system provides one. + /// The lists above are not exhaustive; support is determined by the interfaces the file system implements. + /// Returns null for file systems that support none of these interfaces. + /// + /// The opened disc file system + /// Path of the file within the file system + /// Populated or null when not available + internal static FileEntryMetadata? TryGetFileMetadata(DiscFileSystem fs, string filePath) + { + FileEntryMetadata? metadata = null; + + if (fs is IUnixFileSystem unixFs && SupportsUnixMetadata(fs)) + { + metadata = ApplyUnixMetadata(unixFs, filePath, metadata); + } + + if (fs is IDosFileSystem dosFs) + { + try + { + var winInfo = dosFs.GetFileStandardInformation(filePath); + metadata ??= new FileEntryMetadata(); + metadata.FileAttributes = winInfo.FileAttributes; + } + catch (Exception e) + { + Logger.Debug(e, "Could not retrieve DOS file attributes for {0}", filePath); + } + } + + if (fs is IWindowsFileSystem windowsFs) + { + try + { + var securityDescriptor = windowsFs.GetSecurity(filePath); + var sddl = securityDescriptor?.GetSddlForm( + DiscUtils.Core.WindowsSecurity.AccessControl.AccessControlSections.All); + if (!string.IsNullOrEmpty(sddl)) + { + metadata ??= new FileEntryMetadata(); + metadata.SecurityDescriptorSddl = sddl; + } + } + catch (Exception e) + { + Logger.Debug(e, "Could not retrieve security descriptor for {0}", filePath); + } + } + + return metadata; + } + + /// + /// Reads the Unix mode, UID and GID for a file and applies them to , + /// creating it when the caller does not have one yet. + /// + /// The opened Unix file system + /// Path of the file within the file system + /// The metadata to populate, or null to create one on demand + /// The populated metadata, or the value passed in when the read failed + private static FileEntryMetadata? ApplyUnixMetadata(IUnixFileSystem fs, string filePath, FileEntryMetadata? metadata) + { + try + { + var info = fs.GetUnixFileInfo(filePath); + metadata ??= new FileEntryMetadata(); + metadata.Mode = (long)info.Permissions; + metadata.Uid = info.UserId; + metadata.Gid = info.GroupId; + } + catch (Exception e) + { + Logger.Debug(e, "Could not retrieve Unix metadata for {0}", filePath); + } + + return metadata; + } + + /// + /// Determines whether a file system that implements can actually + /// return Unix metadata. CDReader implements the interface unconditionally but only exposes + /// Unix information when the active ISO 9660 variant is RockRidge, and throws for every other + /// variant. Checking up front avoids throwing and catching an exception for every file in an image. + /// + private static bool SupportsUnixMetadata(DiscFileSystem fs) + => fs is not CDReader cdReader || cdReader.ActiveVariant == Iso9660Variant.RockRidge; + + /// + /// Pre-collects metadata for all files while the file system is still open. + /// Used by extractors (e.g., ISO) where the file system is disposed before files are processed. + /// + /// + /// This does not throw. An entry whose metadata cannot be read is logged and skipped, so a problem + /// reading metadata never causes the archive itself to be reported as unreadable. + /// + /// The opened disc file system + /// The file entries to collect metadata for + /// A dictionary mapping file paths to metadata, or null if the file system does not support metadata + internal static Dictionary? CollectMetadata(DiscFileSystem fs, DiscFileInfo[] fileInfos) + { + if (fs is not IUnixFileSystem && fs is not IDosFileSystem && fs is not IWindowsFileSystem) + { + return null; + } + + var result = new Dictionary(); + foreach (var fi in fileInfos) + { + string? fullName = null; + try + { + fullName = fi.FullName; + var metadata = TryGetFileMetadata(fs, fullName); + if (metadata != null) + { + result[fullName] = metadata; + } + } + catch (Exception e) + { + Logger.Debug(e, "Could not collect metadata for {0}", fullName ?? "an unnamed entry"); + } + } + return result; + } + + /// + /// Pre-collects metadata for the files of an ISO 9660 image while the reader is still open. + /// + /// + /// CDReader is opened with Joliet given priority over RockRidge, and DiscUtils only parses + /// SUSP records for the variant it activates. On an image built with both extensions (for example + /// mkisofs -J -R) the Joliet tree wins, and the RockRidge mode, UID and GID would be lost. + /// When the active variant is not RockRidge, a second reader that skips Joliet is opened to recover + /// them. Entries are matched by path; the two directory trees normally agree on names, and an entry + /// that does not match simply keeps whatever the active tree provided. + /// Like , this does not throw. + /// + /// The opened ISO 9660 reader + /// The file entries to collect metadata for + /// The stream the reader was opened over, used for the RockRidge fallback + /// A dictionary mapping file paths to metadata + internal static Dictionary? CollectIsoMetadata(CDReader cd, DiscFileInfo[] fileInfos, Stream isoStream) + { + var metadataByPath = CollectMetadata(cd, fileInfos); + + if (metadataByPath == null || cd.ActiveVariant == Iso9660Variant.RockRidge) + { + return metadataByPath; + } + + try + { + using var rockRidgeReader = new CDReader(isoStream, false); + if (rockRidgeReader.ActiveVariant != Iso9660Variant.RockRidge) + { + return metadataByPath; + } + + foreach (var fi in rockRidgeReader.Root.GetFiles("*.*", SearchOption.AllDirectories)) + { + string? fullName = null; + try + { + fullName = fi.FullName; + if (metadataByPath.TryGetValue(fullName, out var metadata)) + { + ApplyUnixMetadata(rockRidgeReader, fullName, metadata); + } + } + catch (Exception e) + { + Logger.Debug(e, "Could not collect RockRidge metadata for {0}", fullName ?? "an unnamed entry"); + } + } + } + catch (Exception e) + { + Logger.Debug(e, "Could not read RockRidge metadata from an ISO with active variant {0}", cd.ActiveVariant); + } + + return metadataByPath; + } + /// /// Dump the FileEntries from a Logical Volume asynchronously /// @@ -59,6 +253,7 @@ public static async IAsyncEnumerable DumpLogicalVolumeAsync(LogicalVo if (fileStream != null && fi != null) { var newFileEntry = await FileEntry.FromStreamAsync($"{volume.Identity}{Path.DirectorySeparatorChar}{fi.FullName}", fileStream, parent, fi.CreationTime, fi.LastWriteTime, fi.LastAccessTime, memoryStreamCutoff: options.MemoryStreamCutoff).ConfigureAwait(false); + newFileEntry.Metadata = TryGetFileMetadata(fs, file); if (options.Recurse || topLevel) { await foreach (var entry in Context.ExtractAsync(newFileEntry, options, governor, false)) @@ -124,6 +319,7 @@ public static IEnumerable DumpLogicalVolume(LogicalVolumeInfo volume, if (fileStream != null) { var newFileEntry = new FileEntry($"{volume.Identity}{Path.DirectorySeparatorChar}{file}", fileStream, parent, false, creation, modification, access, memoryStreamCutoff: options.MemoryStreamCutoff); + newFileEntry.Metadata = TryGetFileMetadata(fs, file); if (options.Recurse || topLevel) { foreach (var extractedFile in Context.Extract(newFileEntry, options, governor, false)) diff --git a/RecursiveExtractor/Extractors/IsoExtractor.cs b/RecursiveExtractor/Extractors/IsoExtractor.cs index 3756b319..f3455df7 100644 --- a/RecursiveExtractor/Extractors/IsoExtractor.cs +++ b/RecursiveExtractor/Extractors/IsoExtractor.cs @@ -31,11 +31,13 @@ public IsoExtractor(Extractor context) public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { DiscUtils.DiscFileInfo[]? entries = null; + Dictionary? metadataByPath = null; var failed = false; try { using var cd = new CDReader(fileEntry.Content, true); entries = cd.Root.GetFiles("*.*", SearchOption.AllDirectories).ToArray(); + metadataByPath = DiscCommon.CollectIsoMetadata(cd, entries, fileEntry.Content); } catch (Exception e) { @@ -69,6 +71,10 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra { var name = fileInfo.FullName.Replace('/', Path.DirectorySeparatorChar); var newFileEntry = await FileEntry.FromStreamAsync(name, stream, fileEntry, fileInfo.CreationTime, fileInfo.LastWriteTime, fileInfo.LastAccessTime, memoryStreamCutoff: options.MemoryStreamCutoff).ConfigureAwait(false); + if (metadataByPath != null && metadataByPath.TryGetValue(fileInfo.FullName, out var entryMetadata)) + { + newFileEntry.Metadata = entryMetadata; + } if (options.Recurse || topLevel) { await foreach (var entry in Context.ExtractAsync(newFileEntry, options, governor, false)) @@ -92,11 +98,13 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { DiscUtils.DiscFileInfo[]? entries = null; + Dictionary? metadataByPath = null; var failed = false; try { using var cd = new CDReader(fileEntry.Content, true); entries = cd.Root.GetFiles("*.*", SearchOption.AllDirectories).ToArray(); + metadataByPath = DiscCommon.CollectIsoMetadata(cd, entries, fileEntry.Content); } catch(Exception e) { @@ -130,6 +138,10 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti { var name = fileInfo.FullName.Replace('/', Path.DirectorySeparatorChar); var newFileEntry = new FileEntry(name, stream, fileEntry, createTime: file.CreationTime, modifyTime: file.LastWriteTime, accessTime: file.LastAccessTime, memoryStreamCutoff: options.MemoryStreamCutoff); + if (metadataByPath != null && metadataByPath.TryGetValue(fileInfo.FullName, out var entryMetadata)) + { + newFileEntry.Metadata = entryMetadata; + } if (options.Recurse || topLevel) { foreach (var entry in Context.Extract(newFileEntry, options, governor, false)) diff --git a/RecursiveExtractor/Extractors/RarExtractor.cs b/RecursiveExtractor/Extractors/RarExtractor.cs index 829b3d30..85d7ff80 100644 --- a/RecursiveExtractor/Extractors/RarExtractor.cs +++ b/RecursiveExtractor/Extractors/RarExtractor.cs @@ -31,7 +31,7 @@ public RarExtractor(Extractor context) try { - rarArchive = RarArchive.Open(fileEntry.Content); + rarArchive = (RarArchive)RarArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { LeaveStreamOpen = true }); // Test for invalid archives. This will throw invalidformatexception var t = rarArchive.IsSolid; if (rarArchive.Entries.Any(x => x.IsEncrypted)) @@ -66,7 +66,7 @@ public RarExtractor(Extractor context) try { fileEntry.Content.Position = 0; - rarArchive = RarArchive.Open(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { Password = password, LookForHeader = true }); + rarArchive = (RarArchive)RarArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { Password = password, LookForHeader = true, LeaveStreamOpen = true }); var byt = new byte[1]; var encryptedEntry = rarArchive.Entries.FirstOrDefault(x => x is { IsEncrypted: true, Size: > 0 }); // Justification for !: Because we use FirstOrDefault encryptedEntry may be null, but we have a catch below for it @@ -197,4 +197,4 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti } } } -} \ No newline at end of file +} diff --git a/RecursiveExtractor/Extractors/SevenZipExtractor.cs b/RecursiveExtractor/Extractors/SevenZipExtractor.cs index 40f6b68e..869da19d 100644 --- a/RecursiveExtractor/Extractors/SevenZipExtractor.cs +++ b/RecursiveExtractor/Extractors/SevenZipExtractor.cs @@ -83,7 +83,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra var needsPassword = false; try { - sevenZipArchive = SevenZipArchive.Open(fileEntry.Content); + sevenZipArchive = (SevenZipArchive)SevenZipArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { LeaveStreamOpen = true }); if (sevenZipArchive.Entries.Where(x => !x.IsDirectory).Any(x => x.IsEncrypted)) { needsPassword = true; @@ -114,7 +114,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra try { fileEntry.Content.Position = 0; - sevenZipArchive = SevenZipArchive.Open(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { Password = password }); + sevenZipArchive = (SevenZipArchive)SevenZipArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { Password = password, LeaveStreamOpen = true }); // When filenames are encrypted we can't access the size of individual files // But if we can accesss the total uncompressed size we have the right password try @@ -197,4 +197,4 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti } } } -} \ No newline at end of file +} diff --git a/RecursiveExtractor/Extractors/TarExtractor.cs b/RecursiveExtractor/Extractors/TarExtractor.cs index 4d5a2dc5..662f8597 100644 --- a/RecursiveExtractor/Extractors/TarExtractor.cs +++ b/RecursiveExtractor/Extractors/TarExtractor.cs @@ -29,7 +29,7 @@ public TarExtractor(Extractor context) /// public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - using TarArchive archive = TarArchive.Open(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() + using TarArchive archive = (TarArchive)TarArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { LeaveStreamOpen = true }); @@ -62,6 +62,8 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra catch (Exception e) { Logger.Debug(Extractor.FAILED_PARSING_ERROR_MESSAGE_STRING, ArchiveFileType.TAR, fileEntry.FullPath, tarEntry.Key, e.GetType()); + fs?.Dispose(); + continue; } var name = tarEntry.Key?.Replace('/', Path.DirectorySeparatorChar); if (string.IsNullOrEmpty(name)) @@ -101,7 +103,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra /// public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { - using TarArchive archive = TarArchive.Open(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() + using TarArchive archive = (TarArchive)TarArchive.OpenArchive(fileEntry.Content, new SharpCompress.Readers.ReaderOptions() { LeaveStreamOpen = true }); @@ -135,6 +137,8 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti catch (Exception e) { Logger.Debug(Extractor.FAILED_PARSING_ERROR_MESSAGE_STRING, ArchiveFileType.TAR, fileEntry.FullPath, tarEntry.Key, e.GetType()); + fs?.Dispose(); + continue; } var name = tarEntry.Key?.Replace('/', Path.DirectorySeparatorChar); if (string.IsNullOrEmpty(name)) @@ -220,4 +224,4 @@ internal TarEntryCollectionEnumerator(ICollection entries, stri } } } -} \ No newline at end of file +} diff --git a/RecursiveExtractor/Extractors/UdfExtractor.cs b/RecursiveExtractor/Extractors/UdfExtractor.cs index efc016c8..27d1bb53 100644 --- a/RecursiveExtractor/Extractors/UdfExtractor.cs +++ b/RecursiveExtractor/Extractors/UdfExtractor.cs @@ -31,11 +31,13 @@ public UdfExtractor(Extractor context) public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { DiscUtils.DiscFileInfo[]? entries = null; + Dictionary? metadataByPath = null; var failed = false; try { using var cd = new UdfReader(fileEntry.Content); entries = cd.Root.GetFiles("*.*", SearchOption.AllDirectories).ToArray(); + metadataByPath = DiscCommon.CollectMetadata(cd, entries); } catch (Exception e) { @@ -69,6 +71,10 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra { var name = fileInfo.FullName.Replace('/', Path.DirectorySeparatorChar); var newFileEntry = await FileEntry.FromStreamAsync(name, stream, fileEntry, fileInfo.CreationTime, fileInfo.LastWriteTime, fileInfo.LastAccessTime, memoryStreamCutoff: options.MemoryStreamCutoff).ConfigureAwait(false); + if (metadataByPath != null && metadataByPath.TryGetValue(fileInfo.FullName, out var entryMetadata)) + { + newFileEntry.Metadata = entryMetadata; + } if (options.Recurse || topLevel) { await foreach (var entry in Context.ExtractAsync(newFileEntry, options, governor, false)) @@ -92,11 +98,13 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions options, ResourceGovernor governor, bool topLevel = true) { DiscUtils.DiscFileInfo[]? entries = null; + Dictionary? metadataByPath = null; var failed = false; try { using var cd = new UdfReader(fileEntry.Content); entries = cd.Root.GetFiles("*.*", SearchOption.AllDirectories).ToArray(); + metadataByPath = DiscCommon.CollectMetadata(cd, entries); } catch(Exception e) { @@ -130,6 +138,10 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti { var name = fileInfo.FullName.Replace('/', Path.DirectorySeparatorChar); var newFileEntry = new FileEntry(name, stream, fileEntry, createTime: file.CreationTime, modifyTime: file.LastWriteTime, accessTime: file.LastAccessTime, memoryStreamCutoff: options.MemoryStreamCutoff); + if (metadataByPath != null && metadataByPath.TryGetValue(fileInfo.FullName, out var entryMetadata)) + { + newFileEntry.Metadata = entryMetadata; + } if (options.Recurse || topLevel) { foreach (var entry in Context.Extract(newFileEntry, options, governor, false)) diff --git a/RecursiveExtractor/Extractors/WimExtractor.cs b/RecursiveExtractor/Extractors/WimExtractor.cs index fe7fd3df..4cf9d516 100644 --- a/RecursiveExtractor/Extractors/WimExtractor.cs +++ b/RecursiveExtractor/Extractors/WimExtractor.cs @@ -58,6 +58,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra { var name = file.Replace('\\', Path.DirectorySeparatorChar); var newFileEntry = await FileEntry.FromStreamAsync($"{image.FriendlyName}{Path.DirectorySeparatorChar}{name}", stream, fileEntry, memoryStreamCutoff: options.MemoryStreamCutoff).ConfigureAwait(false); + newFileEntry.Metadata = DiscCommon.TryGetFileMetadata(image, file); if (options.Recurse || topLevel) { @@ -128,6 +129,7 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti var name = file.Replace('\\', Path.DirectorySeparatorChar); var newFileEntry = new FileEntry($"{image.FriendlyName}{Path.DirectorySeparatorChar}{name}", stream, fileEntry, memoryStreamCutoff: options.MemoryStreamCutoff); + newFileEntry.Metadata = DiscCommon.TryGetFileMetadata(image, file); if (options.Recurse || topLevel) { foreach (var extractedFile in Context.Extract(newFileEntry, options, governor, false)) diff --git a/RecursiveExtractor/Extractors/ZipExtractor.cs b/RecursiveExtractor/Extractors/ZipExtractor.cs index 92dfc040..5140a60d 100644 --- a/RecursiveExtractor/Extractors/ZipExtractor.cs +++ b/RecursiveExtractor/Extractors/ZipExtractor.cs @@ -37,7 +37,7 @@ public ZipExtractor(Extractor context) { // Create a new archive instance with the password to test it fileEntry.Content.Position = 0; - using var testArchive = ZipArchive.Open(fileEntry.Content, new ReaderOptions() + using var testArchive = (ZipArchive)ZipArchive.OpenArchive(fileEntry.Content, new ReaderOptions() { Password = password, LeaveStreamOpen = true @@ -73,7 +73,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra try { fileEntry.Content.Position = 0; - zipArchive = ZipArchive.Open(fileEntry.Content, new ReaderOptions() + zipArchive = (ZipArchive)ZipArchive.OpenArchive(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); @@ -104,7 +104,7 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra // Recreate archive with password zipArchive.Dispose(); fileEntry.Content.Position = 0; - zipArchive = ZipArchive.Open(fileEntry.Content, new ReaderOptions() + zipArchive = (ZipArchive)ZipArchive.OpenArchive(fileEntry.Content, new ReaderOptions() { Password = foundPassword, LeaveStreamOpen = true @@ -141,11 +141,12 @@ public async IAsyncEnumerable ExtractAsync(FileEntry fileEntry, Extra catch (Exception e) { Logger.Debug(Extractor.FAILED_PARSING_ERROR_MESSAGE_STRING, ArchiveFileType.ZIP, fileEntry.FullPath, zipEntry.Key, e.GetType()); + target?.Dispose(); + continue; } - target ??= new MemoryStream(); var name = zipEntry.Key?.Replace('/', Path.DirectorySeparatorChar) ?? ""; - var newFileEntry = new FileEntry(name, target, fileEntry, modifyTime: zipEntry.LastModifiedTime, memoryStreamCutoff: options.MemoryStreamCutoff); + var newFileEntry = new FileEntry(name, target, fileEntry, modifyTime: zipEntry.LastModifiedTime, memoryStreamCutoff: options.MemoryStreamCutoff, passthroughStream: true); try { @@ -198,7 +199,7 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti try { fileEntry.Content.Position = 0; - zipArchive = ZipArchive.Open(fileEntry.Content, new ReaderOptions() + zipArchive = (ZipArchive)ZipArchive.OpenArchive(fileEntry.Content, new ReaderOptions() { LeaveStreamOpen = true }); @@ -229,7 +230,7 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti // Recreate archive with password zipArchive.Dispose(); fileEntry.Content.Position = 0; - zipArchive = ZipArchive.Open(fileEntry.Content, new ReaderOptions() + zipArchive = (ZipArchive)ZipArchive.OpenArchive(fileEntry.Content, new ReaderOptions() { Password = foundPassword, LeaveStreamOpen = true @@ -254,7 +255,7 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti governor.CheckResourceGovernor(zipEntry.Size); - using var fs = StreamFactory.GenerateAppropriateBackingStream(options, zipEntry.Size); + var fs = StreamFactory.GenerateAppropriateBackingStream(options, zipEntry.Size); try { @@ -264,10 +265,12 @@ public IEnumerable Extract(FileEntry fileEntry, ExtractorOptions opti catch (Exception e) { Logger.Debug(Extractor.FAILED_PARSING_ERROR_MESSAGE_STRING, ArchiveFileType.ZIP, fileEntry.FullPath, zipEntry.Key, e.GetType()); + fs?.Dispose(); + continue; } var name = zipEntry.Key?.Replace('/', Path.DirectorySeparatorChar) ?? ""; - var newFileEntry = new FileEntry(name, fs, fileEntry, modifyTime: zipEntry.LastModifiedTime, memoryStreamCutoff: options.MemoryStreamCutoff); + var newFileEntry = new FileEntry(name, fs, fileEntry, passthroughStream: true, modifyTime: zipEntry.LastModifiedTime, memoryStreamCutoff: options.MemoryStreamCutoff); try { @@ -326,7 +329,7 @@ private async IAsyncEnumerable YieldNonIndexedEntriesAsync( IReader? forwardReader = null; try { - forwardReader = ReaderFactory.Open(parentEntry.Content, new ReaderOptions { LeaveStreamOpen = true }); + forwardReader = ReaderFactory.OpenReader(parentEntry.Content, new ReaderOptions { LeaveStreamOpen = true }); } catch (Exception ex) { @@ -410,7 +413,7 @@ private IEnumerable YieldNonIndexedEntries( IReader? forwardReader = null; try { - forwardReader = ReaderFactory.Open(parentEntry.Content, new ReaderOptions { LeaveStreamOpen = true }); + forwardReader = ReaderFactory.OpenReader(parentEntry.Content, new ReaderOptions { LeaveStreamOpen = true }); } catch (Exception ex) { diff --git a/RecursiveExtractor/FileEntry.cs b/RecursiveExtractor/FileEntry.cs index c5427fdd..ca74e5a0 100644 --- a/RecursiveExtractor/FileEntry.cs +++ b/RecursiveExtractor/FileEntry.cs @@ -40,8 +40,10 @@ public FileEntry(string name, Stream inputStream, FileEntry? parent = null, bool ModifyTime = modifyTime ?? DateTime.MinValue; AccessTime = accessTime ?? DateTime.MinValue; - // Sanitize so its safe to use with Path APIs - string sanitizedName = SanitizePath(name); + // Sanitize traversal and absolute paths first while path structure is intact + string zipSlipSafeName = ZipSlipSanitize(name); + // Then replace any remaining OS-invalid characters + string sanitizedName = SanitizePath(zipSlipSafeName); Name = Path.GetFileName(sanitizedName); // If parent is null use the provided name as the FullPath @@ -297,25 +299,76 @@ public static async Task FromStreamAsync(string name, Stream content, } /// - /// Replace .. for ZipSlip and remove any doubled up directory separators as a result - https://snyk.io/research/zip-slip-vulnerability + /// Sanitize paths to prevent ZipSlip (directory traversal) and absolute path traversal. + /// Strips leading directory separators and drive letter roots, removes ".." sequences, + /// and collapses resulting double separators. See https://snyk.io/research/zip-slip-vulnerability /// /// The path to sanitize /// The string to replace .. with - /// A path without ZipSlip + /// A relative path safe from directory traversal [Pure] - private static string ZipSlipSanitize(string fullPath, string replacement = "") + internal static string ZipSlipSanitize(string fullPath, string replacement = "") { - if (fullPath.Contains("..")) + // Normalize all separators to the OS-native separator + fullPath = fullPath.Replace('\\', Path.DirectorySeparatorChar).Replace('/', Path.DirectorySeparatorChar); + + // Strip drive letter roots and leading separators. Must run before and after ".." + // removal because ".." removal can expose a new drive root (e.g. "../C:\file" → "C:\file"). + fullPath = StripLeadingRootComponents(fullPath); + + if (fullPath.Length > 0) { - fullPath = fullPath.Replace("..", replacement); - var directorySeparator = Path.DirectorySeparatorChar.ToString(); - var doubleSeparator = $"{directorySeparator},{directorySeparator}"; - while (fullPath.Contains(doubleSeparator)) + var separator = Path.DirectorySeparatorChar; + var segments = fullPath.Split(separator); + + for (var i = 0; i < segments.Length; i++) { - fullPath = fullPath.Replace(doubleSeparator, $"{directorySeparator}"); + // Replace traversal segments (".." and ".") only when they are whole segments + if (segments[i] == ".." || segments[i] == ".") + { + segments[i] = replacement; + } } + + // Rebuild the path from non-empty segments + fullPath = string.Join(separator.ToString(), segments.Where(s => !string.IsNullOrEmpty(s))); } + fullPath = StripLeadingRootComponents(fullPath); + + return fullPath; + } + + /// + /// Repeatedly strips Windows drive letter roots (e.g. "C:\") and leading directory + /// separators until the path is stable. A single pass is insufficient for crafted paths + /// like "D:\C:\" where stripping "D:" exposes a new root "C:\". + /// + private static string StripLeadingRootComponents(string fullPath) + { + bool changed; + do + { + changed = false; + + // Strip Windows drive letter roots (e.g., "C:\", "D:/") but not relative names like "a:file.txt" + if (fullPath.Length >= 3 + && char.IsLetter(fullPath[0]) + && fullPath[1] == ':' + && fullPath[2] == Path.DirectorySeparatorChar) + { + fullPath = fullPath.Substring(2); + changed = true; + } + + // Strip leading directory separators to prevent absolute path traversal + while (fullPath.Length > 0 && fullPath[0] == Path.DirectorySeparatorChar) + { + fullPath = fullPath.Substring(1); + changed = true; + } + } + while (changed); return fullPath; } diff --git a/RecursiveExtractor/FileEntryMetadata.cs b/RecursiveExtractor/FileEntryMetadata.cs index 4254c442..e78618fb 100644 --- a/RecursiveExtractor/FileEntryMetadata.cs +++ b/RecursiveExtractor/FileEntryMetadata.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft Corporation. Licensed under the MIT License. +using System.IO; + namespace Microsoft.CST.RecursiveExtractor { /// @@ -43,5 +45,19 @@ public class FileEntryMetadata /// Null if not available from the archive format. /// public long? Gid { get; set; } + + /// + /// The Windows file attributes (e.g., ReadOnly, Hidden, System, Archive). + /// Available for disc image file systems that expose DOS attributes, such as NTFS, FAT, WIM, and ISO 9660. + /// Null if not available from the archive format. + /// + public FileAttributes? FileAttributes { get; set; } + + /// + /// The Windows security descriptor in SDDL (Security Descriptor Definition Language) format. + /// Available for disc image file systems that expose Windows security descriptors, such as NTFS and WIM. + /// Null if not available from the archive format, or if the file system did not record one for this file. + /// + public string? SecurityDescriptorSddl { get; set; } } } diff --git a/RecursiveExtractor/RecursiveExtractor.csproj b/RecursiveExtractor/RecursiveExtractor.csproj index 1b6bd271..53fd4f35 100644 --- a/RecursiveExtractor/RecursiveExtractor.csproj +++ b/RecursiveExtractor/RecursiveExtractor.csproj @@ -31,25 +31,25 @@ - - - - - - - - - - - - - + + + + + + + + + + + + + - - - - - + + + + + diff --git a/nuget.config b/nuget.config index 227ad0ce..ba47b6aa 100644 --- a/nuget.config +++ b/nuget.config @@ -1,7 +1,7 @@ - + - \ No newline at end of file +