From 770fdc53eafbed30829fb6b5a08f0e46748c47a4 Mon Sep 17 00:00:00 2001 From: William Murphy Date: Thu, 3 Oct 2024 16:42:57 -0400 Subject: [PATCH] Fix: make failed CPE validation correctly return error (#2762) * Test CPE attributes correctly returns error Previously, this method incorrectly return an empty Attributes object and a nil error, leading to callers attempting to use the empty attributes object. Signed-off-by: Will Murphy * chore: merge with main and refactor call that relied on old nil behavior Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com> * test: add test to cover new OSCPE err pattern Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com> --------- Signed-off-by: Will Murphy Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com> Co-authored-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com> --- syft/cpe/cpe.go | 2 +- syft/cpe/cpe_test.go | 12 ++++++++++-- syft/pkg/cataloger/binary/elf_package.go | 15 +++++++++------ syft/pkg/cataloger/binary/elf_package_test.go | 12 ++++++++++++ syft/pkg/collection_test.go | 2 +- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/syft/cpe/cpe.go b/syft/cpe/cpe.go index fe678977d..c996d4ad9 100644 --- a/syft/cpe/cpe.go +++ b/syft/cpe/cpe.go @@ -95,7 +95,7 @@ func NewAttributes(cpeStr string) (Attributes, error) { } // ensure that this Attributes can be validated after being fully sanitized - if ValidateString(c.String()) != nil { + if err = ValidateString(c.String()); err != nil { return Attributes{}, err } diff --git a/syft/cpe/cpe_test.go b/syft/cpe/cpe_test.go index 4b3f60ae0..fd778c9a8 100644 --- a/syft/cpe/cpe_test.go +++ b/syft/cpe/cpe_test.go @@ -17,6 +17,7 @@ func Test_NewAttributes(t *testing.T) { name string input string expected Attributes + wantErr require.ErrorAssertionFunc }{ { name: "gocase", @@ -33,14 +34,21 @@ func Test_NewAttributes(t *testing.T) { input: `cpe:/a:%240.99_kindle_books_project:%240.99_kindle_books:6::~~~android~~`, expected: MustAttributes(`cpe:2.3:a:\$0.99_kindle_books_project:\$0.99_kindle_books:6:*:*:*:*:android:*:*`), }, + { + name: "null byte in version for some reason", + input: "cpe:2.3:a:oracle:openjdk:11.0.22+7\u0000-J-ms8m:*:*:*:*:*:*:*", + wantErr: require.Error, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { actual, err := NewAttributes(test.input) - if err != nil { - t.Fatalf("got an error while creating Attributes: %+v", err) + if test.wantErr != nil { + test.wantErr(t, err) + return } + require.NoError(t, err) if d := cmp.Diff(actual, test.expected); d != "" { t.Errorf("Attributes mismatch (-want +got):\n%s", d) diff --git a/syft/pkg/cataloger/binary/elf_package.go b/syft/pkg/cataloger/binary/elf_package.go index 9b13647a3..99a989752 100644 --- a/syft/pkg/cataloger/binary/elf_package.go +++ b/syft/pkg/cataloger/binary/elf_package.go @@ -2,6 +2,7 @@ package binary import ( "github.com/anchore/packageurl-go" + "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/cpe" "github.com/anchore/syft/syft/file" "github.com/anchore/syft/syft/pkg" @@ -29,13 +30,15 @@ func packageURL(metadata elfBinaryPackageNotes) string { os := metadata.OS osVersion := metadata.OSVersion + var atts cpe.Attributes atts, err := cpe.NewAttributes(metadata.OSCPE) - if err == nil { - // only "upgrade" the OS information if there is something more specific to use in it's place - if os == "" && osVersion == "" || os == "" && atts.Version != "" || atts.Product != "" && osVersion == "" { - os = atts.Product - osVersion = atts.Version - } + if err != nil { + log.WithFields("error", err).Warn("unable to parse cpe attributes for elf binary package") + } + // only "upgrade" the OS information if there is something more specific to use in it's place + if os == "" && osVersion == "" || os == "" && atts.Version != "" || atts.Product != "" && osVersion == "" { + os = atts.Product + osVersion = atts.Version } if os != "" { diff --git a/syft/pkg/cataloger/binary/elf_package_test.go b/syft/pkg/cataloger/binary/elf_package_test.go index 85fa42221..689e259d1 100644 --- a/syft/pkg/cataloger/binary/elf_package_test.go +++ b/syft/pkg/cataloger/binary/elf_package_test.go @@ -113,6 +113,18 @@ func Test_packageURL(t *testing.T) { }, want: "pkg:generic/system/test@1.0", }, + { + name: "bad or missing OSCPE data cannot be parsed allows for correct string", + metadata: elfBinaryPackageNotes{ + Name: "test", + Version: "1.0", + ELFBinaryPackageNoteJSONPayload: pkg.ELFBinaryPackageNoteJSONPayload{ + System: "system", + OSCPE: "%$#*(#*@&$(", + }, + }, + want: "pkg:generic/system/test@1.0", + }, } for _, test := range tests { diff --git a/syft/pkg/collection_test.go b/syft/pkg/collection_test.go index 65d74c33c..f22dc0b48 100644 --- a/syft/pkg/collection_test.go +++ b/syft/pkg/collection_test.go @@ -381,7 +381,7 @@ func TestCatalog_MergeRecords(t *testing.T) { Type: RpmPkg, }, { - CPEs: []cpe.CPE{cpe.Must("cpe:2.3:b:package:1:1:*:*:*:*:*:*:*", cpe.NVDDictionaryLookupSource)}, + CPEs: []cpe.CPE{cpe.Must("cpe:2.3:a:package:2:2:*:*:*:*:*:*:*", cpe.NVDDictionaryLookupSource)}, Locations: file.NewLocationSet( file.NewVirtualLocationFromCoordinates( file.Coordinates{