Skip to content

Commit

Permalink
Add go binary scanning (#1011)
Browse files Browse the repository at this point in the history
Add go binary scanning extractor, and use it in image scanning. 

This shows quite a few false positives that can be resolved with call
analysis, which will be implemented in a followup PR.
  • Loading branch information
another-rex committed Jun 11, 2024
1 parent d857676 commit 0c01488
Show file tree
Hide file tree
Showing 10 changed files with 299 additions and 51 deletions.
70 changes: 48 additions & 22 deletions internal/image/extractor.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package image

import (
"errors"
"fmt"
"io"
"os"
"path"
"sort"

Expand All @@ -15,36 +16,61 @@ var artifactExtractors map[string]lockfile.Extractor = map[string]lockfile.Extra
"node_modules": lockfile.NodeModulesExtractor{},
"apk-installed": lockfile.ApkInstalledExtractor{},
"dpkg": lockfile.DpkgStatusExtractor{},
"go-binary": lockfile.GoBinaryExtractor{},
}

func findArtifactExtractor(path string) (lockfile.Extractor, string) {
type extractorPair struct {
extractor lockfile.Extractor
name string
}

func findArtifactExtractor(path string) []extractorPair {
// Use ShouldExtract to collect and return a slice of artifactExtractors
var extractors []extractorPair
for name, extractor := range artifactExtractors {
if extractor.ShouldExtract(path) {
return extractor, name
extractors = append(extractors, extractorPair{extractor, name})
}
}

return nil, ""
return extractors
}

func extractArtifactDeps(path string, img *Image) (lockfile.Lockfile, error) {
extractor, extractedAs := findArtifactExtractor(path)

if extractor == nil {
foundExtractors := findArtifactExtractor(path)
if len(foundExtractors) == 0 {
return lockfile.Lockfile{}, fmt.Errorf("%w for %s", lockfile.ErrExtractorNotFound, path)
}

f, err := OpenLayerFile(path, img.LastLayer())
if err != nil {
return lockfile.Lockfile{}, fmt.Errorf("attempted to open file but failed: %w", err)
}
packages := []lockfile.PackageDetails{}
var extractedAs string
for _, extPair := range foundExtractors {
// File has to be reopened per extractor as each extractor moves the read cursor
f, err := OpenLayerFile(path, img.LastLayer())
if err != nil {
return lockfile.Lockfile{}, fmt.Errorf("attempted to open file but failed: %w", err)
}

newPackages, err := extPair.extractor.Extract(f)
f.Close()

defer f.Close()
if err != nil {
if errors.Is(lockfile.ErrIncompatibleFileFormat, err) {
continue
}

packages, err := extractor.Extract(f)
if err != nil && extractedAs != "" {
err = fmt.Errorf("(extracting as %s) %w", extractedAs, err)
return lockfile.Lockfile{}, fmt.Errorf("failed to close file: %w", err)
return lockfile.Lockfile{}, fmt.Errorf("(extracting as %s) %w", extPair.name, err)
}

extractedAs = extPair.name
packages = newPackages
// TODO(rexpan): Determine if this it's acceptable to have multiple extractors
// extract from the same file successfully
break
}

if extractedAs == "" {
return lockfile.Lockfile{}, fmt.Errorf("%w for %s", lockfile.ErrExtractorNotFound, path)
}

// Sort to have deterministic output, and to match behavior of lockfile.extractDeps
Expand All @@ -57,15 +83,15 @@ func extractArtifactDeps(path string, img *Image) (lockfile.Lockfile, error) {
})

return lockfile.Lockfile{
FilePath: f.Path(),
FilePath: path,
ParsedAs: extractedAs,
Packages: packages,
}, err
}, nil
}

// A ImageFile represents a file that exists in an image
type ImageFile struct {
io.ReadCloser
*os.File

layer fileMap
path string
Expand Down Expand Up @@ -93,9 +119,9 @@ func OpenLayerFile(path string, layer fileMap) (ImageFile, error) {
}

return ImageFile{
ReadCloser: readCloser,
path: path,
layer: layer,
File: readCloser,
path: path,
layer: layer,
}, nil
}

Expand Down
2 changes: 1 addition & 1 deletion internal/image/filemap.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type fileMap struct {
// TODO: Use hashset to speed up path lookups
}

func (filemap fileMap) OpenFile(path string) (fs.File, error) {
func (filemap fileMap) OpenFile(path string) (*os.File, error) {
node, ok := filemap.fileNodeTrie.Get(path).(fileNode)
if !ok {
return nil, fs.ErrNotExist
Expand Down
5 changes: 5 additions & 0 deletions pkg/lockfile/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package lockfile

import "errors"

var ErrIncompatibleFileFormat = errors.New("file format is incompatible, but this is expected")
1 change: 1 addition & 0 deletions pkg/lockfile/extractor.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type Extractor interface {

// A LocalFile represents a file that exists on the local filesystem.
type LocalFile struct {
// TODO(rexpan): This should be *os.File, as that would allow us to access other underlying functions that definitely will exist
io.ReadCloser

path string
Expand Down
Binary file added pkg/lockfile/fixtures/go/binaries/has-one-dep
Binary file not shown.
Binary file added pkg/lockfile/fixtures/go/binaries/just-go
Binary file not shown.
76 changes: 76 additions & 0 deletions pkg/lockfile/go-binary.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package lockfile

import (
"bytes"
"debug/buildinfo"
"io"
"path/filepath"
"strings"
)

type GoBinaryExtractor struct{}

func (e GoBinaryExtractor) ShouldExtract(path string) bool {
if path == "" {
return false
}

if strings.HasSuffix(path, string(filepath.Separator)) { // Don't extract directories
return false
}

if filepath.Ext(path) != ".exe" && filepath.Ext(path) != "" {
// Assume if a file has an extension (that's not exe), it is not a go binary
// This also filters out hidden files on Unix
// This is a heuristic to improve performance and can result in false negatives
// TODO(another-rex): When we have access to the full FS interface, we can open and check
// magic bytes to be more accurate
return false
}

// Any other path can be a go binary
return true
}

func (e GoBinaryExtractor) Extract(f DepFile) ([]PackageDetails, error) {
var readerAt io.ReaderAt
if fileWithReaderAt, ok := f.(io.ReaderAt); ok {
readerAt = fileWithReaderAt
} else {
buf := bytes.NewBuffer([]byte{})
_, err := io.Copy(buf, f)
if err != nil {
return []PackageDetails{}, err
}
readerAt = bytes.NewReader(buf.Bytes())
}

info, err := buildinfo.Read(readerAt)
if err != nil {
return []PackageDetails{}, ErrIncompatibleFileFormat
}

pkgs := make([]PackageDetails, 0, len(info.Deps)+1)
pkgs = append(pkgs, PackageDetails{
Name: "stdlib",
Version: strings.TrimPrefix(info.GoVersion, "go"),
Ecosystem: GoEcosystem,
CompareAs: GoEcosystem,
})

for _, dep := range info.Deps {
if dep.Replace != nil { // Use the replaced dep if it has been replaced
dep = dep.Replace
}
pkgs = append(pkgs, PackageDetails{
Name: dep.Path,
Version: strings.TrimPrefix(dep.Version, "v"),
Ecosystem: GoEcosystem,
CompareAs: GoEcosystem,
})
}

return pkgs, nil
}

var _ Extractor = GoBinaryExtractor{}
139 changes: 139 additions & 0 deletions pkg/lockfile/go-binary_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
package lockfile_test

import (
"testing"

"github.com/google/osv-scanner/pkg/lockfile"
)

func TestGoBinaryExtractor_ShouldExtract(t *testing.T) {
t.Parallel()

tests := []struct {
name string
path string
want bool
}{
{
name: "",
path: "",
want: false,
},
{
name: "",
path: "binary.json",
want: false,
},
{
name: "",
path: "path/to/my/binary.json",
want: false,
},
{
name: "",
path: "path/to/my/binary-lock.json/file",
want: true,
},
{
name: "",
path: "path/to/my/binary",
want: true,
},
{
name: "",
path: "path/to/my/binary.exe",
want: true,
},
{
name: "",
path: "path/to/my/.hidden-binary",
want: false,
},
{
name: "",
path: "path/to/my/binary.exe.1",
want: false,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
e := lockfile.GoBinaryExtractor{}
got := e.ShouldExtract(tt.path)
if got != tt.want {
t.Errorf("Extract(%v) got = %v, want %v", tt.path, got, tt.want)
}
})
}
}

func TestExtractGoBinary_NoPackages(t *testing.T) {
t.Parallel()

file, err := lockfile.OpenLocalDepFile("fixtures/go/binaries/just-go")
if err != nil {
t.Fatalf("could not open file %v", err)
}

packages, err := lockfile.GoBinaryExtractor{}.Extract(file)
if err != nil {
t.Errorf("Got unexpected error: %v", err)
}

expectPackages(t, packages, []lockfile.PackageDetails{
{
Name: "stdlib",
Version: "1.21.10",
Ecosystem: lockfile.GoEcosystem,
CompareAs: lockfile.GoEcosystem,
},
})
}

func TestExtractGoBinary_OnePackage(t *testing.T) {
t.Parallel()

file, err := lockfile.OpenLocalDepFile("fixtures/go/binaries/has-one-dep")
if err != nil {
t.Fatalf("could not open file %v", err)
}

packages, err := lockfile.GoBinaryExtractor{}.Extract(file)
if err != nil {
t.Errorf("Got unexpected error: %v", err)
}

expectPackages(t, packages, []lockfile.PackageDetails{
{
Name: "stdlib",
Version: "1.21.10",
Ecosystem: lockfile.GoEcosystem,
CompareAs: lockfile.GoEcosystem,
},
{
Name: "github.com/BurntSushi/toml",
Version: "1.4.0",
Ecosystem: lockfile.GoEcosystem,
CompareAs: lockfile.GoEcosystem,
},
})
}

func TestExtractGoBinary_NotAGoBinary(t *testing.T) {
t.Parallel()

file, err := lockfile.OpenLocalDepFile("fixtures/go/one-package.mod")
if err != nil {
t.Fatalf("could not open file %v", err)
}

packages, err := lockfile.GoBinaryExtractor{}.Extract(file)
if err == nil {
t.Errorf("did not get expected error when extracting")
}

if len(packages) != 0 {
t.Errorf("packages not empty")
}
}
29 changes: 29 additions & 0 deletions pkg/lockfile/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package lockfile_test
import (
"errors"
"fmt"
"os"
"reflect"
"strings"
"testing"
Expand Down Expand Up @@ -117,3 +118,31 @@ func expectPackages(t *testing.T, actualPackages []lockfile.PackageDetails, expe
}
}
}

func createTestDir(t *testing.T) (string, func()) {
t.Helper()

p, err := os.MkdirTemp("", "osv-scanner-test-*")
if err != nil {
t.Fatalf("could not create test directory: %v", err)
}

return p, func() {
_ = os.RemoveAll(p)
}
}

func copyFile(t *testing.T, from, to string) string {
t.Helper()

b, err := os.ReadFile(from)
if err != nil {
t.Fatalf("could not read test file: %v", err)
}

if err := os.WriteFile(to, b, 0600); err != nil {
t.Fatalf("could not copy test file: %v", err)
}

return to
}
Loading

0 comments on commit 0c01488

Please sign in to comment.