mirror of
https://github.com/anchore/syft.git
synced 2026-10-30 14:02:26 +01:00
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 <wagoodman@users.noreply.github.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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}
|
||||
|
||||
Reference in New Issue
Block a user