recover from bad parsing of golang binary (#1371)

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

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")

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
+}

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)
+		})
+	}
+}