From 7a69e2129b7b3ef13ac16f39fff1f47eef6b37f8 Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Tue, 29 Nov 2022 10:56:46 -0500 Subject: [PATCH] recover from bad parsing of golang binary (#1371) Signed-off-by: Alex Goodman Signed-off-by: Alex Goodman --- syft/pkg/cataloger/generic/cataloger.go | 2 + syft/pkg/cataloger/golang/scan_binary.go | 52 ++++++++++++------- syft/pkg/cataloger/golang/scan_binary_test.go | 40 ++++++++++++++ 3 files changed, 76 insertions(+), 18 deletions(-) create mode 100644 syft/pkg/cataloger/golang/scan_binary_test.go diff --git a/syft/pkg/cataloger/generic/cataloger.go b/syft/pkg/cataloger/generic/cataloger.go index c843c680d..b9341bb34 100644 --- a/syft/pkg/cataloger/generic/cataloger.go +++ b/syft/pkg/cataloger/generic/cataloger.go @@ -118,6 +118,8 @@ func (c *Cataloger) Catalog(resolver source.FileResolver) ([]pkg.Package, []arti for _, req := range c.selectFiles(resolver) { location, parser := req.Location, req.Parser + log.WithFields("path", location.RealPath).Trace("parsing file contents") + contentReader, err := resolver.FileContentsByLocation(location) if err != nil { logger.WithFields("location", location.RealPath, "error", err).Warn("unable to fetch contents") diff --git a/syft/pkg/cataloger/golang/scan_binary.go b/syft/pkg/cataloger/golang/scan_binary.go index 58ad1a3d4..f14484d69 100644 --- a/syft/pkg/cataloger/golang/scan_binary.go +++ b/syft/pkg/cataloger/golang/scan_binary.go @@ -2,6 +2,8 @@ package golang import ( "debug/buildinfo" + "fmt" + "io" "runtime/debug" "github.com/anchore/syft/internal/log" @@ -14,31 +16,20 @@ func scanFile(reader unionreader.UnionReader, filename string) ([]*debug.BuildIn // with more than one binary readers, err := unionreader.GetReaders(reader) if err != nil { - log.Warnf("golang cataloger: failed to open a binary: %v", err) + log.WithFields("error", err).Warnf("failed to open a golang binary") return nil, nil } var builds []*debug.BuildInfo for _, r := range readers { - bi, err := buildinfo.Read(r) - - // note: the stdlib does not export the error we need to check for + bi, err := getBuildInfo(r) if err != nil { - if err.Error() == "not a Go executable" { - // since the cataloger can only select executables and not distinguish if they are a go-compiled - // binary, we should not show warnings/logs in this case. - return nil, nil - } - // in this case we could not read the or parse the file, but not explicitly because it is not a - // go-compiled binary (though it still might be). - // TODO: We should change this back to "warn" eventually. - // But right now it's catching too many cases where the reader IS NOT a Go binary at all. - // It'd be great to see how we can get those cases to be detected and handled above before we get to - // this point in execution. - log.Infof("golang cataloger: unable to read buildinfo (file=%q): %v", filename, err) - return nil, nil + log.WithFields("file", filename, "error", err).Warn("unable to read golang buildinfo") + continue + } + if bi == nil { + continue } - builds = append(builds, bi) } @@ -46,3 +37,28 @@ func scanFile(reader unionreader.UnionReader, filename string) ([]*debug.BuildIn return builds, archs } + +func getBuildInfo(r io.ReaderAt) (bi *debug.BuildInfo, err error) { + defer func() { + if r := recover(); r != nil { + // this can happen in cases where a malformed binary is passed in can be initially parsed, but not + // used without error later down the line. This is the case with : + // https://github.com/llvm/llvm-project/blob/llvmorg-15.0.6/llvm/test/Object/Inputs/macho-invalid-dysymtab-bad-size + err = fmt.Errorf("recovered from panic: %v", r) + } + }() + bi, err = buildinfo.Read(r) + + // note: the stdlib does not export the error we need to check for + if err != nil { + if err.Error() == "not a Go executable" { + // since the cataloger can only select executables and not distinguish if they are a go-compiled + // binary, we should not show warnings/logs in this case. + return + } + // in this case we could not read the or parse the file, but not explicitly because it is not a + // go-compiled binary (though it still might be). + return + } + return +} diff --git a/syft/pkg/cataloger/golang/scan_binary_test.go b/syft/pkg/cataloger/golang/scan_binary_test.go new file mode 100644 index 000000000..8a5541137 --- /dev/null +++ b/syft/pkg/cataloger/golang/scan_binary_test.go @@ -0,0 +1,40 @@ +package golang + +import ( + "fmt" + "io" + "runtime/debug" + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_getBuildInfo(t *testing.T) { + type args struct { + r io.ReaderAt + } + tests := []struct { + name string + args args + wantBi *debug.BuildInfo + wantErr assert.ErrorAssertionFunc + }{ + { + name: "recover from panic", + args: args{ + r: nil, // trying to use a nil reader will cause a panic + }, + wantBi: nil, // we should not return anything useful + wantErr: assert.Error, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotBi, err := getBuildInfo(tt.args.r) + if !tt.wantErr(t, err, fmt.Sprintf("getBuildInfo(%v)", tt.args.r)) { + return + } + assert.Equalf(t, tt.wantBi, gotBi, "getBuildInfo(%v)", tt.args.r) + }) + } +}