bug(gobinary): incorrect ldflags parsing when `version` part has prefix
Description
Trivy detects different versions of Gobinary modules during multiple scans. This happens when gobinari uses 2 ldflags with the suffix versions. e.g.:
build -ldflags="-X github.com/argoproj/argo-cd/v2/common.version=2.11.0
-X github.com/argoproj/argo-cd/v2/common.kubectlVersion=v0.26.11
-X \"github.com/argoproj/argo-cd/v2/common.extraBuildInfo=\" -extldflags \"-static\""
Discussed in https://github.com/aquasecurity/trivy/discussions/6669
https://github.com/aquasecurity/trivy/pull/6705 addresses the case of ArgoCD, but a bigger problem is the inconsistent result. In the following case, we will face the same problem.
-X github.com/argoproj/argo-cd/v2/common.version=2.11.0
-X github.com/argoproj/argo-cd/v2/kubectl.version=v0.26.11
What if we collect all versions and select one of them deterministically? Or return an error, like several versions found?
Or return an error, like several versions found?
I think this is bad idea. I think it's not their own binaries that users are scanning, so they can't update ldflags.
And then users will not be able to scan binaries with multiple *.version=x.x.x flags.
What if we collect all versions and select one of them deterministically?
I didn't find many binaries using ldflags.
I saw main, version, common.
I didn't notice this, but I guess we can also check module name (something like that -X https://github.com/aquasecurity/trivy/trivy.version=v0.51.0
I suggest the following order <module_name> => common => main =>version.
And then users will not be able to scan binaries with multiple *.version=x.x.x flags.
First of all, the scan does not fail because analyzer errors are always converted into warnings. Next, when I said return error, I meant leave the version empty, rather than select a version that could be wrong on our end.
I didn't find many binaries using ldflags.
How do they embed versions without using ldflags?
How do they embed versions without using ldflags?
Sorry, my brain isn't working well today. I checked installed in my PC binaries which were installed using go install command.
Obviously these binaries do not use ldflags.
First of all, the scan does not fail because analyzer errors are always converted into warnings. Next, when I said return error, I meant leave the version empty, rather than select a version that could be wrong on our end.
Got it. Then this solution looks good.
I tried to discover some logic that we can use to detect core packages, but ldglag is so custom that all users use their own logic...
e.g. go-containerregistry uses github.com/google/go-containerregistry/cmd/crane/cmd.Version + github.com/google/go-containerregistry/pkg/v1/remote/transport.Version (path not included "transport").
Goreleaser has a default value - https://goreleaser.com/cookbooks/using-main.version/, but it is easy to configure.
It also looks like we need to parse URL.
e.g. user might use version.Version=x.x.x for two modules in their repository:
- github.com/owner/cmd/module/version.Version=2.0.0
- github.com/owner/pkg/submodule/submodule/version.Version=1.0.0
It looks like you are right and best solution is to skip version for binaries with more than one version in ldflags.
I didn't notice this, but I guess we can also check module name (something like that -X https://github.com/aquasecurity/trivy/trivy.version=v0.51.0
As you suggested, it may be a good idea to increase the priority if -X starts with the module name and /cmd/, like github.com/google/go-containerregistry/cmd. If we still find more than one applicable version, it should leave it empty.
Just to confirm that I understood you correctly:
-
github.com/<owner>/<repo>/cmd/*/*.version=x.x.xformat- format only:
github.com/google/go-containerregistry/cmd/crane/any.Version=1.0.0=>1.0.0.- (cmd from non-root dir):
github.com/google/go-containerregistry/v1/cmd/crane/any.Version=1.0.0=> step 2
- format + hardcoded prefix:
github.com/google/go-containerregistry/cmd/crane/any.Version=1.0.0+github.com/google/go-containerregistry/pkg/v1/remote/main.2.0.0=>1.0.0or empty?
- format + another prefix:
github.com/google/go-containerregistry/cmd/crane/any.Version=1.0.0+github.com/google/go-containerregistry/pkg/v1/remote/transport.2.0.0=>1.0.0or empty?
- format only:
-
hardcoded prefixes (
main,common,version,cmd):- one version with hardcoded prefix:
common.Version=1.0.0=>1.0.0
- hardcoded + another prefixes:
common.Version=1.0.0+transport.Version=2.0.0=>1.0.0
- two hardcoded prefixes:
common.Version=1.0.0+main.Version=2.0.0=> empty version.
- one version with hardcoded prefix:
-
other prefixes
- one version:
kubectl.Version=1.0.0=>1.0.0
- two version:
terraform.Version=1.0.0+kubectl.Version=2.0.0=> empty version.
- one version:
In any case, it is a heuristic, so it is simply how we define it. If there is more than one matching version in the same category (1, 2, 3 as you described), make the version empty. And in order of priority from the top.
github.com/<owner>/<repo>/cmd/*/*.version=x.x.xformat- hardcoded prefixes (main, common, version, cmd):
- other prefixes
graph TD
A{Number of versions matching 1} -->|0| B{Number of versions matching 2}
A -->|1| C[Adopt that version]
A -->|2+| D[Empty]
B -->|0| E{Number of versions matching 3}
B -->|1| F[Adopt that version]
B -->|2+| G[Empty]
E -->|0| H[Empty]
E -->|1| I[Adopt that version]
E -->|2+| J[Empty]
- format + hardcoded prefix:
- github.com/google/go-containerregistry/cmd/crane/any.Version=1.0.0 + github.com/google/go-containerregistry/pkg/v1/remote/main.2.0.0 => 1.0.0 or empty?
Since there is only one version matching the category 1, we take 1.0.0.
- format + another prefix:
same as above.
Actually, all categories must start with the module name and end with version or ver. So, I think we can simplify them.
- Include
/cmd/path after the module name - hardcoded prefixes (main, common, version, cmd):
- other prefixes