From 1dc16fbac463aea94a4d456fb1dacb8ea21bf47b Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Tue, 29 Sep 2026 16:52:58 -0400 Subject: [PATCH] fix: report scanner errors and raise the line cap in metadata parsers (#5353) * fix: report scanner errors instead of returning a partial parse as complete A line longer than `bufio.Scanner`'s 64KB buffer ends the scan with `ErrTooLong`, which only `Err()` reports. Parsers now return what they found along with the error (recorded as an unknown for the file), or log it where there is no error to return. Signed-off-by: Alex Goodman * fix: allow lines up to 1MB in line-oriented package metadata parsers Adds `internal.NewLineScanner`, a `bufio.Scanner` capped at 1MB per line rather than the 64KB default, the same cap the alpm parser already uses. Long single-line fields (descriptions, license text, dependency lists) now parse instead of ending the scan early. Signed-off-by: Alex Goodman * fix: use io.ReadFull where a short read was silently accepted - dotnet bundle 7-bit int decoding could read a zero-byte result as a zero byte - wordpress plugin header parsing included the unread tail of its buffer Signed-off-by: Alex Goodman * fix: finish moving line scanners to the 1MB cap and checking scanner errors - java manifest, pom.properties, Gemfile.lock, requirements.txt, homebrew formula, alpm and the purl/cpe decoders now use `internal.NewLineScanner` - snap kernel changelog reports a scan error instead of silently dropping the base kernel package - the attestation output reader keeps draining the pipe after a scan error so cosign can't block on a full pipe Signed-off-by: Alex Goodman * fix: stop logging the full setup.py line once per dependency match The debug log ran once per regex match and included the whole line, so log volume grew with the square of line length. With 1MB lines that is hundreds of GB of debug output from a single crafted file. Signed-off-by: Alex Goodman * fix(gentoo): don't dereference a nil location when a LICENSE scan fails `extractLicenses` accepts a nil location, but the scan error log added earlier on this branch dereferenced it. Signed-off-by: Alex Goodman * fix(gentoo): bound license group expansion on hostile license_groups - dedup within a group uses a set instead of `slices.Contains`, which was quadratic in tokens per line and got much worse under the 1MB line cap - each group is expanded once and memoized, so a chain of groups that reference each other repeatedly no longer expands exponentially - repeated references to the same group within one line are skipped Signed-off-by: Alex Goodman --------- Signed-off-by: Alex Goodman --- cmd/syft/cli/ui/handle_attestation.go | 5 + internal/line_scanner.go | 19 +++ internal/line_scanner_test.go | 24 +++ syft/format/cpes/decoder.go | 6 +- syft/format/purls/decoder.go | 6 +- syft/pkg/cataloger/arch/parse_alpm_db.go | 7 +- syft/pkg/cataloger/debian/parse_copyright.go | 7 +- syft/pkg/cataloger/gentoo/license.go | 151 +++++++++--------- syft/pkg/cataloger/gentoo/license_test.go | 29 ++++ .../gentoo/parse_portage_contents.go | 11 +- syft/pkg/cataloger/golang/parse_go_mod.go | 6 +- .../homebrew/parse_homebrew_formula.go | 4 +- .../internal/dotnet/bundle/bundle.go | 2 +- .../cataloger/java/parse_gradle_lockfile.go | 9 +- .../java/parse_gradle_lockfile_test.go | 18 +++ .../pkg/cataloger/java/parse_java_manifest.go | 4 +- .../cataloger/java/parse_pom_properties.go | 4 +- syft/pkg/cataloger/nix/db_cataloger.go | 2 +- .../cataloger/python/parse_requirements.go | 3 +- syft/pkg/cataloger/python/parse_setup.go | 21 ++- .../python/parse_wheel_egg_metadata.go | 8 +- syft/pkg/cataloger/r/parse_description.go | 8 +- syft/pkg/cataloger/ruby/parse_gemfile_lock.go | 4 +- syft/pkg/cataloger/ruby/parse_gemspec.go | 7 +- .../cataloger/snap/parse_kernel_changelog.go | 7 +- syft/pkg/cataloger/wordpress/parse_plugin.go | 9 +- .../cataloger/wordpress/parse_plugin_test.go | 19 +++ 27 files changed, 280 insertions(+), 120 deletions(-) create mode 100644 internal/line_scanner.go create mode 100644 internal/line_scanner_test.go diff --git a/cmd/syft/cli/ui/handle_attestation.go b/cmd/syft/cli/ui/handle_attestation.go index 838d77c89..4c44410b5 100644 --- a/cmd/syft/cli/ui/handle_attestation.go +++ b/cmd/syft/cli/ui/handle_attestation.go @@ -158,6 +158,11 @@ func (l *backgroundLineReader) read(reader io.Reader, stage *progress.Stage) { l.lock.Unlock() } + if err := s.Err(); err != nil { + log.WithFields("error", err).Debug("unable to read attestation output") + // keep draining so the writer never blocks on a full pipe + _, _ = io.Copy(io.Discard, reader) + } } func (l backgroundLineReader) Lines() []string { diff --git a/internal/line_scanner.go b/internal/line_scanner.go new file mode 100644 index 000000000..ad4da5e29 --- /dev/null +++ b/internal/line_scanner.go @@ -0,0 +1,19 @@ +package internal + +import ( + "bufio" + "io" +) + +// maxScannedLineSize is the longest line NewLineScanner will return before Scan stops with bufio.ErrTooLong. +// The buffer only grows to this size when a line needs it. +const maxScannedLineSize = 1024 * 1024 + +// NewLineScanner returns a bufio.Scanner that accepts lines up to 1MB rather than the 64KB default, since +// package metadata files carry long single-line fields (descriptions, license text, dependency lists). Callers +// must still check Err() after scanning. +func NewLineScanner(r io.Reader) *bufio.Scanner { + s := bufio.NewScanner(r) + s.Buffer(nil, maxScannedLineSize) + return s +} diff --git a/internal/line_scanner_test.go b/internal/line_scanner_test.go new file mode 100644 index 000000000..f29f0928d --- /dev/null +++ b/internal/line_scanner_test.go @@ -0,0 +1,24 @@ +package internal + +import ( + "bufio" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNewLineScanner(t *testing.T) { + long := strings.Repeat("a", 100*1024) + s := NewLineScanner(strings.NewReader(long + "\nb\n")) + require.True(t, s.Scan()) + require.Equal(t, long, s.Text()) + require.True(t, s.Scan()) + require.Equal(t, "b", s.Text()) + require.False(t, s.Scan()) + require.NoError(t, s.Err()) + + s = NewLineScanner(strings.NewReader(strings.Repeat("a", maxScannedLineSize+1))) + require.False(t, s.Scan()) + require.ErrorIs(t, s.Err(), bufio.ErrTooLong) +} diff --git a/syft/format/cpes/decoder.go b/syft/format/cpes/decoder.go index b5d3ab62e..9063f6fae 100644 --- a/syft/format/cpes/decoder.go +++ b/syft/format/cpes/decoder.go @@ -7,6 +7,7 @@ import ( "io" "strings" + syftinternal "github.com/anchore/syft/internal" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/cpe" "github.com/anchore/syft/syft/format/internal" @@ -61,7 +62,7 @@ func toSyftModel(r io.Reader) (*sbom.SBOM, error) { var errs []error pkgs := pkg.NewCollection() - scanner := bufio.NewScanner(r) + scanner := syftinternal.NewLineScanner(r) for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) @@ -86,6 +87,9 @@ func toSyftModel(r io.Reader) (*sbom.SBOM, error) { p.SetID() pkgs.Add(p) } + if err := scanner.Err(); err != nil { + errs = append(errs, err) + } return &sbom.SBOM{ Artifacts: sbom.Artifacts{ diff --git a/syft/format/purls/decoder.go b/syft/format/purls/decoder.go index 9cce2dcf5..89e37f739 100644 --- a/syft/format/purls/decoder.go +++ b/syft/format/purls/decoder.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/anchore/packageurl-go" + syftinternal "github.com/anchore/syft/internal" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/format/internal" "github.com/anchore/syft/syft/pkg" @@ -61,7 +62,7 @@ func toSyftModel(r io.Reader) (*sbom.SBOM, error) { var errs []error pkgs := pkg.NewCollection() - scanner := bufio.NewScanner(r) + scanner := syftinternal.NewLineScanner(r) for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) @@ -84,6 +85,9 @@ func toSyftModel(r io.Reader) (*sbom.SBOM, error) { p.SetID() pkgs.Add(p) } + if err := scanner.Err(); err != nil { + errs = append(errs, err) + } return &sbom.SBOM{ Artifacts: sbom.Artifacts{ diff --git a/syft/pkg/cataloger/arch/parse_alpm_db.go b/syft/pkg/cataloger/arch/parse_alpm_db.go index fe9762087..db7505be6 100644 --- a/syft/pkg/cataloger/arch/parse_alpm_db.go +++ b/syft/pkg/cataloger/arch/parse_alpm_db.go @@ -148,12 +148,7 @@ func parseAlpmDBEntry(reader io.Reader) (*parsedData, error) { } func newScanner(reader io.Reader) *bufio.Scanner { - // This is taken from the apk parser - // https://github.com/anchore/syft/blob/v0.47.0/syft/pkg/cataloger/apkdb/parse_apk_db.go#L37 - const maxScannerCapacity = 1024 * 1024 - bufScan := make([]byte, maxScannerCapacity) - scanner := bufio.NewScanner(reader) - scanner.Buffer(bufScan, maxScannerCapacity) + scanner := internal.NewLineScanner(reader) onDoubleLF := func(data []byte, atEOF bool) (advance int, token []byte, err error) { for i := range data { if i > 0 && data[i-1] == '\n' && data[i] == '\n' { diff --git a/syft/pkg/cataloger/debian/parse_copyright.go b/syft/pkg/cataloger/debian/parse_copyright.go index e6936ce2d..0b5e86647 100644 --- a/syft/pkg/cataloger/debian/parse_copyright.go +++ b/syft/pkg/cataloger/debian/parse_copyright.go @@ -1,7 +1,6 @@ package debian import ( - "bufio" "io" "regexp" "sort" @@ -10,6 +9,7 @@ import ( "github.com/scylladb/go-set/strset" "github.com/anchore/syft/internal" + "github.com/anchore/syft/internal/log" ) // For more information see: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/#license-syntax @@ -34,7 +34,7 @@ const ( func parseLicensesFromCopyright(reader io.Reader) []string { findings := strset.New() - scanner := bufio.NewScanner(reader) + scanner := internal.NewLineScanner(reader) // per the DEP-5 spec, machine-readable copyright files MUST have a // Format field whose value is a URI for the specification. Only files @@ -69,6 +69,9 @@ func parseLicensesFromCopyright(reader io.Reader) []string { findings.Add(found) } } + if err := scanner.Err(); err != nil { + log.WithFields("error", err).Debug("failed to fully read debian copyright file") + } if !formatVerified { return nil diff --git a/syft/pkg/cataloger/gentoo/license.go b/syft/pkg/cataloger/gentoo/license.go index ee23c0372..4fedf5938 100644 --- a/syft/pkg/cataloger/gentoo/license.go +++ b/syft/pkg/cataloger/gentoo/license.go @@ -5,7 +5,6 @@ import ( "bytes" "fmt" "io" - "slices" "strings" "github.com/scylladb/go-set/strset" @@ -35,7 +34,7 @@ import ( func extractLicenses(resolver file.Resolver, closestLocation *file.Location, reader io.Reader) (string, string) { findings := strset.New() contentsWriter := bytes.Buffer{} - scanner := bufio.NewScanner(io.TeeReader(reader, &contentsWriter)) + scanner := internal.NewLineScanner(io.TeeReader(reader, &contentsWriter)) scanner.Split(bufio.ScanWords) var ( mandatoryLicenses, conditionalLicenses, useflagLicenses []string @@ -69,6 +68,13 @@ func extractLicenses(resolver file.Resolver, closestLocation *file.Location, rea } } } + if err := scanner.Err(); err != nil { + fields := []any{"error", err} + if closestLocation != nil { + fields = append(fields, "path", closestLocation.RealPath) + } + log.WithFields(fields...).Debug("failed to fully read portage LICENSE") + } var licenseGroups map[string][]string if usesGroups { @@ -80,31 +86,22 @@ func extractLicenses(resolver file.Resolver, closestLocation *file.Location, rea findings.Add(conditionalLicenses...) findings.Add(useflagLicenses...) - var mandatoryStatement, conditionalStatement string + return strings.TrimSpace(contentsWriter.String()), licenseExpression(mandatoryLicenses, conditionalLicenses) +} - // attempt to build valid SPDX license expression - if len(mandatoryLicenses) > 0 { - mandatoryStatement = strings.Join(mandatoryLicenses, " AND ") +// licenseExpression attempts to build a valid SPDX license expression +func licenseExpression(mandatoryLicenses, conditionalLicenses []string) string { + mandatoryStatement := strings.Join(mandatoryLicenses, " AND ") + conditionalStatement := strings.Join(conditionalLicenses, " OR ") + + switch { + case mandatoryStatement != "" && conditionalStatement != "": + return mandatoryStatement + " AND (" + conditionalStatement + ")" + case mandatoryStatement != "": + return mandatoryStatement + default: + return conditionalStatement } - if len(conditionalLicenses) > 0 { - conditionalStatement = strings.Join(conditionalLicenses, " OR ") - } - - contents := strings.TrimSpace(contentsWriter.String()) - - if mandatoryStatement != "" && conditionalStatement != "" { - return contents, mandatoryStatement + " AND (" + conditionalStatement + ")" - } - - if mandatoryStatement != "" { - return contents, mandatoryStatement - } - - if conditionalStatement != "" { - return contents, conditionalStatement - } - - return contents, "" } func readLicenseGroups(resolver file.Resolver, closestLocation *file.Location) map[string][]string { @@ -161,10 +158,9 @@ func replaceLicenseGroups(licenses []string, groups map[string][]string) []strin } func parseLicenseGroups(reader io.Reader) (map[string][]string, error) { - result := make(map[string][]string) rawGroups := make(map[string][]string) - scanner := bufio.NewScanner(reader) + scanner := internal.NewLineScanner(reader) // first collect all raw groups for scanner.Scan() { @@ -191,58 +187,69 @@ func parseLicenseGroups(reader io.Reader) (map[string][]string, error) { } // next process each group to expand nested references - for groupName, licenses := range rawGroups { - expanded, err := expandLicenses(groupName, licenses, rawGroups, make(map[string]bool)) + expanded := make(map[string][]string) + for groupName := range rawGroups { + if _, err := expandLicenses(groupName, rawGroups, expanded, make(map[string]bool)); err != nil { + return nil, err + } + } + + return expanded, nil +} + +// expandLicenses handles the recursive expansion of license groups. Each group is expanded once and memoized in +// 'expanded' (the file is untrusted, so re-expanding on every reference would be exponential), and 'visiting' holds +// the groups on the current path to detect cycles. We are always in terms of slices instead of sets to ensure +// original ordering is preserved. +func expandLicenses(currentGroup string, rawGroups, expanded map[string][]string, visiting map[string]bool) ([]string, error) { + if result, ok := expanded[currentGroup]; ok { + return result, nil + } + if visiting[currentGroup] { + return nil, fmt.Errorf("cycle detected in license group definitions for group: %s", currentGroup) + } + visiting[currentGroup] = true + defer delete(visiting, currentGroup) + + result := make([]string, 0) + // sets keep dedup linear, since a single group line can hold hundreds of thousands of tokens + seen := strset.New() + seenRefs := strset.New() + + for _, item := range rawGroups[currentGroup] { + refGroupName, isRef := strings.CutPrefix(item, "@") + if !isRef { + // this is a regular license + if !seen.Has(item) { + seen.Add(item) + result = append(result, item) + } + continue + } + + // this is a reference to another group + if seenRefs.Has(refGroupName) { + continue + } + seenRefs.Add(refGroupName) + + if _, exists := rawGroups[refGroupName]; !exists { + return nil, fmt.Errorf("referenced group not found: %s", refGroupName) + } + + refLicenses, err := expandLicenses(refGroupName, rawGroups, expanded, visiting) if err != nil { return nil, err } - result[groupName] = expanded - } - return result, nil -} - -// expandLicenses handles the recursive expansion of license groups, 'visited' is used to detect cycles. We are always -// in terms of slices instead of sets to ensure original ordering is preserved. -func expandLicenses(currentGroup string, licenses []string, rawGroups map[string][]string, visited map[string]bool) ([]string, error) { - if visited[currentGroup] { - return nil, fmt.Errorf("cycle detected in license group definitions for group: %s", currentGroup) - } - - visited[currentGroup] = true - - result := make([]string, 0) - - for _, item := range licenses { - if strings.HasPrefix(item, "@") { - // this is a reference to another group - refGroupName := item[1:] // remove '@' prefix - - refLicenses, exists := rawGroups[refGroupName] - if !exists { - return nil, fmt.Errorf("referenced group not found: %s", refGroupName) + for _, license := range refLicenses { + if !seen.Has(license) { + seen.Add(license) + result = append(result, license) } - - newVisited := make(map[string]bool) - for k, v := range visited { - newVisited[k] = v - } - - expanded, err := expandLicenses(refGroupName, refLicenses, rawGroups, newVisited) - if err != nil { - return nil, err - } - - for _, license := range expanded { - if !slices.Contains(result, license) { - result = append(result, license) - } - } - } else if !slices.Contains(result, item) { - // ...this is a regular license - result = append(result, item) } } + expanded[currentGroup] = result return result, nil } diff --git a/syft/pkg/cataloger/gentoo/license_test.go b/syft/pkg/cataloger/gentoo/license_test.go index e533c2636..47b0cfda7 100644 --- a/syft/pkg/cataloger/gentoo/license_test.go +++ b/syft/pkg/cataloger/gentoo/license_test.go @@ -2,6 +2,7 @@ package gentoo import ( "bytes" + "fmt" "os" "strings" "testing" @@ -181,3 +182,31 @@ func TestReplaceLicenseGroups(t *testing.T) { }) } } + +func Test_extractLicenses_tokenTooLong(t *testing.T) { + // a nil location must not panic when the scan fails partway through + license := "MIT " + strings.Repeat("a", 2<<20) + _, expression := extractLicenses(nil, nil, strings.NewReader(license)) + assert.Equal(t, "MIT", expression) +} + +func TestParseLicenseGroups_hostileInput(t *testing.T) { + // a single wide group was quadratic to dedup, and a chain where each group references the next + // twice was exponential to expand; both must now finish quickly + var wide strings.Builder + wide.WriteString("WIDE") + for i := range 120_000 { + fmt.Fprintf(&wide, " L%d", i) + } + + var chain strings.Builder + chain.WriteString("G0 MIT\n") + for i := 1; i <= 64; i++ { + fmt.Fprintf(&chain, "G%d @G%d @G%d\n", i, i-1, i-1) + } + + groups, err := parseLicenseGroups(strings.NewReader(wide.String() + "\n" + chain.String())) + require.NoError(t, err) + assert.Len(t, groups["WIDE"], 120_000) + assert.Equal(t, []string{"MIT"}, groups["G64"]) +} diff --git a/syft/pkg/cataloger/gentoo/parse_portage_contents.go b/syft/pkg/cataloger/gentoo/parse_portage_contents.go index 7d4d1d8c2..db9d68035 100644 --- a/syft/pkg/cataloger/gentoo/parse_portage_contents.go +++ b/syft/pkg/cataloger/gentoo/parse_portage_contents.go @@ -1,7 +1,6 @@ package gentoo import ( - "bufio" "context" "fmt" "path" @@ -70,7 +69,7 @@ func addFiles(resolver file.Resolver, dbLocation file.Location, entry *pkg.Porta } defer internal.CloseAndLogError(contentsReader, dbLocation.RealPath) - scanner := bufio.NewScanner(contentsReader) + scanner := internal.NewLineScanner(contentsReader) for scanner.Scan() { line := strings.Trim(scanner.Text(), "\n") fields := strings.Split(line, " ") @@ -87,6 +86,9 @@ func addFiles(resolver file.Resolver, dbLocation file.Location, entry *pkg.Porta entry.Files = append(entry.Files, record) } } + if err := scanner.Err(); err != nil { + log.WithFields("path", dbLocation.RealPath, "error", err).Debug("failed to fully read portage contents") + } } func addLicenses(ctx context.Context, resolver file.Resolver, dbLocation file.Location, entry *pkg.PortageEntry) (pkg.LicenseSet, []file.Location) { @@ -128,7 +130,7 @@ func addSize(resolver file.Resolver, dbLocation file.Location, entry *pkg.Portag } defer internal.CloseAndLogError(sizeReader, location.RealPath) - scanner := bufio.NewScanner(sizeReader) + scanner := internal.NewLineScanner(sizeReader) for scanner.Scan() { line := strings.Trim(scanner.Text(), "\n") size, err := strconv.Atoi(line) @@ -136,6 +138,9 @@ func addSize(resolver file.Resolver, dbLocation file.Location, entry *pkg.Portag entry.InstalledSize = size } } + if err := scanner.Err(); err != nil { + log.WithFields("path", location.RealPath, "error", err).Debug("failed to fully read portage SIZE") + } return []file.Location{location.WithAnnotation(pkg.EvidenceAnnotationKey, pkg.SupportingEvidenceAnnotation)} } diff --git a/syft/pkg/cataloger/golang/parse_go_mod.go b/syft/pkg/cataloger/golang/parse_go_mod.go index 4a40403b1..e9d021729 100644 --- a/syft/pkg/cataloger/golang/parse_go_mod.go +++ b/syft/pkg/cataloger/golang/parse_go_mod.go @@ -1,7 +1,6 @@ package golang import ( - "bufio" "context" "fmt" "go/build" @@ -437,7 +436,7 @@ func parseGoSumFile(resolver file.Resolver, reader file.LocationReadCloser) (map // github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= // github.com/BurntSushi/toml v0.4.1 h1:GaI7EiDXDRfa8VshkTj7Fym7ha+y8/XxIgD2okUIjLw= // github.com/BurntSushi/toml v0.4.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= - scanner := bufio.NewScanner(contents) + scanner := internal.NewLineScanner(contents) // optionally, resize scanner's capacity for lines over 64K, see next example for scanner.Scan() { line := scanner.Text() @@ -449,6 +448,9 @@ func parseGoSumFile(resolver file.Resolver, reader file.LocationReadCloser) (map hash := parts[2] out[nameVersion] = hash } + if err := scanner.Err(); err != nil { + return out, fmt.Errorf("unable to read go.sum: %w", err) + } return out, nil } diff --git a/syft/pkg/cataloger/homebrew/parse_homebrew_formula.go b/syft/pkg/cataloger/homebrew/parse_homebrew_formula.go index 1255ccbc6..93d866746 100644 --- a/syft/pkg/cataloger/homebrew/parse_homebrew_formula.go +++ b/syft/pkg/cataloger/homebrew/parse_homebrew_formula.go @@ -1,11 +1,11 @@ package homebrew import ( - "bufio" "context" "path" "strings" + "github.com/anchore/syft/internal" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" @@ -46,7 +46,7 @@ func parseHomebrewFormula(ctx context.Context, resolver file.Resolver, _ *generi func parseFormulaFile(reader file.LocationReadCloser) (*parsedHomebrewData, error) { pd := parsedHomebrewData{} - scanner := bufio.NewScanner(reader) + scanner := internal.NewLineScanner(reader) for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) if strings.Contains(line, "class ") && strings.Contains(line, " < Formula") { diff --git a/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go b/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go index 3bead6fb9..434b1d2ac 100644 --- a/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go +++ b/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go @@ -209,7 +209,7 @@ func read7BitEncodedInt(r io.Reader) (int, error) { shift := 0 for { var b [1]byte - if _, err := r.Read(b[:]); err != nil { + if _, err := io.ReadFull(r, b[:]); err != nil { return 0, err } result |= int(b[0]&0x7F) << shift diff --git a/syft/pkg/cataloger/java/parse_gradle_lockfile.go b/syft/pkg/cataloger/java/parse_gradle_lockfile.go index 4ba86afb3..4e9e8c694 100644 --- a/syft/pkg/cataloger/java/parse_gradle_lockfile.go +++ b/syft/pkg/cataloger/java/parse_gradle_lockfile.go @@ -1,10 +1,11 @@ package java import ( - "bufio" "context" + "fmt" "strings" + "github.com/anchore/syft/internal" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" "github.com/anchore/syft/syft/pkg" @@ -22,7 +23,7 @@ func parseGradleLockfile(_ context.Context, _ file.Resolver, _ *generic.Environm var pkgs []pkg.Package // Create a new scanner to read the file - scanner := bufio.NewScanner(reader) + scanner := internal.NewLineScanner(reader) // Create slices to hold the dependencies and plugins dependencies := []lockfileDependency{} @@ -45,6 +46,7 @@ func parseGradleLockfile(_ context.Context, _ file.Resolver, _ *generic.Environm dependencies = append(dependencies, dep) } } + scanErr := scanner.Err() // map the dependencies for _, dep := range dependencies { @@ -72,5 +74,8 @@ func parseGradleLockfile(_ context.Context, _ file.Resolver, _ *generic.Environm pkgs = append(pkgs, mappedPkg) } + if scanErr != nil { + return pkgs, nil, fmt.Errorf("unable to read gradle lockfile: %w", scanErr) + } return pkgs, nil, nil } diff --git a/syft/pkg/cataloger/java/parse_gradle_lockfile_test.go b/syft/pkg/cataloger/java/parse_gradle_lockfile_test.go index d6fc4c2a8..a9208a1b8 100644 --- a/syft/pkg/cataloger/java/parse_gradle_lockfile_test.go +++ b/syft/pkg/cataloger/java/parse_gradle_lockfile_test.go @@ -1,8 +1,15 @@ package java import ( + "bufio" + "context" + "io" + "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/anchore/syft/syft/file" "github.com/anchore/syft/syft/pkg" "github.com/anchore/syft/syft/pkg/cataloger/internal/pkgtest" @@ -69,3 +76,14 @@ func Test_parserGradleLockfile(t *testing.T) { }) } } + +func Test_parserGradleLockfile_lineTooLong(t *testing.T) { + // packages before an oversized line are kept, and the scan error is still reported + contents := "org.example:before:1.0=compileClasspath\n" + strings.Repeat("a", 2<<20) + "\norg.example:after:2.0=compileClasspath\n" + reader := file.NewLocationReadCloser(file.NewLocation("gradle.lockfile"), io.NopCloser(strings.NewReader(contents))) + + pkgs, _, err := parseGradleLockfile(context.Background(), nil, nil, reader) + require.ErrorIs(t, err, bufio.ErrTooLong) + require.Len(t, pkgs, 1) + assert.Equal(t, "before", pkgs[0].Name) +} diff --git a/syft/pkg/cataloger/java/parse_java_manifest.go b/syft/pkg/cataloger/java/parse_java_manifest.go index cb7945eca..0c1e3129a 100644 --- a/syft/pkg/cataloger/java/parse_java_manifest.go +++ b/syft/pkg/cataloger/java/parse_java_manifest.go @@ -1,12 +1,12 @@ package java import ( - "bufio" "fmt" "io" "strings" "unicode" + "github.com/anchore/syft/internal" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/pkg" ) @@ -26,7 +26,7 @@ func parseJavaManifest(path string, reader io.Reader) (*pkg.JavaManifest, error) } var lastKey string - scanner := bufio.NewScanner(reader) + scanner := internal.NewLineScanner(reader) for scanner.Scan() { line := scanner.Text() diff --git a/syft/pkg/cataloger/java/parse_pom_properties.go b/syft/pkg/cataloger/java/parse_pom_properties.go index 8ef3676be..b4704b099 100644 --- a/syft/pkg/cataloger/java/parse_pom_properties.go +++ b/syft/pkg/cataloger/java/parse_pom_properties.go @@ -1,13 +1,13 @@ package java import ( - "bufio" "fmt" "io" "strings" "github.com/go-viper/mapstructure/v2" + "github.com/anchore/syft/internal" "github.com/anchore/syft/syft/pkg" ) @@ -16,7 +16,7 @@ const pomPropertiesGlob = "**/*pom.properties" func parsePomProperties(path string, reader io.Reader) (*pkg.JavaPomProperties, error) { var props pkg.JavaPomProperties propMap := make(map[string]string) - scanner := bufio.NewScanner(reader) + scanner := internal.NewLineScanner(reader) for scanner.Scan() { line := scanner.Text() diff --git a/syft/pkg/cataloger/nix/db_cataloger.go b/syft/pkg/cataloger/nix/db_cataloger.go index 615e213ef..825895b69 100644 --- a/syft/pkg/cataloger/nix/db_cataloger.go +++ b/syft/pkg/cataloger/nix/db_cataloger.go @@ -95,7 +95,7 @@ func (c dbCataloger) selectDBParser(dbLocation file.Location, resolver file.Reso scanner := bufio.NewScanner(schemaContents) if !scanner.Scan() { - log.WithFields("path", loc.RealPath).Tracef("failed to read Nix database schema file, assuming %d", defaultSchema) + log.WithFields("path", loc.RealPath, "error", scanner.Err()).Tracef("failed to read Nix database schema file, assuming %d", defaultSchema) return c.schemaProcessor[defaultSchema], 0 } diff --git a/syft/pkg/cataloger/python/parse_requirements.go b/syft/pkg/cataloger/python/parse_requirements.go index d167c9d16..6622496fd 100644 --- a/syft/pkg/cataloger/python/parse_requirements.go +++ b/syft/pkg/cataloger/python/parse_requirements.go @@ -1,7 +1,6 @@ package python import ( - "bufio" "context" "fmt" "regexp" @@ -105,7 +104,7 @@ func (rp requirementsParser) parseRequirementsTxt(ctx context.Context, _ file.Re var errs error var packages []pkg.Package - scanner := bufio.NewScanner(reader) + scanner := internal.NewLineScanner(reader) var lastLine string for scanner.Scan() { line := trimRequirementsTxtLine(scanner.Text()) diff --git a/syft/pkg/cataloger/python/parse_setup.go b/syft/pkg/cataloger/python/parse_setup.go index a42c785ac..d8cff06de 100644 --- a/syft/pkg/cataloger/python/parse_setup.go +++ b/syft/pkg/cataloger/python/parse_setup.go @@ -1,11 +1,12 @@ package python import ( - "bufio" "context" + "fmt" "regexp" "strings" + "github.com/anchore/syft/internal" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" @@ -36,7 +37,7 @@ var unquotedPinnedDependency = regexp.MustCompile(`^\s*(\w+)\s*==\s*([\w\.\-]+)` func (sp setupFileParser) parseSetupFile(ctx context.Context, _ file.Resolver, _ *generic.Environment, reader file.LocationReadCloser) ([]pkg.Package, []artifact.Relationship, error) { var packages []pkg.Package - scanner := bufio.NewScanner(reader) + scanner := internal.NewLineScanner(reader) for scanner.Scan() { line := scanner.Text() @@ -45,20 +46,23 @@ func (sp setupFileParser) parseSetupFile(ctx context.Context, _ file.Resolver, _ packages = sp.processQuotedDependencies(ctx, line, reader, packages) packages = sp.processUnquotedDependency(ctx, line, reader, packages) } + if err := scanner.Err(); err != nil { + return packages, nil, fmt.Errorf("unable to read setup.py: %w", err) + } return packages, nil, nil } func (sp setupFileParser) processQuotedDependencies(ctx context.Context, line string, reader file.LocationReadCloser, packages []pkg.Package) []pkg.Package { for _, match := range pinnedDependency.FindAllString(line, -1) { - if p, ok := sp.parseQuotedDependency(ctx, match, line, reader); ok { + if p, ok := sp.parseQuotedDependency(ctx, match, reader); ok { packages = append(packages, p) } } return packages } -func (sp setupFileParser) parseQuotedDependency(ctx context.Context, match, line string, reader file.LocationReadCloser) (pkg.Package, bool) { +func (sp setupFileParser) parseQuotedDependency(ctx context.Context, match string, reader file.LocationReadCloser) (pkg.Package, bool) { parts := strings.Split(match, "==") if len(parts) != 2 { return pkg.Package{}, false @@ -67,7 +71,7 @@ func (sp setupFileParser) parseQuotedDependency(ctx context.Context, match, line name := cleanDependencyString(parts[0]) version := cleanDependencyString(parts[len(parts)-1]) - return sp.validateAndCreatePackage(ctx, name, version, line, reader) + return sp.validateAndCreatePackage(ctx, name, version, reader) } // processUnquotedDependency extracts and processes an unquoted dependency from a line @@ -80,7 +84,7 @@ func (sp setupFileParser) processUnquotedDependency(ctx context.Context, line st name := strings.TrimSpace(matches[1]) version := strings.TrimSpace(matches[2]) - if p, ok := sp.validateAndCreatePackage(ctx, name, version, line, reader); ok { + if p, ok := sp.validateAndCreatePackage(ctx, name, version, reader); ok { if !isDuplicatePackage(p, packages) { packages = append(packages, p) } @@ -96,14 +100,15 @@ func cleanDependencyString(s string) string { return s } -func (sp setupFileParser) validateAndCreatePackage(ctx context.Context, name, version, line string, reader file.LocationReadCloser) (pkg.Package, bool) { +func (sp setupFileParser) validateAndCreatePackage(ctx context.Context, name, version string, reader file.LocationReadCloser) (pkg.Package, bool) { if hasTemplateDirective(name) || hasTemplateDirective(version) { // this can happen in more dynamic setup.py where there is templating return pkg.Package{}, false } if name == "" || version == "" { - log.WithFields("path", reader.RealPath).Debugf("unable to parse package in setup.py line: %q", line) + // the raw line is not logged since this runs once per match, which is quadratic on a long line + log.WithFields("path", reader.RealPath, "name", name, "version", version).Debug("unable to parse package in setup.py") return pkg.Package{}, false } diff --git a/syft/pkg/cataloger/python/parse_wheel_egg_metadata.go b/syft/pkg/cataloger/python/parse_wheel_egg_metadata.go index 0cb060a23..5b0572e3a 100644 --- a/syft/pkg/cataloger/python/parse_wheel_egg_metadata.go +++ b/syft/pkg/cataloger/python/parse_wheel_egg_metadata.go @@ -1,7 +1,6 @@ package python import ( - "bufio" "fmt" "path/filepath" "strings" @@ -9,6 +8,7 @@ import ( "github.com/bmatcuk/doublestar/v4" "github.com/go-viper/mapstructure/v2" + "github.com/anchore/syft/internal" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/file" "github.com/anchore/syft/syft/pkg" @@ -68,7 +68,7 @@ func extractRFC5322Fields(locationReader file.LocationReadCloser) (map[string]an // though this spec is governed by RFC 5322 (mail message), the metadata files are not guaranteed to be compliant. // We must survive parsing as much info as possible without failing and dropping the data. - scanner := bufio.NewScanner(locationReader) + scanner := internal.NewLineScanner(locationReader) for scanner.Scan() { line := scanner.Text() line = strings.TrimRight(line, "\n") @@ -108,6 +108,10 @@ func extractRFC5322Fields(locationReader file.LocationReadCloser) (map[string]an } } } + if err := scanner.Err(); err != nil { + // the caller discards everything on error, so keep the partial fields and log instead + log.WithFields("path", locationReader.Path(), "error", err).Debug("failed to fully read python wheel/egg metadata") + } return fields, nil } diff --git a/syft/pkg/cataloger/r/parse_description.go b/syft/pkg/cataloger/r/parse_description.go index 01aa89f99..340e395ec 100644 --- a/syft/pkg/cataloger/r/parse_description.go +++ b/syft/pkg/cataloger/r/parse_description.go @@ -1,12 +1,13 @@ package r import ( - "bufio" "context" "io" "regexp" "strings" + "github.com/anchore/syft/internal" + "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" "github.com/anchore/syft/syft/pkg" @@ -105,7 +106,7 @@ func extractFieldsFromDescriptionFile(reader io.Reader) map[string]string { result := make(map[string]string) key := "" var valueFragment strings.Builder - scanner := bufio.NewScanner(reader) + scanner := internal.NewLineScanner(reader) for scanner.Scan() { line := scanner.Text() @@ -139,6 +140,9 @@ func extractFieldsFromDescriptionFile(reader io.Reader) map[string]string { if key != "" { result[key] = valueFragment.String() } + if err := scanner.Err(); err != nil { + log.WithFields("error", err).Debug("failed to fully read R DESCRIPTION file") + } return result } diff --git a/syft/pkg/cataloger/ruby/parse_gemfile_lock.go b/syft/pkg/cataloger/ruby/parse_gemfile_lock.go index d8fe50850..29bc1cac3 100644 --- a/syft/pkg/cataloger/ruby/parse_gemfile_lock.go +++ b/syft/pkg/cataloger/ruby/parse_gemfile_lock.go @@ -1,12 +1,12 @@ package ruby import ( - "bufio" "context" "strings" "github.com/scylladb/go-set/strset" + "github.com/anchore/syft/internal" "github.com/anchore/syft/internal/unknown" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" @@ -21,7 +21,7 @@ var sectionsOfInterest = strset.New("GEM", "GIT", "PATH", "PLUGIN SOURCE") // parseGemFileLockEntries is a parser function for Gemfile.lock contents, returning all Gems discovered. func parseGemFileLockEntries(_ context.Context, _ file.Resolver, _ *generic.Environment, reader file.LocationReadCloser) ([]pkg.Package, []artifact.Relationship, error) { var pkgs []pkg.Package - scanner := bufio.NewScanner(reader) + scanner := internal.NewLineScanner(reader) var currentSection string diff --git a/syft/pkg/cataloger/ruby/parse_gemspec.go b/syft/pkg/cataloger/ruby/parse_gemspec.go index 92b586987..144fe7878 100644 --- a/syft/pkg/cataloger/ruby/parse_gemspec.go +++ b/syft/pkg/cataloger/ruby/parse_gemspec.go @@ -1,7 +1,6 @@ package ruby import ( - "bufio" "context" "encoding/json" "fmt" @@ -75,7 +74,7 @@ func processList(s string) []string { func parseGemSpecEntries(ctx context.Context, resolver file.Resolver, _ *generic.Environment, reader file.LocationReadCloser) ([]pkg.Package, []artifact.Relationship, error) { var pkgs []pkg.Package var fields = make(map[string]any) - scanner := bufio.NewScanner(reader) + scanner := internal.NewLineScanner(reader) for scanner.Scan() { line := scanner.Text() @@ -101,6 +100,7 @@ func parseGemSpecEntries(ctx context.Context, resolver file.Resolver, _ *generic } } } + scanErr := scanner.Err() resolveRubyInterpolationsInFields(fields) @@ -121,6 +121,9 @@ func parseGemSpecEntries(ctx context.Context, resolver file.Resolver, _ *generic ) } + if scanErr != nil { + return pkgs, nil, fmt.Errorf("unable to read gemspec: %w", scanErr) + } return pkgs, nil, nil } diff --git a/syft/pkg/cataloger/snap/parse_kernel_changelog.go b/syft/pkg/cataloger/snap/parse_kernel_changelog.go index 5b5a81b4f..7ad37f300 100644 --- a/syft/pkg/cataloger/snap/parse_kernel_changelog.go +++ b/syft/pkg/cataloger/snap/parse_kernel_changelog.go @@ -1,13 +1,13 @@ package snap import ( - "bufio" "compress/gzip" "context" "fmt" "regexp" "strings" + "github.com/anchore/syft/internal" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" "github.com/anchore/syft/syft/pkg" @@ -32,7 +32,7 @@ func parseKernelChangelog(_ context.Context, _ file.Resolver, _ *generic.Environ } defer gzReader.Close() - scanner := bufio.NewScanner(gzReader) + scanner := internal.NewLineScanner(gzReader) // read the first line to extract kernel version // Format: "linux (5.4.0-195.215) focal; urgency=medium" @@ -65,6 +65,9 @@ func parseKernelChangelog(_ context.Context, _ file.Resolver, _ *generic.Environ break } } + if err := scanner.Err(); err != nil { + return packages, nil, fmt.Errorf("failed to read changelog content: %w", err) + } return packages, nil, nil } diff --git a/syft/pkg/cataloger/wordpress/parse_plugin.go b/syft/pkg/cataloger/wordpress/parse_plugin.go index 4956dc6f8..a3de5060c 100644 --- a/syft/pkg/cataloger/wordpress/parse_plugin.go +++ b/syft/pkg/cataloger/wordpress/parse_plugin.go @@ -2,7 +2,9 @@ package wordpress import ( "context" + "errors" "fmt" + "io" "path/filepath" "regexp" "strings" @@ -42,12 +44,13 @@ func parseWordpressPluginFiles(ctx context.Context, resolver file.Resolver, _ *g var pkgs []pkg.Package buffer := make([]byte, contentBufferSize) - _, err := reader.Read(buffer) - if err != nil { + // a file shorter than the buffer is expected, so only the header it does have is parsed + n, err := io.ReadFull(reader, buffer) + if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) { return nil, nil, fmt.Errorf("failed to read %s file: %w", reader.Path(), err) } - fields := extractFields(string(buffer)) + fields := extractFields(string(buffer[:n])) name, nameOk := fields["name"] version, versionOk := fields["version"] diff --git a/syft/pkg/cataloger/wordpress/parse_plugin_test.go b/syft/pkg/cataloger/wordpress/parse_plugin_test.go index 640f1d42b..97a3941c4 100644 --- a/syft/pkg/cataloger/wordpress/parse_plugin_test.go +++ b/syft/pkg/cataloger/wordpress/parse_plugin_test.go @@ -2,9 +2,13 @@ package wordpress import ( "context" + "io" + "os" "testing" + "testing/iotest" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/anchore/syft/syft/file" "github.com/anchore/syft/syft/pkg" @@ -58,3 +62,18 @@ func Test_extractFields(t *testing.T) { }) } } + +func TestParseWordpressPluginFiles_shortReads(t *testing.T) { + // a reader that returns one byte per Read must still yield the full header + fixture := "testdata/glob-paths/wp-content/plugins/akismet/akismet.php" + f, err := os.Open(fixture) + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + reader := file.NewLocationReadCloser(file.NewLocation(fixture), io.NopCloser(iotest.OneByteReader(f))) + + pkgs, _, err := parseWordpressPluginFiles(context.Background(), nil, nil, reader) + require.NoError(t, err) + require.Len(t, pkgs, 1) + assert.Equal(t, "Akismet Anti-spam: Spam Protection", pkgs[0].Name) + assert.Equal(t, "5.3", pkgs[0].Version) +}