mirror of
https://github.com/anchore/syft.git
synced 2026-08-19 16:48:27 +02:00
fix(arch): bound mtree entry count, not just decompressed bytes
The byte cap alone does not bound what the parse retains. go-mtree keeps an entry per line and appends it unconditionally, blank lines included, so short lines cost about 1KB of heap each. Measured on the previous commit: 15KB of gzipped newlines expanding to 16MB, comfortably inside the 64MB byte cap, drove 8.1GB of peak heap over 74 seconds and returned no records and a nil error. Same input now rejects in 0.05s at 51MB peak. Counting lines before parsing is what makes the entry allocation bounded, since the parser materializes all of them before returning. Lines the parser skips only make the count high, never low. Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
This commit is contained in:
parent
b8381670d6
commit
c0d37d130a
@ -284,6 +284,14 @@ func parsePkgFiles(pkgFields map[string]any) (*parsedData, error) {
|
|||||||
// mtree.ParseSpec materializes every entry before returning any of them.
|
// mtree.ParseSpec materializes every entry before returning any of them.
|
||||||
const maxMtreeSize = 64 * 1024 * 1024
|
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) {
|
func parseMtree(r io.Reader) ([]pkg.AlpmFileRecord, error) {
|
||||||
var entries []pkg.AlpmFileRecord
|
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)
|
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))
|
specDh, err := mtree.ParseSpec(bytes.NewReader(data))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
|
|||||||
@ -221,15 +221,15 @@ func TestMtreeParse(t *testing.T) {
|
|||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// gzipOfSize returns a gzip member that decompresses to exactly n bytes. The payload is
|
// gzipOf returns a gzip member that decompresses to exactly n bytes of payload repeated. The
|
||||||
// highly compressible, which is the whole point: the caller supplies kilobytes and the
|
// payload is highly compressible, which is the whole point: the caller supplies kilobytes and
|
||||||
// decompressed stream is whatever size it asks for.
|
// the decompressed stream is whatever size it asks for.
|
||||||
func gzipOfSize(t *testing.T, n int64) io.Reader {
|
func gzipOf(t *testing.T, payload byte, n int64) io.Reader {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
var buf bytes.Buffer
|
var buf bytes.Buffer
|
||||||
w := gzip.NewWriter(&buf)
|
w := gzip.NewWriter(&buf)
|
||||||
chunk := make([]byte, 32*1024)
|
chunk := bytes.Repeat([]byte{payload}, 32*1024)
|
||||||
for remaining := n; remaining > 0; {
|
for remaining := n; remaining > 0; {
|
||||||
size := int64(len(chunk))
|
size := int64(len(chunk))
|
||||||
if remaining < size {
|
if remaining < size {
|
||||||
@ -245,8 +245,11 @@ func gzipOfSize(t *testing.T, n int64) io.Reader {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func Test_parseMtree_boundsDecompressedSize(t *testing.T) {
|
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) {
|
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)
|
_, 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) {
|
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
|
// 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
|
// happens next is the mtree parser's business and not ours
|
||||||
r := gzipOfSize(t, maxMtreeSize)
|
r := gzipOf(t, 0x00, maxMtreeSize)
|
||||||
|
|
||||||
_, err := parseMtree(r)
|
_, 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")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user