Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(go): parse main mod version from build info settings #6564

Merged
8 changes: 5 additions & 3 deletions docs/docs/coverage/language/golang.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,18 @@ $ trivy rootfs ./your_binary
It doesn't work with UPX-compressed binaries.

#### Empty versions
There are times when Go uses the `(devel)` version for modules/dependencies and Trivy can't resolve them:
There are times when Go uses the `(devel)` version for modules/dependencies.

- Only Go binaries installed using the `go install` command contain correct (semver) version for the main module.
In other cases, Go uses the `(devel)` version[^3].
- Dependencies replaced with local ones use the `(devel)` versions.

In these cases, the version of such packages is empty.
In the first case, Trivy will attempt to parse any `-ldflags` as a secondary source, and will leave the version
empty if it cannot do so[^4]. For the second case, the version of such packages is empty.

[^1]: It doesn't require the Internet access.
[^2]: Need to download modules to local cache beforehand
[^3]: See https://github.com/aquasecurity/trivy/issues/1837#issuecomment-1832523477
[^4]: See https://github.com/golang/go/issues/63432#issuecomment-1751610604

[dependency-graph]: ../../configuration/reporting.md#show-origins-of-vulnerable-dependencies
[dependency-graph]: ../../configuration/reporting.md#show-origins-of-vulnerable-dependencies
73 changes: 70 additions & 3 deletions pkg/dependency/parser/golang/binary/parse.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package binary

