Skip to content

Commit

Permalink
recover from bad parsing of golang binary (#1371)
Browse files Browse the repository at this point in the history
Signed-off-by: Alex Goodman <[email protected]>

Signed-off-by: Alex Goodman <[email protected]>
  • Loading branch information
wagoodman authored Nov 29, 2022
1 parent f6996f7 commit 7a69e21
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 18 deletions.
2 changes: 2 additions & 0 deletions syft/pkg/cataloger/generic/cataloger.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
52 changes: 34 additions & 18 deletions syft/pkg/cataloger/golang/scan_binary.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package golang

import (
"debug/buildinfo"
"fmt"
"io"
"runtime/debug"

"github.com/anchore/syft/internal/log"
Expand All @@ -14,35 +16,49 @@ 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)
}

archs := getArchs(readers, builds)

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
}
40 changes: 40 additions & 0 deletions syft/pkg/cataloger/golang/scan_binary_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}

0 comments on commit 7a69e21

Please sign in to comment.