fix: bound total decompression under each top-level archive

the limiter bounds what's held *at once*, and each archive gives its bytes back when it's released. So a recursive bomb (42.zip shape) never trips a limit: every level fits on its own, and the scan just runs for effectively forever. Capping each archive at N× its own size wouldn't help either, since every level of such a bomb is high-ratio, so the work still grows about N-fold per level.

- each top-level archive gets one decompression budget of `max(100 × its size, 1 GiB)`, spent by every archive nested under it at any depth
- the budget is attached to the context when the walk enters a top-level archive, so `Extract` picks it up without a signature change
- once it's spent, the archive being extracted is truncated with a "possible decompression bomb" unknown and the walk moves on. The next top-level archive starts fresh

real trees (a war of jars, a layer of tarballs) expand a few times over, well under the ratio.

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
This commit is contained in:
Alex Goodman
2026-09-24 13:23:32 -04:00
parent d27b6ebb1b
commit 81ee189bf4
7 changed files with 114 additions and 1 deletions
+4
View File
@@ -152,6 +152,10 @@ func (r *Resolver) extract(ctx context.Context, format archives.Extractor, conte
r.truncate("extraction stopped at the configured memory or disk limit") r.truncate("extraction stopped at the configured memory or disk limit")
return nil return nil
} }
if errors.Is(err, errBudgetSpent) {
r.truncate("extraction stopped: the archive and those nested in it decompressed to far more than its size (possible decompression bomb)")
return nil
}
return err return err
} }
+29
View File
@@ -5,6 +5,35 @@ import (
"sync" "sync"
) )
// Budget bounds the bytes decompressed under one top-level archive, summed over every archive nested in
// it. Unlike Limits it never refills as archives are released: it bounds work, not what is held at once,
// which is what stops a recursive bomb (42.zip) whose every level fits the limits on its own. A nil
// *Budget admits everything.
//
// ponytail: no lock, since the archive walk is sequential; guard it if the walk ever runs concurrently.
type Budget struct {
remaining int64
}
func NewBudget(bytes int64) *Budget {
return &Budget{remaining: bytes}
}
// take spends n bytes, or reports false (and spends the rest) when fewer than n remain.
func (b *Budget) take(n int64) bool {
if b == nil {
return true
}
if n > b.remaining {
b.remaining = 0
return false
}
b.remaining -= n
return true
}
var errBudgetSpent = errors.New("archive tree decompression budget spent")
// ErrDiskLimitReached reports that placing an archive's content would exceed the disk limit. // ErrDiskLimitReached reports that placing an archive's content would exceed the disk limit.
var ErrDiskLimitReached = errors.New("archive content would exceed the disk limit") var ErrDiskLimitReached = errors.New("archive content would exceed the disk limit")
+3 -1
View File
@@ -35,13 +35,14 @@ type Resolver struct {
Digests []file.Digest Digests []file.Digest
// Truncated reports that the resolver covers only part of the archive, and TruncatedReason says why: // Truncated reports that the resolver covers only part of the archive, and TruncatedReason says why:
// the disk limit stopped extraction early, or an entry was too large to store. // the disk limit or the decompression budget stopped extraction early, or an entry was too large.
Truncated bool Truncated bool
TruncatedReason string TruncatedReason string
fileSystemID string fileSystemID string
archivePath string archivePath string
charge *charge charge *charge
budget *Budget
// content is held in memory while the memory limit admits it and in one spill file once it does // content is held in memory while the memory limit admits it and in one spill file once it does
// not. The file is created on first use, so an archive that stays in memory never touches the // not. The file is created on first use, so an archive that stays in memory never touches the
@@ -92,6 +93,7 @@ func newResolver(ctx context.Context, fileSystemID, archivePath string, charge *
fileSystemID: fileSystemID, fileSystemID: fileSystemID,
archivePath: archivePath, archivePath: archivePath,
charge: charge, charge: charge,
budget: budgetFromContext(ctx),
tempDir: tmpdir.FromContext(ctx), tempDir: tmpdir.FromContext(ctx),
byPath: map[string]*node{"/": {path: "/", isDir: true}}, byPath: map[string]*node{"/": {path: "/", isDir: true}},
} }
+3
View File
@@ -43,6 +43,9 @@ func (r *Resolver) put(b *blob, content io.Reader) error {
for { for {
n, readErr := content.Read(r.chunk) n, readErr := content.Read(r.chunk)
if n > 0 { if n > 0 {
if !r.budget.take(int64(n)) {
return errBudgetSpent
}
if err := r.write(b, r.chunk[:n]); err != nil { if err := r.write(b, r.chunk[:n]); err != nil {
return err return err
} }
+13
View File
@@ -52,6 +52,19 @@ func TraversalFromContext(ctx context.Context) *Traversal {
return t return t
} }
type budgetCtxKey struct{}
// WithBudget sets the decompression budget that Extract spends from, for this archive and every archive
// extracted beneath it.
func WithBudget(ctx context.Context, b *Budget) context.Context {
return context.WithValue(ctx, budgetCtxKey{}, b)
}
func budgetFromContext(ctx context.Context) *Budget {
b, _ := ctx.Value(budgetCtxKey{}).(*Budget)
return b
}
type nestedCatalogingCtxKey struct{} type nestedCatalogingCtxKey struct{}
// WithNestedCataloging marks the context of a scan whose archive cataloger task will extract every // WithNestedCataloging marks the context of a scan whose archive cataloger task will extract every
+21
View File
@@ -184,6 +184,11 @@ func (c *archiveCataloger) processArchive(ctx context.Context, parentResolver fi
started := time.Now() started := time.Now()
if depth == 0 {
// one budget for this archive and everything nested in it, however deep
ctx = archive.WithBudget(ctx, archive.NewBudget(archiveTreeBudget(parentResolver, location)))
}
// extracted files keep the filesystem ID of where the archive was found; the nesting chain is // extracted files keep the filesystem ID of where the archive was found; the nesting chain is
// carried separately as the archive path. The chain names real paths, not the link the archive was // carried separately as the archive path. The chain names real paths, not the link the archive was
// reached by, so it is stable and joins to the archive's package location by prefix. // reached by, so it is stable and joins to the archive's package location by prefix.
@@ -234,6 +239,22 @@ func (c *archiveCataloger) processArchive(ctx context.Context, parentResolver fi
return c.catalog(ctx, extracted, depth+1, builder) return c.catalog(ctx, extracted, depth+1, builder)
} }
// a top-level archive and everything nested in it may decompress to archiveTreeRatio times its size, or
// archiveTreeFloorBytes if that is more. Real trees (a war of jars, a layer of tarballs) expand a few
// times over; a recursive bomb expands by orders of magnitude at every level.
var (
archiveTreeRatio int64 = 100
archiveTreeFloorBytes int64 = 1 << 30
)
func archiveTreeBudget(resolver file.Resolver, location file.Location) int64 {
var size int64
if m, err := resolver.FileMetadataByLocation(location); err == nil && m.FileInfo != nil {
size = m.Size()
}
return max(archiveTreeRatio*size, archiveTreeFloorBytes)
}
func (c *archiveCataloger) isAncestor(digests []file.Digest) bool { func (c *archiveCataloger) isAncestor(digests []file.Digest) bool {
for _, d := range digests { for _, d := range digests {
if slices.Contains(c.ancestors, d) { if slices.Contains(c.ancestors, d) {
+41
View File
@@ -369,6 +369,33 @@ func Test_archiveCataloger_stopsWhenTheContextIsCancelled(t *testing.T) {
assert.Zero(t, ran) assert.Zero(t, ran)
} }
func Test_archiveCataloger_decompressionBudgetIsSharedByATreeAndFreshPerTopLevelArchive(t *testing.T) {
// a recursive bomb fits the limits one level at a time, so what bounds it is the total decompressed
// under one top-level archive
origRatio, origFloor := archiveTreeRatio, archiveTreeFloorBytes
archiveTreeRatio, archiveTreeFloorBytes = 0, 100_000
t.Cleanup(func() { archiveTreeRatio, archiveTreeFloorBytes = origRatio, origFloor })
zeros := make([]byte, 60_000)
nested := makeZip(t, map[string][]byte{"zeros.bin": zeros})
rootDir := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(rootDir, "bomb.zip"), makeZip(t, map[string][]byte{"a.zip": nested, "b.zip": nested}), 0o600))
require.NoError(t, os.WriteFile(filepath.Join(rootDir, "sibling.zip"), makeZip(t, map[string][]byte{"zeros.bin": zeros}), 0o600))
var seen []string
fileCounts := map[string]int{}
tsk := newTestTask(t, cataloging.DefaultArchiveSearchConfig().WithMaxDepth(2), capturingTask(t, &seen, &fileCounts))
s := newTestSBOM()
require.NoError(t, tsk.Execute(context.Background(), dirTestResolver{dir: rootDir}, sbomsync.NewBuilder(s)))
assert.Equal(t, 1, fileCounts["/bomb.zip:a.zip"], "the first nested archive fits the tree's budget")
assert.Equal(t, 0, fileCounts["/bomb.zip:b.zip"], "the second does not: the budget is shared, not per archive")
reasons := s.Artifacts.Unknowns[file.Coordinates{RealPath: "b.zip", ArchivePath: "/bomb.zip"}]
require.Len(t, reasons, 1)
assert.Contains(t, reasons[0], "possible decompression bomb")
assert.Equal(t, 1, fileCounts["/sibling.zip"], "another top-level archive starts with a fresh budget")
}
func Test_NewArchiveCatalogerTask_dropsItselfFromSubPipeline(t *testing.T) { func Test_NewArchiveCatalogerTask_dropsItselfFromSubPipeline(t *testing.T) {
// a copy of itself in the sub-pipeline would process every nesting level twice // a copy of itself in the sub-pipeline would process every nesting level twice
cfg := cataloging.DefaultArchiveSearchConfig().WithMaxDepth(2) cfg := cataloging.DefaultArchiveSearchConfig().WithMaxDepth(2)
@@ -949,6 +976,11 @@ func (d dirTestResolver) FileContentsByLocation(loc file.Location) (io.ReadClose
return os.Open(filepath.Join(d.dir, filepath.FromSlash(loc.RealPath))) return os.Open(filepath.Join(d.dir, filepath.FromSlash(loc.RealPath)))
} }
func (d dirTestResolver) FileMetadataByLocation(loc file.Location) (file.Metadata, error) {
info, err := os.Stat(filepath.Join(d.dir, filepath.FromSlash(loc.RealPath)))
return file.Metadata{FileInfo: info}, err
}
// makeZip builds a zip in sorted entry-name order: limits are enforced as the walk proceeds, so entry // makeZip builds a zip in sorted entry-name order: limits are enforced as the walk proceeds, so entry
// order decides which entries land before a truncation. // order decides which entries land before a truncation.
func makeZip(t *testing.T, entries map[string][]byte) []byte { func makeZip(t *testing.T, entries map[string][]byte) []byte {
@@ -1034,6 +1066,11 @@ func (r twoLayerResolver) FileContentsByLocation(loc file.Location) (io.ReadClos
return io.NopCloser(bytes.NewReader(body)), nil return io.NopCloser(bytes.NewReader(body)), nil
} }
// FileMetadataByLocation reports no metadata, so the archive falls back to the minimum budget.
func (r twoLayerResolver) FileMetadataByLocation(file.Location) (file.Metadata, error) {
return file.Metadata{}, os.ErrNotExist
}
// incompressible returns n bytes deflate cannot shrink, so an archive built from them has a // incompressible returns n bytes deflate cannot shrink, so an archive built from them has a
// predictable size to set a limit against. // predictable size to set a limit against.
func incompressible(n int) []byte { func incompressible(n int) []byte {
@@ -1091,6 +1128,10 @@ func (a allFilesResolver) FileContentsByLocation(loc file.Location) (io.ReadClos
return os.Open(filepath.Join(a.dir, filepath.FromSlash(loc.RealPath))) return os.Open(filepath.Join(a.dir, filepath.FromSlash(loc.RealPath)))
} }
func (a allFilesResolver) FileMetadataByLocation(loc file.Location) (file.Metadata, error) {
return dirTestResolver{dir: a.dir}.FileMetadataByLocation(loc)
}
func (p *recordingPublisher) Publish(e partybus.Event) { func (p *recordingPublisher) Publish(e partybus.Event) {
p.events = append(p.events, e) p.events = append(p.events, e)
} }