diff --git a/source/Calamari.Common/Features/Packages/NuGet/NupkgExtractor.cs b/source/Calamari.Common/Features/Packages/NuGet/NupkgExtractor.cs index f92d7f4e2e..e3f72432f6 100644 --- a/source/Calamari.Common/Features/Packages/NuGet/NupkgExtractor.cs +++ b/source/Calamari.Common/Features/Packages/NuGet/NupkgExtractor.cs @@ -52,6 +52,8 @@ public int Extract(string packageFile, string directory) if (IsExcludedPath(unescapedKey)) continue; + PackageExtractorUtils.ThrowIfPathTraversalAttempted(unescapedKey, directory); + var targetDirectory = Path.Combine(directory, Path.GetDirectoryName(unescapedKey) ?? string.Empty); if (!Directory.Exists(targetDirectory)) diff --git a/source/Calamari.Common/Features/Packages/PackageExtractorUtils.cs b/source/Calamari.Common/Features/Packages/PackageExtractorUtils.cs index de33d91b06..b15637e739 100644 --- a/source/Calamari.Common/Features/Packages/PackageExtractorUtils.cs +++ b/source/Calamari.Common/Features/Packages/PackageExtractorUtils.cs @@ -10,6 +10,22 @@ public static class PackageExtractorUtils { public static void EnsureTargetDirectoryExists(string directory) => Directory.CreateDirectory(directory); + /// + /// Throws if the archive entry key would resolve to a path outside the intended extraction root (zip-slip / path traversal). + /// + public static void ThrowIfPathTraversalAttempted(string entryKey, string extractionDirectory) + { + var extractionRoot = Path.GetFullPath(extractionDirectory); + // Ensure the root ends with a separator so "root" can't be a prefix of "rootEvil" + if (!extractionRoot.EndsWith(Path.DirectorySeparatorChar.ToString(), StringComparison.Ordinal)) + extractionRoot += Path.DirectorySeparatorChar; + + var destination = Path.GetFullPath(Path.Combine(extractionDirectory, entryKey)); + + if (!destination.StartsWith(extractionRoot, StringComparison.OrdinalIgnoreCase)) + throw new InvalidOperationException($"Archive entry '{entryKey}' would extract to '{destination}', which is outside the intended extraction directory '{extractionDirectory}'. The archive may be malicious."); + } + public static ResiliencePipeline CreateIoExceptionRetryStrategy(ILog log) { return new ResiliencePipelineBuilder() diff --git a/source/Calamari.Common/Features/Packages/TarPackageExtractor.cs b/source/Calamari.Common/Features/Packages/TarPackageExtractor.cs index 827f5c7116..fae89e3245 100644 --- a/source/Calamari.Common/Features/Packages/TarPackageExtractor.cs +++ b/source/Calamari.Common/Features/Packages/TarPackageExtractor.cs @@ -33,6 +33,8 @@ public int Extract(string packageFile, string directory) using var reader = TarReader.Open(compressionStream, new ReaderOptions { ArchiveEncoding = new ArchiveEncoding { Default = Encoding.UTF8 } }); while (reader.MoveToNextEntry()) { + if (reader.Entry.Key != null) + PackageExtractorUtils.ThrowIfPathTraversalAttempted(reader.Entry.Key, directory); ProcessEvent(ref files, reader.Entry); ExtractEntry(directory, reader); } diff --git a/source/Calamari.Common/Features/Packages/ZipPackageExtractor.cs b/source/Calamari.Common/Features/Packages/ZipPackageExtractor.cs index 04064c2461..9aea2c6436 100644 --- a/source/Calamari.Common/Features/Packages/ZipPackageExtractor.cs +++ b/source/Calamari.Common/Features/Packages/ZipPackageExtractor.cs @@ -36,6 +36,8 @@ public int Extract(string packageFile, string directory) foreach (var entry in archive.Entries) { + if (entry.Key != null) + PackageExtractorUtils.ThrowIfPathTraversalAttempted(entry.Key, directory); ProcessEvent(ref filesExtracted, entry); ExtractEntry(directory, entry); } diff --git a/source/Calamari.Tests/Fixtures/Integration/Packages/PackageExtractorFixture.cs b/source/Calamari.Tests/Fixtures/Integration/Packages/PackageExtractorFixture.cs index 17486788d0..9c115a36de 100644 --- a/source/Calamari.Tests/Fixtures/Integration/Packages/PackageExtractorFixture.cs +++ b/source/Calamari.Tests/Fixtures/Integration/Packages/PackageExtractorFixture.cs @@ -222,6 +222,33 @@ public void ExtractIgnoresSymbolicLinks() log.StandardOut.Should().ContainMatch("Cannot create symbolic link*"); } + [Test] + [TestCase(typeof(NupkgExtractor), "nupkg", ArchiveType.Zip, CompressionType.Deflate)] + [TestCase(typeof(ZipPackageExtractor), "zip", ArchiveType.Zip, CompressionType.Deflate)] + [TestCase(typeof(TarPackageExtractor), "tar", ArchiveType.Tar, CompressionType.None)] + [TestCase(typeof(TarGzipPackageExtractor), "tar.gz", ArchiveType.Tar, CompressionType.GZip)] + [TestCase(typeof(TarBzipPackageExtractor), "tar.bz2", ArchiveType.Tar, CompressionType.BZip2)] + public void ExtractThrowsWhenArchiveEntryAttemptsPathTraversal(Type extractorType, string extension, ArchiveType archiveType, CompressionType compressionType) + { + using var tempFolder = TemporaryDirectory.Create(); + var packageFile = Path.Combine(tempFolder.DirectoryPath, $"malicious.{extension}"); + var extractionDir = Path.Combine(tempFolder.DirectoryPath, "extraction"); + Directory.CreateDirectory(extractionDir); + + using (var stream = File.OpenWrite(packageFile)) + using (var writer = WriterFactory.Open(stream, archiveType, new WriterOptions(compressionType) { ArchiveEncoding = new ArchiveEncoding { Default = Encoding.UTF8 } })) + { + var payload = "malicious content"u8.ToArray(); + writer.Write("safe-file.txt", new MemoryStream(payload)); + writer.Write("../traversal.txt", new MemoryStream(payload)); + } + + var extractor = (IPackageExtractor)Activator.CreateInstance(extractorType, ConsoleLog.Instance); + + Assert.Throws(() => extractor.Extract(packageFile, extractionDir)); + Assert.That(File.Exists(Path.Combine(tempFolder.DirectoryPath, "traversal.txt")), Is.False, "Traversal file should not have been written outside the extraction directory"); + } + private string GetFileName(string extension) { return GetFixtureResource("Samples", string.Format("{0}.{1}.{2}", PackageId, PackageVersion, extension));