diff --git a/syft/pkg/cataloger/arch/parse_alpm_db.go b/syft/pkg/cataloger/arch/parse_alpm_db.go index 1d4066c23..5878ed90b 100644 --- a/syft/pkg/cataloger/arch/parse_alpm_db.go +++ b/syft/pkg/cataloger/arch/parse_alpm_db.go @@ -284,6 +284,14 @@ func parsePkgFiles(pkgFields map[string]any) (*parsedData, error) { // mtree.ParseSpec materializes every entry before returning any of them. const maxMtreeSize = 64 * 1024 * 1024 +// maxMtreeEntries bounds how many lines the listing may hold, which the byte cap alone does not. +// go-mtree gives every line its own entry and appends it unconditionally, blank lines included, so +// short lines cost about 1KB of retained heap each. 16MB of newlines is 8GB of peak heap while +// staying comfortably inside maxMtreeSize, and it returns no records and no error while doing it. +// Real listings run 150 to 250 bytes per line, so a listing that trips this held far more lines than +// any package has files. +const maxMtreeEntries = 200_000 + func parseMtree(r io.Reader) ([]pkg.AlpmFileRecord, error) { var entries []pkg.AlpmFileRecord @@ -302,6 +310,12 @@ func parseMtree(r io.Reader) ([]pkg.AlpmFileRecord, error) { return nil, fmt.Errorf("mtree file is larger than the max allowed size (%d bytes)", maxMtreeSize) } + // counting lines up front is what keeps the entry allocation bounded, since the parser materializes + // every line before it returns. Lines the parser skips only make this an over-count, never an under. + if lines := bytes.Count(data, []byte("\n")); lines > maxMtreeEntries { + return nil, fmt.Errorf("mtree file has more entries than allowed (%d entries, max %d)", lines, maxMtreeEntries) + } + specDh, err := mtree.ParseSpec(bytes.NewReader(data)) if err != nil { return nil, err diff --git a/syft/pkg/cataloger/arch/parse_alpm_db_test.go b/syft/pkg/cataloger/arch/parse_alpm_db_test.go index be4fc0d65..414263f60 100644 --- a/syft/pkg/cataloger/arch/parse_alpm_db_test.go +++ b/syft/pkg/cataloger/arch/parse_alpm_db_test.go @@ -221,15 +221,15 @@ func TestMtreeParse(t *testing.T) { } -// gzipOfSize returns a gzip member that decompresses to exactly n bytes. The payload is -// highly compressible, which is the whole point: the caller supplies kilobytes and the -// decompressed stream is whatever size it asks for. -func gzipOfSize(t *testing.T, n int64) io.Reader { +// gzipOf returns a gzip member that decompresses to exactly n bytes of payload repeated. The +// payload is highly compressible, which is the whole point: the caller supplies kilobytes and +// the decompressed stream is whatever size it asks for. +func gzipOf(t *testing.T, payload byte, n int64) io.Reader { t.Helper() var buf bytes.Buffer w := gzip.NewWriter(&buf) - chunk := make([]byte, 32*1024) + chunk := bytes.Repeat([]byte{payload}, 32*1024) for remaining := n; remaining > 0; { size := int64(len(chunk)) if remaining < size { @@ -245,8 +245,11 @@ func gzipOfSize(t *testing.T, n int64) io.Reader { } func Test_parseMtree_boundsDecompressedSize(t *testing.T) { + // note: a NUL payload holds no newlines, so these exercise the byte cap without tripping the + // entry cap first + t.Run("rejects a listing past the cap", func(t *testing.T) { - r := gzipOfSize(t, maxMtreeSize+1) + r := gzipOf(t, 0x00, maxMtreeSize+1) _, err := parseMtree(r) @@ -256,7 +259,7 @@ func Test_parseMtree_boundsDecompressedSize(t *testing.T) { t.Run("a listing at the cap is not rejected on size", func(t *testing.T) { // guards the off-by-one: at exactly the cap the size check must not fire, so whatever // happens next is the mtree parser's business and not ours - r := gzipOfSize(t, maxMtreeSize) + r := gzipOf(t, 0x00, maxMtreeSize) _, err := parseMtree(r) @@ -265,3 +268,27 @@ func Test_parseMtree_boundsDecompressedSize(t *testing.T) { } }) } + +func Test_parseMtree_boundsEntryCount(t *testing.T) { + // the byte cap alone does not bound retained memory, since the parser keeps an entry per line + // including blank ones. Measured on this parser before the entry cap existed: 16MB of newlines, + // well inside the byte cap, cost 8GB of peak heap and returned no records and no error. + + t.Run("rejects a listing with too many entries", func(t *testing.T) { + r := gzipOf(t, '\n', maxMtreeEntries+1) + + _, err := parseMtree(r) + + require.ErrorContains(t, err, "more entries than allowed") + }) + + t.Run("a listing at the entry cap is not rejected on count", func(t *testing.T) { + r := gzipOf(t, '\n', maxMtreeEntries) + + _, err := parseMtree(r) + + if err != nil { + require.NotContains(t, err.Error(), "more entries than allowed") + } + }) +}