fix: reject archive entries that extract outside the output directory (#2720)

## 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.
This commit is contained in:
Kamron Batman 2026-10-18 20:06:23 -07:00 • committed by GitHub
parent 0f89870793
commit 078be52777
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 53 additions and 1 deletions

View file

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