From d27b6ebb1b6fae7944dc43e394076b827ab76dbc Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Thu, 24 Sep 2026 13:21:08 -0400 Subject: [PATCH] fix: cap each extracted archive entry at 2 GB `put` had no per-entry cap, unlike the legacy java path which capped every file at 2 GB through `intFile.SafeCopy`. At depth 1, one entry of an overlapping-entry zip bomb could decompress all the way to the 100 GB disk limit before anything stopped it. - entries are now read through the same 2 GB cap (`intFile.PerFileReadLimit`, now exported). An entry over it is dropped after cap+1 bytes and the rest of the archive is still extracted - `Resolver` gains `TruncatedReason`, so the truncation unknown says what happened ("an entry larger than 2.0 GiB was skipped", or the existing disk-limit text) instead of always blaming the limits Signed-off-by: Alex Goodman --- internal/archive/extract.go | 2 +- internal/archive/resolver.go | 33 +++++++++++++++++++++++++++---- internal/archive/resolver_test.go | 32 ++++++++++++++++++++++++++++++ internal/file/copy.go | 7 ++++--- internal/task/archive_tasks.go | 2 +- 5 files changed, 67 insertions(+), 9 deletions(-) diff --git a/internal/archive/extract.go b/internal/archive/extract.go index ae46b30c9..2939a9d29 100644 --- a/internal/archive/extract.go +++ b/internal/archive/extract.go @@ -149,7 +149,7 @@ func (r *Resolver) extract(ctx context.Context, format archives.Extractor, conte }) r.finish() if errors.Is(err, ErrDiskLimitReached) { - r.Truncated = true + r.truncate("extraction stopped at the configured memory or disk limit") return nil } return err diff --git a/internal/archive/resolver.go b/internal/archive/resolver.go index 04ae820cd..38d69ed2c 100644 --- a/internal/archive/resolver.go +++ b/internal/archive/resolver.go @@ -13,9 +13,11 @@ import ( "strings" "time" + "github.com/dustin/go-humanize" "github.com/mholt/archives" stereoscopeFile "github.com/anchore/stereoscope/pkg/file" + intFile "github.com/anchore/syft/internal/file" syftindex "github.com/anchore/syft/internal/index" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/internal/tmpdir" @@ -32,9 +34,10 @@ type Resolver struct { // Digests are of the archive file itself. Digests []file.Digest - // Truncated reports that the disk limit stopped extraction early; the resolver covers the entries - // stored before that. - Truncated bool + // 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. + Truncated bool + TruncatedReason string fileSystemID string archivePath string @@ -148,11 +151,20 @@ func (r *Resolver) add(hdr tar.Header, content io.Reader) error { } if hdr.Typeflag == tar.TypeReg && content != nil { - if err := r.put(&n.content, content); err != nil { + // reading one byte past the cap is how an entry over it is told apart from one exactly at it + err := r.put(&n.content, io.LimitReader(content, maxEntryBytes+1)) + if err == nil && n.content.size > maxEntryBytes { + err = errEntryTooLarge + } + if err != nil { r.discard(&n.content) if !existed { delete(r.byPath, entryPath) } + if errors.Is(err, errEntryTooLarge) { + r.truncate(fmt.Sprintf("an entry larger than %s was skipped", humanize.IBytes(uint64(maxEntryBytes)))) + return nil + } if !errors.Is(err, ErrDiskLimitReached) { return fmt.Errorf("unable to read archive entry %q: %w", hdr.Name, err) } @@ -172,6 +184,19 @@ func (r *Resolver) add(hdr tar.Header, content io.Reader) error { return nil } +// maxEntryBytes caps any one entry, as the legacy java extraction did (see intFile.SafeCopy): without it +// a single entry of an overlapping-entry zip bomb can decompress up to the disk limit. +var maxEntryBytes int64 = intFile.PerFileReadLimit + +var errEntryTooLarge = errors.New("archive entry exceeds the per-entry size cap") + +// truncate marks the resolver as covering part of the archive, keeping the first reason given. +func (r *Resolver) truncate(reason string) { + if !r.Truncated { + r.Truncated, r.TruncatedReason = true, reason + } +} + // chargeIndex charges index bookkeeping, which lives in memory. Content held in memory can move to // disk to make room for it; the index cannot, so when it is refused the entries are. func (r *Resolver) chargeIndex(n int64) bool { diff --git a/internal/archive/resolver_test.go b/internal/archive/resolver_test.go index b3fda91ba..2c691f66b 100644 --- a/internal/archive/resolver_test.go +++ b/internal/archive/resolver_test.go @@ -510,3 +510,35 @@ func TestResolver_allLocationsKeepsEveryPath(t *testing.T) { assert.Equal(t, []string{"opt/gone.jar", "opt/real.jar", "opt/sym.jar"}, got, "every path the archive names, links included") } + +func TestResolver_add_anEntryOverTheCapIsSkippedAndReadNoFurther(t *testing.T) { + // one entry of an overlapping-entry bomb can inflate without bound; it must stop at the cap, cost + // the archive only that entry, and say so + orig := maxEntryBytes + maxEntryBytes = 1024 + t.Cleanup(func() { maxEntryBytes = orig }) + + r, _ := resolverIn(t, memCharge(-1)) + endless := &countingReader{r: zeroSource{}} + require.NoError(t, r.add(tar.Header{Name: "bomb.bin", Typeflag: tar.TypeReg, Mode: 0o644}, endless)) + require.NoError(t, r.add(tar.Header{Name: "ok.txt", Typeflag: tar.TypeReg, Mode: 0o644}, strings.NewReader("ok"))) + r.finish() + + assert.LessOrEqual(t, endless.n, 1024+copyChunkSize, "reading stops just past the cap") + assert.True(t, r.Truncated) + assert.Contains(t, r.TruncatedReason, "an entry larger than 1.0 KiB was skipped") + locations, err := r.FilesByGlob("**") + require.NoError(t, err) + assert.Equal(t, []string{"ok.txt"}, realPaths(locations)) +} + +type countingReader struct { + r io.Reader + n int +} + +func (c *countingReader) Read(p []byte) (int, error) { + n, err := c.r.Read(p) + c.n += n + return n, err +} diff --git a/internal/file/copy.go b/internal/file/copy.go index 27c12ef93..636b3b431 100644 --- a/internal/file/copy.go +++ b/internal/file/copy.go @@ -6,12 +6,13 @@ import ( "io" ) -const perFileReadLimit = 2 * GB +// PerFileReadLimit is the most read from any one archive entry, guarding against decompression bombs. +const PerFileReadLimit = 2 * GB // SafeCopy bounds the copy to guard against decompression bombs when extracting from archives. func SafeCopy(writer io.Writer, reader io.Reader) error { - numBytes, err := io.Copy(writer, io.LimitReader(reader, perFileReadLimit)) - if numBytes >= perFileReadLimit { + numBytes, err := io.Copy(writer, io.LimitReader(reader, PerFileReadLimit)) + if numBytes >= PerFileReadLimit { return fmt.Errorf("zip read limit hit (potential decompression bomb attack)") } // Propagate decompression / read errors up to the caller. io.Copy diff --git a/internal/task/archive_tasks.go b/internal/task/archive_tasks.go index 8034fa3b2..40e28f2ee 100644 --- a/internal/task/archive_tasks.go +++ b/internal/task/archive_tasks.go @@ -215,7 +215,7 @@ func (c *archiveCataloger) processArchive(ctx context.Context, parentResolver fi defer func() { c.ancestors = c.ancestors[:len(c.ancestors)-len(extracted.Digests)] }() if extracted.Truncated { - recordArchiveUnknown(builder, location.Coordinates, "archive cataloged from part of its contents: extraction stopped at the configured memory or disk limit") + recordArchiveUnknown(builder, location.Coordinates, "archive cataloged from part of its contents: "+extracted.TruncatedReason) } traversal := &archive.Traversal{Location: location, Digests: extracted.Digests}