From 35f5790f3ffbdf6ac2c8525238be47fe19bd538f Mon Sep 17 00:00:00 2001 From: Ben Davis <45952064+bmdavis419@users.noreply.github.com> Date: Tue, 13 Oct 2026 04:32:22 -0700 Subject: [PATCH] BUGFIX: Cap total uncompressed size in data-tar validation ValidateDataTar capped member count but not summed regular-file size, so a small crafted .tx9 could pass validation and expand without bound when tx9 import extracted it onto a volume. Reject archives past 100 GiB total (mirroring guest/hermes-state MAX_UNCOMPRESSED) and reject negative member sizes. Includes cumulative and boundary regression tests and marks the 2026-08-27 audit item addressed. --- docs/audit-2026-08-27.md | 12 ++++++++++ internal/archive/validator.go | 26 +++++++++++++++++++++ internal/archive/validator_test.go | 36 ++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+) diff --git a/docs/audit-2026-08-27.md b/docs/audit-2026-08-27.md index 2b43e4b..aa05993 100644 --- a/docs/audit-2026-08-27.md +++ b/docs/audit-2026-08-27.md @@ -141,6 +141,18 @@ files without a live SQLite read transaction, so even a successful log export is not guaranteed to represent one consistent point in time. Log redaction cannot recognize every secret or encoding. +**Follow-up (archive size ceiling): addressed.** The Go data-tar validator +(`ValidateDataTar`) capped member count but not total payload size, so a +small crafted `.tx9` could pass validation and then expand without bound +when `tx9 import` extracted it onto a volume. It now rejects archives whose +regular-file members sum past 100 GiB — the same decompression-bomb safety +ceiling `guest/hermes-state` already enforced (`MAX_UNCOMPRESSED`) — +before any extraction writes happen, and rejects negative member sizes +instead of letting them offset the total. This is a safety ceiling, not +the storage-quota/retention policy in the table below: that decision +(per-box budgets, history retention) remains open, as do large +logs/snapshots and snapshot consistency above. + ## Decisions left open | Decision | Recommended direction and consequence | diff --git a/internal/archive/validator.go b/internal/archive/validator.go index d190589..7e89d66 100644 --- a/internal/archive/validator.go +++ b/internal/archive/validator.go @@ -29,6 +29,16 @@ import ( // to generate a million-entry archive. var maxArchiveMembers = 1_000_000 +// maxArchiveUncompressedBytes hard-caps the summed size of regular-file +// members ValidateDataTar will accept, so a small crafted .tx9 whose +// gzip stream expands without bound cannot pass validation and then +// fill the target volume (or backup host disk) during extraction. It +// mirrors guest/hermes-state's MAX_UNCOMPRESSED safety ceiling — a +// decompression-bomb guard, not a per-box storage quota (volume budgets +// stay advisory; see internal/box Resources). A var so tests can lower +// it without generating a huge archive. +var maxArchiveUncompressedBytes int64 = 100 * 1024 * 1024 * 1024 + // Match the Linux filesystem's symlink traversal bound instead of allowing // link-resolution work to grow with the total number of archive members. const maxArchiveLinkHops = 40 @@ -44,6 +54,7 @@ func ValidateDataTar(r io.Reader) error { // members = archive.getmembers() — the Python version reads the whole // member list eagerly before doing anything else. var headers []*tar.Header + var totalUncompressed int64 for { hdr, err := tr.Next() if err == io.EOF { @@ -53,6 +64,21 @@ func ValidateDataTar(r io.Reader) error { gz.Close() return fmt.Errorf("archive is corrupt: %w", err) } + // Tar header sizes are the exact byte counts extraction will + // write for regular files, so summing them here bounds the + // restore before any volume write happens. Reject negative + // sizes outright instead of letting them offset the total. + if hdr.Typeflag == tar.TypeReg || hdr.Typeflag == tar.TypeRegA { + if hdr.Size < 0 { + gz.Close() + return fmt.Errorf("archive is corrupt: member %q has negative size %d", hdr.Name, hdr.Size) + } + if hdr.Size > maxArchiveUncompressedBytes-totalUncompressed { + gz.Close() + return fmt.Errorf("archive uncompressed size exceeds the %d byte safety limit", maxArchiveUncompressedBytes) + } + totalUncompressed += hdr.Size + } headers = append(headers, hdr) // A crafted archive with millions of tiny entries would otherwise // exhaust memory (the entire header slice is buffered) before any diff --git a/internal/archive/validator_test.go b/internal/archive/validator_test.go index 6dc02e4..5731d1e 100644 --- a/internal/archive/validator_test.go +++ b/internal/archive/validator_test.go @@ -346,6 +346,42 @@ func TestValidateDataTar_MemberCapExceeded(t *testing.T) { } } +// TestValidateDataTar_UncompressedCapExceeded exercises the total +// uncompressed-size guard without needing a 100 GiB archive: the var +// (not const) is lowered for the duration of the test. The payload is +// split across two files so a per-member check cannot pass it. +func TestValidateDataTar_UncompressedCapExceeded(t *testing.T) { + orig := maxArchiveUncompressedBytes + maxArchiveUncompressedBytes = 10 + defer func() { maxArchiveUncompressedBytes = orig }() + + entries := append(validBase(), + file("home/agent/a", "123456"), + file("home/agent/b", "12345"), + ) + err := runValidate(t, entries) + if err == nil { + t.Fatal("expected error for archive exceeding the uncompressed cap") + } + if !strings.Contains(err.Error(), "uncompressed size") { + t.Fatalf("expected an uncompressed-cap error, got: %v", err) + } +} + +func TestValidateDataTar_UncompressedCapBoundary(t *testing.T) { + orig := maxArchiveUncompressedBytes + maxArchiveUncompressedBytes = 11 + defer func() { maxArchiveUncompressedBytes = orig }() + + entries := append(validBase(), + file("home/agent/a", "123456"), + file("home/agent/b", "12345"), + ) + if err := runValidate(t, entries); err != nil { + t.Fatalf("archive exactly at the uncompressed cap should pass, got: %v", err) + } +} + func TestValidateDataTar_TruncatedGzip(t *testing.T) { // Cut the stream in half rather than trimming a few trailing bytes: the // gzip footer alone can be dropped without tripping an error, since the