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 <[email protected]> * chore: merge with main and refactor call that relied on old nil behavior Signed-off-by: Christopher Phillips <[email protected]> * test: add test to cover new OSCPE err pattern Signed-off-by: Christopher Phillips <[email protected]> --------- Signed-off-by: Will Murphy <[email protected]> Signed-off-by: Christopher Phillips <[email protected]> Co-authored-by: Christopher Phillips <[email protected]>
anchore · Oct 3, 2024 · 770fdc5 · 770fdc5
1 parent 32c0d1e
commit 770fdc5
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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
@@ -113,6 +113,18 @@ func Test_packageURL(t *testing.T) {
 			},
 			want: "pkg:generic/system/[email protected]",
 		},
+		{
+			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/[email protected]",
+		},
 	}
 
 	for _, test := range tests {

diff --git 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{