From 078be52777c417afef6d82e1f301e3f65789d256 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Sun, 18 Oct 2026 20:06:23 -0700 Subject: [PATCH] fix: reject archive entries that extract outside the output directory (#2720) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `ManagedArchive.ExtractTarFromStream` joined each tar entry name directly onto the output directory, so an entry such as `../x`, `sub/../../x` or an absolute path was written outside it. The restore path (`AutoArchive` → `ExtractTarZstd`/`ExtractTarGz`/`ExtractTar`) feeds archives through this method. Each entry now resolves to a full path and must stay under the output root (case-insensitive on Windows). An entry that escapes fails the extraction; the existing `catch` in each `Extract*` method logs it and returns `false`. The input is the server's own backup archives, so this is hardening against a tampered or foreign archive, not something a player can reach. ## Tests - `ExtractTar_EntryOutsideOutputDirectory_IsRejected` (`../escaped.txt`, `sub/../../escaped.txt`) and `ExtractTar_RootedEntry_IsRejected`. All three fail without the fix. - All 18 `ManagedArchiveTests` pass. --- .../Tests/WorldSaves/ManagedArchiveTests.cs | 44 +++++++++++++++++++ .../UOContent/Compression/ManagedArchive.cs | 10 ++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/Projects/UOContent.Tests/Tests/WorldSaves/ManagedArchiveTests.cs b/Projects/UOContent.Tests/Tests/WorldSaves/ManagedArchiveTests.cs index b5a2c0dc9..7103a65c5 100644 --- a/Projects/UOContent.Tests/Tests/WorldSaves/ManagedArchiveTests.cs +++ b/Projects/UOContent.Tests/Tests/WorldSaves/ManagedArchiveTests.cs @@ -311,6 +311,50 @@ public class ManagedArchiveTests : IDisposable Assert.Equal("Plain tar test data", File.ReadAllText(Path.Combine(extractDir, "plain.txt"))); } + [Theory] + [InlineData("../escaped.txt")] + [InlineData("sub/../../escaped.txt")] + public void ExtractTar_EntryOutsideOutputDirectory_IsRejected(string entryName) + { + var archivePath = Path.Combine(_testDir, "traversal.tar"); + + using (var fileStream = new FileStream(archivePath, FileMode.Create)) + using (var tarWriter = new TarWriter(fileStream, TarEntryFormat.Pax, leaveOpen: true)) + { + var entry = new PaxTarEntry(TarEntryType.RegularFile, entryName) + { + DataStream = new MemoryStream("escaped"u8.ToArray()) + }; + tarWriter.WriteEntry(entry); + } + + var extractDir = Path.Combine(_testDir, "out", "extract"); + + Assert.False(ManagedArchive.ExtractTar(archivePath, extractDir)); + Assert.False(File.Exists(Path.Combine(_testDir, "out", "escaped.txt"))); + Assert.False(File.Exists(Path.Combine(_testDir, "escaped.txt"))); + } + + [Fact] + public void ExtractTar_RootedEntry_IsRejected() + { + var archivePath = Path.Combine(_testDir, "rooted.tar"); + var target = Path.Combine(_testDir, "rooted-escaped.txt"); + + using (var fileStream = new FileStream(archivePath, FileMode.Create)) + using (var tarWriter = new TarWriter(fileStream, TarEntryFormat.Pax, leaveOpen: true)) + { + var entry = new PaxTarEntry(TarEntryType.RegularFile, target) + { + DataStream = new MemoryStream("escaped"u8.ToArray()) + }; + tarWriter.WriteEntry(entry); + } + + Assert.False(ManagedArchive.ExtractTar(archivePath, Path.Combine(_testDir, "rooted-extract"))); + Assert.False(File.Exists(target)); + } + [Fact] public void ExtractTarGz_InvalidFile_ReturnsFalse() { diff --git a/Projects/UOContent/Compression/ManagedArchive.cs b/Projects/UOContent/Compression/ManagedArchive.cs index 32142d9e4..4b7e9da2b 100644 --- a/Projects/UOContent/Compression/ManagedArchive.cs +++ b/Projects/UOContent/Compression/ManagedArchive.cs @@ -224,9 +224,17 @@ public static class ManagedArchive { using var tarReader = new TarReader(stream); + var root = Path.TrimEndingDirectorySeparator(Path.GetFullPath(outputDirectory)) + Path.DirectorySeparatorChar; + var comparison = OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; + while (tarReader.GetNextEntry() is { } entry) { - var destPath = Path.Combine(outputDirectory, entry.Name); + // Rooted names and ".." segments would otherwise let the archive write anywhere the process can. + var destPath = Path.GetFullPath(Path.Combine(root, entry.Name)); + if (!destPath.StartsWith(root, comparison)) + { + throw new InvalidDataException($"Archive entry '{entry.Name}' resolves outside '{root}'"); + } var destDir = Path.GetDirectoryName(destPath); if (destDir != null)