diff --git a/syft/pkg/cataloger/arch/cataloger_test.go b/syft/pkg/cataloger/arch/cataloger_test.go index dd014086c..56e00ab9d 100644 --- a/syft/pkg/cataloger/arch/cataloger_test.go +++ b/syft/pkg/cataloger/arch/cataloger_test.go @@ -1,10 +1,14 @@ package arch import ( + "bytes" "context" + "io" + "os" "testing" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/stretchr/testify/require" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" @@ -12,7 +16,23 @@ import ( "github.com/anchore/syft/syft/pkg/cataloger/internal/pkgtest" ) +const bombFixture = "testdata/installed/var/lib/pacman/local/bombpkg-1.0-1/mtree" + +// writeBombFixture generates bombpkg's mtree: one line over maxMtreeLines, so the cataloger rejects +// it while walking testdata/installed. Generated rather than committed so the size that makes it a +// bomb tracks the constant instead of hiding in a gzip blob. +func writeBombFixture(t *testing.T) { + t.Helper() + + listing := append([]byte("#mtree\n"), bytes.Repeat([]byte("./f\n"), maxMtreeLines+1)...) + compressed, err := io.ReadAll(gzipOf(t, listing)) + require.NoError(t, err) + require.NoError(t, os.WriteFile(bombFixture, compressed, 0o600)) +} + func TestAlpmUnknowns(t *testing.T) { + writeBombFixture(t) + pkgtest.NewCatalogTester(). FromDirectory(t, "testdata/installed"). WithCompareOptions(cmpopts.IgnoreFields(pkg.AlpmFileRecord{}, "Time")). @@ -21,11 +41,14 @@ func TestAlpmUnknowns(t *testing.T) { } func TestAlpmCataloger(t *testing.T) { + writeBombFixture(t) + gmpDbLocation := file.NewLocation("var/lib/pacman/local/gmp-6.2.1-2/desc") treeSitterDbLocation := file.NewLocation("var/lib/pacman/local/tree-sitter-0.22.6-1/desc") emacsDbLocation := file.NewLocation("var/lib/pacman/local/emacs-29.3-3/desc") fuzzyDbLocation := file.NewLocation("var/lib/pacman/local/fuzzy-1.2-3/desc") madeupDbLocation := file.NewLocation("var/lib/pacman/local/madeup-20.30-4/desc") + bombDbLocation := file.NewLocation("var/lib/pacman/local/bombpkg-1.0-1/desc") ctx := context.TODO() treeSitterPkg := pkg.Package{ @@ -119,6 +142,26 @@ func TestAlpmCataloger(t *testing.T) { }, } + // bombPkg's listing exceeds maxMtreeLines, so it emits with an empty file list, not dropped + bombPkg := pkg.Package{ + Name: "bombpkg", + Version: "1.0-1", + Type: pkg.AlpmPkg, + FoundBy: "alpm-db-cataloger", + Locations: file.NewLocationSet(bombDbLocation), + Metadata: pkg.AlpmDBEntry{ + BasePackage: "bombpkg", + Package: "bombpkg", + Version: "1.0-1", + Description: "Package whose mtree listing exceeds the parser's line bound", + Architecture: "x86_64", + Size: 1, + Reason: 1, + Files: []pkg.AlpmFileRecord{}, + Backup: []pkg.AlpmFileRecord{}, + }, + } + gmpPkg := pkg.Package{ Name: "gmp", Version: "6.2.1-2", @@ -283,6 +326,7 @@ func TestAlpmCataloger(t *testing.T) { fuzzyPkg, madeupPkg, gmpPkg, + bombPkg, } expectedRelationships := []artifact.Relationship{ diff --git a/syft/pkg/cataloger/arch/parse_alpm_db.go b/syft/pkg/cataloger/arch/parse_alpm_db.go index 6e409824b..fe9762087 100644 --- a/syft/pkg/cataloger/arch/parse_alpm_db.go +++ b/syft/pkg/cataloger/arch/parse_alpm_db.go @@ -4,6 +4,7 @@ import ( "bufio" "compress/gzip" "context" + "errors" "fmt" "io" "path" @@ -16,6 +17,7 @@ import ( "github.com/vbatts/go-mtree" "github.com/anchore/syft/internal" + intFile "github.com/anchore/syft/internal/file" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/internal/unknown" "github.com/anchore/syft/syft/artifact" @@ -216,6 +218,10 @@ func parseDatabase(b *bufio.Scanner) (*parsedData, error) { var backup []map[string]any for _, f := range strings.Split(value, "\n") { fields := strings.SplitN(f, "\t", 2) + // without the tab there is no digest, and fields[1] would panic + if len(fields) < 2 { + continue + } p := fmt.Sprintf("/%s", fields[0]) if ok := ignoredFiles[p]; !ok { backup = append(backup, map[string]any{ @@ -241,6 +247,12 @@ func parseDatabase(b *bufio.Scanner) (*parsedData, error) { } } + // a field over the scanner's 1MB token cap ends the scan early, which would otherwise look + // like a complete package + if err := b.Err(); err != nil { + return nil, err + } + return parsePkgFiles(pkgFields) } @@ -277,15 +289,107 @@ func parsePkgFiles(pkgFields map[string]any) (*parsedData, error) { return &entry, nil } +// maxMtreeSize bounds the decompressed listing. texlive-fontsextra, the largest package Arch +// ships, runs 16 to 27MB. +// +// this caps bytes, not memory: go-mtree allocates per field, so a crafted listing can still cost +// about a gigabyte, times cataloger parallelism. +const maxMtreeSize = 64 * intFile.MB + +// maxMtreeLines bounds entry count, which the byte cap does not: go-mtree keeps a ~128 byte entry +// per line, blanks included. 300k is ~2.8x the largest package Arch ships. +const maxMtreeLines = 300_000 + +var ( + errMtreeTooLarge = errors.New("mtree file is larger than the max allowed size") + errTooManyMtreeLines = errors.New("mtree file has more lines than allowed") + + // errMtreeLineContinued rejects a line ending in a backslash, which readers treat as "join with + // the next line". We refuse the file instead, for two reasons. + // + // cost: go-mtree joins with a quadratic string concat, so work grows as lines squared. 100k + // continued lines is a 1.2MB listing costing ~3.7s and ~52GB of churn. The bounds above are + // linear and never see it, and no line cap helps, since real packages reach ~107k lines. + // + // spec: mtree(5) says "Each line is interpreted independently" and documents no continuation, + // nor does Arch's ALPM-MTREEv2(5). Readers accept them anyway (BSD mtree via fparseln(3), + // libarchive in readline()), but no writer emits one: libarchive escapes a literal backslash as + // \134, and Arch builds .MTREE with it. So a continuation here is crafted, not a package. + errMtreeLineContinued = errors.New("mtree file uses line continuations") +) + +// lineLimitedReader fails the read once the stream carries more than max newlines, or a line +// continuation. Counting while the bytes flow is what bounds entries: the parser builds one per +// line before returning any of them. +// +// continuations are refused rather than joined, unlike every other mtree reader. See +// errMtreeLineContinued. Both \n and \r\n count, since go-mtree strips a trailing carriage return +// before testing for the backslash. +// +// a tripped bound is latched, so reading past the first error does not come back clean. +type lineLimitedReader struct { + reader io.Reader + lines int + max int + lastByte byte + err error +} + +func (l *lineLimitedReader) Read(p []byte) (int, error) { + if l.err != nil { + return 0, l.err + } + + n, err := l.reader.Read(p) + + // prev carries over the last byte of the previous read, so a backslash split from its line + // ending is still caught + prev := l.lastByte + for _, c := range p[:n] { + if (c == '\n' || c == '\r') && prev == '\\' { + l.err = errMtreeLineContinued + return n, l.err + } + if c == '\n' { + l.lines++ + } + prev = c + } + l.lastByte = prev + + if l.lines > l.max { + l.err = fmt.Errorf("%w (max %d)", errTooManyMtreeLines, l.max) + return n, l.err + } + + return n, err +} + func parseMtree(r io.Reader) ([]pkg.AlpmFileRecord, error) { - var err error + return parseMtreeWithLimits(r, maxMtreeSize, maxMtreeLines) +} + +// parseMtreeWithLimits lets the boundary tests use small exact limits instead of the shipped caps. +// maxSize must be below math.MaxInt64, since the limiter carries a byte of headroom. +func parseMtreeWithLimits(r io.Reader, maxSize int64, maxLines int) ([]pkg.AlpmFileRecord, error) { var entries []pkg.AlpmFileRecord - r, err = gzip.NewReader(r) + gzReader, err := gzip.NewReader(r) if err != nil { return nil, err } - specDh, err := mtree.ParseSpec(r) + defer internal.CloseAndLogError(gzReader, "mtree") + + // the line cap trips mid-stream, the byte cap cannot: io.LimitedReader reports a drained budget + // as io.EOF, which the parser reads as a complete listing, so it is checked after the parse. The + // extra byte tells a drained budget from a listing ending exactly at the cap. + sizeLimited := &io.LimitedReader{R: gzReader, N: maxSize + 1} + specDh, err := mtree.ParseSpec(&lineLimitedReader{reader: sizeLimited, max: maxLines}) + + // before err: a drained budget means the parser saw only part of the listing + if sizeLimited.N <= 0 { + return nil, fmt.Errorf("%w (%d bytes)", errMtreeTooLarge, maxSize) + } 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 a046fff88..cb591e7aa 100644 --- a/syft/pkg/cataloger/arch/parse_alpm_db_test.go +++ b/syft/pkg/cataloger/arch/parse_alpm_db_test.go @@ -2,13 +2,19 @@ package arch import ( "bufio" + "bytes" + "compress/gzip" + "fmt" + "io" "os" + "strings" "testing" "time" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" + intFile "github.com/anchore/syft/internal/file" "github.com/anchore/syft/syft/file" "github.com/anchore/syft/syft/pkg" ) @@ -155,6 +161,26 @@ func TestDatabaseParser(t *testing.T) { } } +func Test_parseAlpmDBEntry_malformedDatabase(t *testing.T) { + t.Run("backup line with no tab", func(t *testing.T) { + // this used to panic, which drops every package rather than the one + entry, err := parseAlpmDBEntry(strings.NewReader("%NAME%\nfoo\n\n%BACKUP%\nnotabhere\n\n")) + + require.NoError(t, err) + require.NotNil(t, entry) + require.Empty(t, entry.Backup) + }) + + t.Run("field over the scanner's token cap", func(t *testing.T) { + // everything after the oversize field is lost, so this has to error rather than look complete + db := "%NAME%\nfoo\n\n%DESC%\n" + strings.Repeat("x", 2*intFile.MB) + "\n\n%ARCH%\nx86_64\n\n" + + _, err := parseAlpmDBEntry(strings.NewReader(db)) + + require.ErrorIs(t, err, bufio.ErrTooLong) + }) +} + func parseTime(stime string) time.Time { t, _ := time.Parse(time.RFC3339, stime) return t @@ -207,6 +233,7 @@ func TestMtreeParse(t *testing.T) { reader := bufio.NewReader(f) + // a normal listing must not trip any of the bounds entry, err := parseMtree(reader) require.NoError(t, err) @@ -217,3 +244,243 @@ func TestMtreeParse(t *testing.T) { } } + +// mtreeSpec builds a valid listing of n files. Boundary tests need the parser to reach the end of +// it, which filler bytes never do. +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 builds a gzip member decompressing to n bytes of one repeated byte, so a few KB of +// input expands to whatever size the caller asks for. +func gzipOfRepeated(t *testing.T, payload byte, n int64) *bytes.Reader { + t.Helper() + + var buf bytes.Buffer + w := gzip.NewWriter(&buf) + chunk := bytes.Repeat([]byte{payload}, 32*1024) + for remaining := n; remaining > 0; { + size := min(remaining, int64(len(chunk))) + written, err := w.Write(chunk[:size]) + require.NoError(t, err) + remaining -= int64(written) + } + require.NoError(t, w.Close()) + + return bytes.NewReader(buf.Bytes()) +} + +func Test_parseMtree_boundsDecompressedSize(t *testing.T) { + // limits come from the fixture, so both sides of the boundary are exact + spec := mtreeSpec(50) + + t.Run("a listing at the cap parses whole", func(t *testing.T) { + records, err := parseMtreeWithLimits(gzipOf(t, spec), int64(len(spec)), maxMtreeLines) + + // records, not just no error: landing exactly on the cap must not truncate + 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("rejects a listing one byte past the cap", func(t *testing.T) { + _, err := parseMtreeWithLimits(gzipOf(t, spec), int64(len(spec))-1, maxMtreeLines) + + require.ErrorIs(t, err, errMtreeTooLarge) + }) +} + +// Test_parseMtree_shippedConstants pins the production values. The boundary tests parameterize the +// limits, so they would pass unchanged if a constant regressed. Driving a real 64MB listing through +// the parser instead cost 12s of a 16s race run. +func Test_parseMtree_shippedConstants(t *testing.T) { + require.Equal(t, int64(64*intFile.MB), int64(maxMtreeSize)) + require.Equal(t, 300_000, maxMtreeLines) +} + +func Test_parseMtree_boundsLineCount(t *testing.T) { + // the byte cap does not bound memory: the parser keeps an entry per line, blanks included + spec := mtreeSpec(50) + lines := bytes.Count(spec, []byte("\n")) // the two header lines get entries of their own + + t.Run("a listing at the cap parses whole", func(t *testing.T) { + records, err := parseMtreeWithLimits(gzipOf(t, spec), maxMtreeSize, lines) + + require.NoError(t, err) + require.Len(t, records, 50) + }) + + t.Run("rejects a listing one line past the cap", func(t *testing.T) { + _, err := parseMtreeWithLimits(gzipOf(t, spec), maxMtreeSize, lines-1) + + require.ErrorIs(t, err, errTooManyMtreeLines) + }) + + t.Run("rejects a bomb at the production limits without reading all of it", func(t *testing.T) { + // what the cap exists for: a few KB expanding past the line cap while inside the byte cap + bomb := gzipOfRepeated(t, '\n', 4*1024*1024) + require.Less(t, bomb.Size(), int64(64*1024), "payload should be small enough to be worth rejecting") + + counted := &countingReader{reader: bomb} + + _, err := parseMtree(counted) + + require.ErrorIs(t, err, errTooManyMtreeLines) + + // what matters is giving up mid-read, not after building the whole listing. Bytes consumed is + // the stable way to check that; an allocation delta drifts with GC and parallelism. + require.Less(t, counted.n, bomb.Size(), + "the whole bomb was read, so the cap tripped after the parse rather than during it") + }) +} + +// countingReader records how many bytes were pulled through it. +type countingReader struct { + reader io.Reader + n int64 +} + +func (c *countingReader) Read(p []byte) (int, error) { + n, err := c.reader.Read(p) + c.n += int64(n) + return n, err +} + +// Test_parseMtree_sizeCapTakesPrecedenceOverLineCap pins which error wins when one read exceeds both +// bounds. No other test exceeds both at once, so a reorder would go unnoticed. +func Test_parseMtree_sizeCapTakesPrecedenceOverLineCap(t *testing.T) { + spec := mtreeSpec(50) + + // maxLines=0 trips on the first newline; maxSize=20 is drained by that same read + _, err := parseMtreeWithLimits(gzipOf(t, spec), 20, 0) + + require.ErrorIs(t, err, errMtreeTooLarge) + require.NotErrorIs(t, err, errTooManyMtreeLines) +} + +func Test_parseMtree_rejectsLineContinuations(t *testing.T) { + // go-mtree strips a trailing carriage return before testing for the backslash, so CRLF continues + // a line too + for _, lineEnding := range []string{"\\\n", "\\\r\n"} { + t.Run(fmt.Sprintf("%q", lineEnding), func(t *testing.T) { + spec := mtreeSpec(5) + spec = bytes.Replace(spec, []byte("size=10 sha256digest="), []byte("size=10 "+lineEnding+"sha256digest="), 1) + + _, err := parseMtree(gzipOf(t, spec)) + + require.ErrorIs(t, err, errMtreeLineContinued) + }) + } +} + +// scriptedReader replays a fixed sequence of Read results, driving the straddling case directly. +type scriptedReader struct { + chunks []string +} + +func (r *scriptedReader) Read(p []byte) (int, error) { + if len(r.chunks) == 0 { + return 0, io.EOF + } + n := copy(p, r.chunks[0]) + // keep what did not fit, so a short buffer still obeys io.Reader + if n < len(r.chunks[0]) { + r.chunks[0] = r.chunks[0][n:] + } else { + r.chunks = r.chunks[1:] + } + return n, nil +} + +func Test_lineLimitedReader_rejectsStraddlingContinuation(t *testing.T) { + // the backslash and its line ending land in separate Read calls, which happens for real: the + // io.LimitedReader truncates the buffer and flate returns short reads. + tests := []struct { + name string + chunks []string + }{ + {name: "lf", chunks: []string{"...\\", "\n..."}}, + {name: "crlf", chunks: []string{"...\\", "\r\n..."}}, + {name: "crlf split at the carriage return", chunks: []string{"...\\\r", "\n..."}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + lr := &lineLimitedReader{reader: &scriptedReader{chunks: tt.chunks}, max: 100} + + var err error + buf := make([]byte, 16) + for err == nil { + _, err = lr.Read(buf) + } + + require.ErrorIs(t, err, errMtreeLineContinued) + }) + } +} + +func Test_lineLimitedReader_latchesTheError(t *testing.T) { + // a tripped bound must not come back clean on the next read + lr := &lineLimitedReader{reader: &scriptedReader{chunks: []string{"a\\\nb", "clean"}}, max: 100} + + buf := make([]byte, 16) + _, first := lr.Read(buf) + require.ErrorIs(t, first, errMtreeLineContinued) + + n, second := lr.Read(buf) + + require.ErrorIs(t, second, errMtreeLineContinued) + require.Zero(t, n) +} + +func Test_parseMtree_malformedInput(t *testing.T) { + t.Run("not gzip", func(t *testing.T) { + _, err := parseMtree(bytes.NewReader(mtreeSpec(2))) + + require.ErrorIs(t, err, gzip.ErrHeader) + }) + + t.Run("truncated gzip", func(t *testing.T) { + spec := mtreeSpec(50) + var buf bytes.Buffer + _, err := io.Copy(&buf, gzipOf(t, spec)) + require.NoError(t, err) + + // the cap sits just above what the truncated stream delivers, so the size check and the + // truncation actually race. The shipped 64MB cap could never fire here. + _, err = parseMtreeWithLimits(bytes.NewReader(buf.Bytes()[:buf.Len()/2]), int64(len(spec)), maxMtreeLines) + + // must fail, but not as a size-bound trip + require.Error(t, err) + require.NotErrorIs(t, err, errMtreeTooLarge) + }) + + t.Run("single line over the scanner's token cap", func(t *testing.T) { + // the third bound is go-mtree's scanner, at the default 64KB token limit. Nothing else here + // would catch a dependency bump that calls scanner.Buffer. + spec := append([]byte("#mtree\n./big "), bytes.Repeat([]byte("a"), 128*1024)...) + + _, err := parseMtree(gzipOf(t, append(spec, '\n'))) + + require.ErrorContains(t, err, "token too long") + }) +} diff --git a/syft/pkg/cataloger/arch/testdata/installed/var/lib/pacman/local/bombpkg-1.0-1/.gitignore b/syft/pkg/cataloger/arch/testdata/installed/var/lib/pacman/local/bombpkg-1.0-1/.gitignore new file mode 100644 index 000000000..dfa63e683 --- /dev/null +++ b/syft/pkg/cataloger/arch/testdata/installed/var/lib/pacman/local/bombpkg-1.0-1/.gitignore @@ -0,0 +1,2 @@ +# generated by writeBombFixture, see cataloger_test.go +/mtree diff --git a/syft/pkg/cataloger/arch/testdata/installed/var/lib/pacman/local/bombpkg-1.0-1/desc b/syft/pkg/cataloger/arch/testdata/installed/var/lib/pacman/local/bombpkg-1.0-1/desc new file mode 100644 index 000000000..abd0ad98d --- /dev/null +++ b/syft/pkg/cataloger/arch/testdata/installed/var/lib/pacman/local/bombpkg-1.0-1/desc @@ -0,0 +1,20 @@ +%NAME% +bombpkg + +%VERSION% +1.0-1 + +%BASE% +bombpkg + +%DESC% +Package whose mtree listing exceeds the parser's line bound + +%ARCH% +x86_64 + +%SIZE% +1 + +%REASON% +1