import (
"cmp"
"debug/buildinfo"
"runtime/debug"
"sort"
"strings"

"github.com/spf13/pflag"
"golang.org/x/xerrors"

"github.com/aquasecurity/trivy/pkg/dependency/types"
Expand Down Expand Up @@ -48,15 +51,18 @@ func (p *Parser) Parse(r xio.ReadSeekerAt) ([]types.Library, []types.Dependency,
return nil, nil, convertError(err)
}

ldflags := p.ldFlags(info.Settings)
libs := make([]types.Library, 0, len(info.Deps)+2)
libs = append(libs, []types.Library{
{
// Add main module
Name: info.Main.Path,
// Only binaries installed with `go install` contain semver version of the main module.
// Other binaries use the `(devel)` version.
// Other binaries use the `(devel)` version, but still may contain a stamped version
// set via `go build -ldflags='-X main.version=<semver>'`, so we fallback to this as.
// as a secondary source.
// See https://github.com/aquasecurity/trivy/issues/1837#issuecomment-1832523477.
Version: p.checkVersion(info.Main.Path, info.Main.Version),
Version: cmp.Or(p.checkVersion(info.Main.Path, info.Main.Version), p.parseLDFlags(info.Main.Path, ldflags)),
Relationship: types.RelationshipRoot,
},
{
Expand Down Expand Up @@ -93,8 +99,69 @@ func (p *Parser) Parse(r xio.ReadSeekerAt) ([]types.Library, []types.Dependency,
// checkVersion detects `(devel)` versions, removes them and adds a debug message about it.
func (p *Parser) checkVersion(name, version string) string {
if version == "(devel)" {
p.logger.Debug("Unable to detect dependency version (`(devel)` is used). Version will be empty.", log.String("dependency", name))
p.logger.Debug("Unable to detect main module's dependency version - `(devel)` is used", log.String("dependency", name))
return ""
}
return version
}

func (p *Parser) ldFlags(settings []debug.BuildSetting) []string {
for _, setting := range settings {
if setting.Key != "-ldflags" {
continue
}

return strings.Fields(setting.Value)
}
return nil
}

// parseLDFlags attempts to parse the binary's version from any `-ldflags` passed to `go build` at build time.
func (p *Parser) parseLDFlags(name string, flags []string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The input is so varied that it is worth exporting this method and writing tests for it.

Suggested change
func (p *Parser) parseLDFlags(name string, flags []string) string {
func (p *Parser) ParseLDFlags(name string, flags []string) string {
func TestParser_ParseLDFlags(t *testing.T) {
	type args struct {
		name  string
		flags []string
	}
	tests := []struct {
		name string
		args args
		want string
	}{
		{
			name: "normal",
			args: args{
				name: "github.com/aquasecurity/trivy",
				flags: []string{
					"-s",
					"-w",
					"-X=foo=bar",
					"-X='github.com/aquasecurity/trivy/pkg/version.ver=0.50.1'",
				},
			},
			want: "0.50.1",
		},
		{
			name: "empty",
			args: args{
				name:  "github.com/aquasecurity/test",
				flags: []string{},
			},
			want: "",
		},
                ... (more tests) ...
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			p := binary.NewParser().(*binary.Parser)
			assert.Equal(t, tt.want, p.ParseLDFlags(tt.args.name, tt.args.flags))
		})
	}
}

log.Debug("Parsing dependency's build info settings", "dependency", name, "-ldflags", flags)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log.Debug("Parsing dependency's build info settings", "dependency", name, "-ldflags", flags)
p.logger.Debug("Parsing dependency's build info settings", "dependency", name, "-ldflags", flags)

fset := pflag.NewFlagSet("ldflags", pflag.ContinueOnError)
// This prevents the flag set from erroring out if other flags were provided.
// This helps keep the implementation small, so that only the -X flag is needed.
fset.ParseErrorsWhitelist.UnknownFlags = true
// The shorthand name is needed here because setting the full name
// to `X` will cause the flag set to look for `--X` instead of `-X`.
// The flag can also be set multiple times, so a string slice is needed
// to handle that edge case.
var x []string
fset.StringSliceVarP(&x, "", "X", nil, `Set the value of the string variable in importpath named name to value.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need the original usage here.

This is only effective if the variable is declared in the source code either uninitialized
or initialized to a constant string expression. -X will not work if the initializer makes
a function call or refers to other variables.
Note that before Go 1.5 this option took two separate arguments.`)
if err := fset.Parse(flags); err != nil {
p.logger.Error("Could not parse -ldflags found in build info", "err", err)
oatovar marked this conversation as resolved.
Show resolved Hide resolved
return ""
}

for _, xx := range x {
// It's valid to set the -X flags with quotes so we trim any that might
// have been provided: Ex:
//
// -X main.version=1.0.0
// -X=main.version=1.0.0
// -X 'main.version=1.0.0'
// -X='main.version=1.0.0'
// -X="main.version=1.0.0"
// -X "main.version=1.0.0"
xx = strings.Trim(xx, `'"`)
key, val, found := strings.Cut(xx, "=")
if !found {
p.logger.Debug("Unable to parse an -ldflags -X flag value", "got", xx)
continue
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StringToStringVarP possibly simplifies parsing.

Suggested change
var x []string
fset.StringSliceVarP(&x, "", "X", nil, `Set the value of the string variable in importpath named name to value.
This is only effective if the variable is declared in the source code either uninitialized
or initialized to a constant string expression. -X will not work if the initializer makes
a function call or refers to other variables.
Note that before Go 1.5 this option took two separate arguments.`)
if err := fset.Parse(flags); err != nil {
p.logger.Error("Could not parse -ldflags found in build info", "err", err)
return ""
}
for _, xx := range x {
// It's valid to set the -X flags with quotes so we trim any that might
// have been provided: Ex:
//
// -X main.version=1.0.0
// -X=main.version=1.0.0
// -X 'main.version=1.0.0'
// -X='main.version=1.0.0'
// -X="main.version=1.0.0"
// -X "main.version=1.0.0"
xx = strings.Trim(xx, `'"`)
key, val, found := strings.Cut(xx, "=")
if !found {
p.logger.Debug("Unable to parse an -ldflags -X flag value", "got", xx)
continue
}
var x map[string]string
fset.StringToStringVarP(&x, "", "X", nil, "")
if err := fset.Parse(flags); err != nil {
p.logger.Error("Could not parse -ldflags found in build info", "err", err)
return ""
}
for key, value := range x {
key = strings.ToLower(key)
// The check for a 'ver' prefix enables the parser to pick up Trivy's own version value that's set.
if (strings.HasSuffix(key, "version") || strings.HasSuffix(key, "ver")) && semver.IsValid(value) {
return value
}
}```


key = strings.ToLower(key)
// The check for a 'ver' prefix enables the parser to pick up Trivy's own version value that's set.
if strings.HasSuffix(key, "version") || strings.HasSuffix(key, "ver") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if validating semver?

Suggested change
if strings.HasSuffix(key, "version") || strings.HasSuffix(key, "ver") {
if (strings.HasSuffix(key, "version") || strings.HasSuffix(key, "ver")) && semver.IsValid(value) {

return val
}
}

p.logger.Debug("Unable to detect dependency version used in `-ldflags` build info settings. Empty version used.", log.String("dependency", name))
return ""
}
16 changes: 16 additions & 0 deletions pkg/dependency/parser/golang/binary/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,22 @@ func TestParse(t *testing.T) {
},
},
},
{
name: "with -ldflags=\"-X main.version=v1.0.0\"",
inputFile: "testdata/main-version-via-ldflags.elf",
want: []types.Library{
{
Name: "github.com/aquasecurity/test",
Version: "v1.0.0",
Relationship: types.RelationshipRoot,
},
{
Name: "stdlib",
Version: "1.22.1",
Relationship: types.RelationshipDirect,
},
},
},
{
name: "sad path",
inputFile: "testdata/dummy",
Expand Down
Binary file not shown.