oss/syft
2
0
Fork 0
mirror of https://github.com/anchore/syft.git synced 2026-08-19 16:48:27 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(arch): apply the mtree bounds while streaming

Browse Source 
Both caps were enforced against a fully buffered listing, so the peak cost
scaled with cataloger parallelism, which runs a goroutine per package. They now
apply as the stream flows: a limited reader for the byte cap and a counting
reader for the entry cap, which has to trip while the parser is still reading
since it materializes every entry before returning any of them.

Both limits are injectable so the boundary tests can be exact on both sides
without allocating their way up to the shipped 64MB. Each cap gets a listing
exactly at it, asserting the records parse whole rather than just that no error
came back, since quiet truncation is the failure a size check invites.

The byte limiter carries one byte of headroom, which is what makes an
overrunning listing distinguishable from one that simply ends at the cap.

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
This commit is contained in:
Alex Goodman 2026-08-14 10:22:35 -04:00
parent c0d37d130a
commit dcf051fbf1
No known key found for this signature in database
2 changed files with 121 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
syft/pkg/cataloger/arch/parse_alpm_db.go
View File

@ -292,7 +292,30 @@ const maxMtreeSize = 64 * 1024 * 1024
// any package has files.
const maxMtreeEntries = 200_000
// lineLimitedReader fails the read once the stream has carried more than max newlines. Counting as
// the bytes flow is what keeps the entries bounded: the parser materializes one per line before it
// returns any of them, so the count has to trip while it is still reading. Lines the parser goes on
// to skip only make this an over-count, never an under.
type lineLimitedReader struct {
reader io.Reader
lines int
max int
}
func (l *lineLimitedReader) Read(p []byte) (int, error) {
n, err := l.reader.Read(p)
l.lines += bytes.Count(p[:n], []byte("\n"))
if l.lines > l.max {
return n, fmt.Errorf("mtree file has more entries than allowed (max %d)", l.max)
}
return n, err
}
func parseMtree(r io.Reader) ([]pkg.AlpmFileRecord, error) {
return parseMtreeWithLimits(r, maxMtreeSize, maxMtreeEntries)
}
func parseMtreeWithLimits(r io.Reader, maxSize int64, maxEntries int) ([]pkg.AlpmFileRecord, error) {
var entries []pkg.AlpmFileRecord
gzReader, err := gzip.NewReader(r)
@ -300,23 +323,19 @@ func parseMtree(r io.Reader) ([]pkg.AlpmFileRecord, error) {
return nil, err
}
// read one byte past the cap so that hitting it is distinguishable from a listing that simply ends
// there. Truncating instead would hand back a package silently missing most of its files.
data, err := io.ReadAll(io.LimitReader(gzReader, maxMtreeSize+1))
if err != nil {
return nil, err
}
if len(data) > maxMtreeSize {
return nil, fmt.Errorf("mtree file is larger than the max allowed size (%d bytes)", maxMtreeSize)
}
// both bounds apply while the listing streams rather than after it is buffered. Buffering made the
// peak cost scale with cataloger parallelism, which runs a goroutine per package by default.
// Allowing one byte past the cap is what makes tripping it distinguishable from a listing that
// simply ends there. Truncating instead would hand back a package silently missing most of its files.
sizeLimited := &io.LimitedReader{R: gzReader, N: maxSize + 1}
specDh, err := mtree.ParseSpec(&lineLimitedReader{reader: sizeLimited, max: maxEntries})
// 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)
// the size bound is checked first because overrunning it looks like a clean EOF to the parser, so
// the parser either succeeds on a truncated listing or fails for some downstream reason. Either way
// the size is the useful error.
if sizeLimited.N <= 0 {
return nil, fmt.Errorf("mtree file is larger than the max allowed size (%d bytes)", maxSize)
}
specDh, err := mtree.ParseSpec(bytes.NewReader(data))
if err != nil {
return nil, err
}

120
syft/pkg/cataloger/arch/parse_alpm_db_test.go
View File

@ -4,6 +4,7 @@ import (
"bufio"
"bytes"
"compress/gzip"
"fmt"
"io"
"os"
"testing"
@ -221,20 +222,42 @@ func TestMtreeParse(t *testing.T) {
}
// 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 {
// mtreeSpec builds a valid listing naming n files, shaped like a real one: a signature, a /set of
// shared keywords, then one line per file. The boundary tests need the parser to actually reach the
// end of the listing, which a payload of filler bytes never does.
func mtreeSpec(n int) []byte {
var buf bytes.Buffer
buf.WriteString("#mtree\n")
buf.WriteString("/set type=file uid=0 gid=0 mode=644\n")
for i := range n {
fmt.Fprintf(&buf, "./file%d time=1649595592.0 size=10 sha256digest=%064x\n", i, i)
}
return buf.Bytes()
}
func gzipOf(t *testing.T, data []byte) io.Reader {
t.Helper()
var buf bytes.Buffer
w := gzip.NewWriter(&buf)
_, err := w.Write(data)
require.NoError(t, err)
require.NoError(t, w.Close())
return bytes.NewReader(buf.Bytes())
}
// gzipOfRepeated returns a gzip member that decompresses to 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 gzipOfRepeated(t *testing.T, payload byte, n int64) io.Reader {
t.Helper()
var buf bytes.Buffer
w := gzip.NewWriter(&buf)
chunk := bytes.Repeat([]byte{payload}, 32*1024)
for remaining := n; remaining > 0; {
size := int64(len(chunk))
if remaining < size {
size = remaining
}
size := min(remaining, int64(len(chunk)))
written, err := w.Write(chunk[:size])
require.NoError(t, err)
remaining -= int64(written)
@ -245,27 +268,25 @@ func gzipOf(t *testing.T, payload byte, 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
// the limits come from the fixture rather than the production constants so both sides of the
// boundary are exact and neither case has to allocate its way up to 64MB
spec := mtreeSpec(50)
t.Run("rejects a listing past the cap", func(t *testing.T) {
r := gzipOf(t, 0x00, maxMtreeSize+1)
t.Run("a listing at the cap parses whole", func(t *testing.T) {
records, err := parseMtreeWithLimits(gzipOf(t, spec), int64(len(spec)), maxMtreeEntries)
_, err := parseMtree(r)
require.ErrorContains(t, err, "larger than the max allowed size")
// asserting the records, not just the absence of an error: hitting the cap exactly must not
// quietly truncate the listing, which is the failure a size check invites
require.NoError(t, err)
require.Len(t, records, 50)
require.Equal(t, "/file0", records[0].Path)
require.Equal(t, "/file49", records[49].Path)
})
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 := gzipOf(t, 0x00, maxMtreeSize)
t.Run("rejects a listing one byte past the cap", func(t *testing.T) {
_, err := parseMtreeWithLimits(gzipOf(t, spec), int64(len(spec))-1, maxMtreeEntries)
_, err := parseMtree(r)
if err != nil {
require.NotContains(t, err.Error(), "larger than the max allowed size")
}
require.ErrorContains(t, err, "larger than the max allowed size")
})
}
@ -273,22 +294,55 @@ 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.
spec := mtreeSpec(50)
lines := bytes.Count(spec, []byte("\n")) // the two header lines get entries of their own
t.Run("rejects a listing with too many entries", func(t *testing.T) {
r := gzipOf(t, '\n', maxMtreeEntries+1)
t.Run("a listing at the cap parses whole", func(t *testing.T) {
records, err := parseMtreeWithLimits(gzipOf(t, spec), maxMtreeSize, lines)
_, err := parseMtree(r)
require.NoError(t, err)
require.Len(t, records, 50)
})
t.Run("rejects a listing one entry past the cap", func(t *testing.T) {
_, err := parseMtreeWithLimits(gzipOf(t, spec), maxMtreeSize, lines-1)
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)
t.Run("rejects a bomb at the production limits", func(t *testing.T) {
// the case the caps exist for: a few KB expanding to 4MB of lines, twenty times the entry cap
// and still well inside the byte cap. Runs against parseMtree so the shipped constants are what
// gets exercised. Kept to 4MB so that a regression here fails on the deadline below rather than
// taking the machine down with it.
bomb := gzipOfRepeated(t, '\n', 4*1024*1024)
compressed := bomb.(*bytes.Reader).Size()
require.Less(t, compressed, int64(64*1024), "payload should be small enough to be worth rejecting")
_, err := parseMtree(r)
start := time.Now()
_, err := parseMtree(bomb)
if err != nil {
require.NotContains(t, err.Error(), "more entries than allowed")
}
require.ErrorContains(t, err, "more entries than allowed")
// it has to give up while reading rather than after materializing the listing
require.Less(t, time.Since(start), 10*time.Second)
})
}
func Test_parseMtree_malformedInput(t *testing.T) {
t.Run("not gzip", func(t *testing.T) {
_, err := parseMtree(bytes.NewReader(mtreeSpec(2)))
require.Error(t, err)
})
t.Run("truncated gzip", func(t *testing.T) {
var buf bytes.Buffer
_, err := io.Copy(&buf, gzipOf(t, mtreeSpec(50)))
require.NoError(t, err)
_, err = parseMtree(bytes.NewReader(buf.Bytes()[:buf.Len()/2]))
// a truncated listing has to fail rather than come back as a package missing most of its files
require.Error(t, err)
})
}