From 7cfdffab5f0152301e15b59853811eb98fa7f576 Mon Sep 17 00:00:00 2001
From: Keith Zantow <kzantow@gmail.com>
Date: Mon, 6 Mar 2023 10:55:23 -0500
Subject: [PATCH] fix: sanitize SPDX LicenseRefs (#1657)

Signed-off-by: Keith Zantow <kzantow@gmail.com>
---
 syft/formats/common/spdxhelpers/license.go    |  6 ++
 .../common/spdxhelpers/license_test.go        | 11 +++
 .../common/spdxhelpers/to_format_model.go     |  6 +-
 .../spdxhelpers/to_format_model_test.go       | 68 ++++++++++++++++++-
 4 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/syft/formats/common/spdxhelpers/license.go b/syft/formats/common/spdxhelpers/license.go
index 7eb271ecbed..68220183831 100644
--- a/syft/formats/common/spdxhelpers/license.go
+++ b/syft/formats/common/spdxhelpers/license.go
@@ -24,6 +24,12 @@ func License(p pkg.Package) string {
 	// take all licenses and assume an AND expression; for information about license expressions see https://spdx.github.io/spdx-spec/appendix-IV-SPDX-license-expressions/
 	parsedLicenses := parseLicenses(p.Licenses)
 
+	for i, v := range parsedLicenses {
+		if strings.HasPrefix(v, spdxlicense.LicenseRefPrefix) {
+			parsedLicenses[i] = SanitizeElementID(v)
+		}
+	}
+
 	if len(parsedLicenses) == 0 {
 		return NOASSERTION
 	}
diff --git a/syft/formats/common/spdxhelpers/license_test.go b/syft/formats/common/spdxhelpers/license_test.go
index 747b330daa2..980c0f0dd23 100644
--- a/syft/formats/common/spdxhelpers/license_test.go
+++ b/syft/formats/common/spdxhelpers/license_test.go
@@ -65,6 +65,17 @@ func Test_License(t *testing.T) {
 			},
 			expected: "GPL-2.0-only",
 		},
+		{
+			name: "includes valid LicenseRef-",
+			input: pkg.Package{
+				Licenses: []string{
+					"one thing first",
+					"two things/#$^second",
+					"MIT",
+				},
+			},
+			expected: "LicenseRef-one-thing-first AND LicenseRef-two-things----second AND MIT",
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
diff --git a/syft/formats/common/spdxhelpers/to_format_model.go b/syft/formats/common/spdxhelpers/to_format_model.go
index 6160c226e30..a6f3f8d4379 100644
--- a/syft/formats/common/spdxhelpers/to_format_model.go
+++ b/syft/formats/common/spdxhelpers/to_format_model.go
@@ -514,8 +514,8 @@ func toFileTypes(metadata *source.FileMetadata) (ty []string) {
 
 func toOtherLicenses(catalog *pkg.Catalog) []*spdx.OtherLicense {
 	licenses := map[string]bool{}
-	for _, pkg := range catalog.Sorted() {
-		for _, license := range parseLicenses(pkg.Licenses) {
+	for _, p := range catalog.Sorted() {
+		for _, license := range parseLicenses(p.Licenses) {
 			if strings.HasPrefix(license, spdxlicense.LicenseRefPrefix) {
 				licenses[license] = true
 			}
@@ -526,7 +526,7 @@ func toOtherLicenses(catalog *pkg.Catalog) []*spdx.OtherLicense {
 		// separate the actual ID from the prefix
 		name := strings.TrimPrefix(license, spdxlicense.LicenseRefPrefix)
 		result = append(result, &spdx.OtherLicense{
-			LicenseIdentifier: license,
+			LicenseIdentifier: SanitizeElementID(license),
 			LicenseName:       name,
 			ExtractedText:     NONE, // we probably should have some extracted text here, but this is good enough for now
 		})
diff --git a/syft/formats/common/spdxhelpers/to_format_model_test.go b/syft/formats/common/spdxhelpers/to_format_model_test.go
index 54c792ee40e..4e3032202c3 100644
--- a/syft/formats/common/spdxhelpers/to_format_model_test.go
+++ b/syft/formats/common/spdxhelpers/to_format_model_test.go
@@ -369,7 +369,7 @@ func Test_fileIDsForPackage(t *testing.T) {
 }
 
 func Test_H1Digest(t *testing.T) {
-	sbom := sbom.SBOM{}
+	s := sbom.SBOM{}
 	tests := []struct {
 		name           string
 		pkg            pkg.Package
@@ -416,7 +416,7 @@ func Test_H1Digest(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			catalog := pkg.NewCatalog(test.pkg)
-			pkgs := toPackages(catalog, sbom)
+			pkgs := toPackages(catalog, s)
 			require.Len(t, pkgs, 1)
 			for _, p := range pkgs {
 				if test.expectedDigest == "" {
@@ -431,3 +431,67 @@ func Test_H1Digest(t *testing.T) {
 		})
 	}
 }
+
+func Test_OtherLicenses(t *testing.T) {
+	tests := []struct {
+		name     string
+		pkg      pkg.Package
+		expected []*spdx.OtherLicense
+	}{
+		{
+			name: "no licenseRef",
+			pkg: pkg.Package{
+				Licenses: []string{
+					"MIT",
+				},
+			},
+			expected: nil,
+		},
+		{
+			name: "single licenseRef",
+			pkg: pkg.Package{
+				Licenses: []string{
+					"un known",
+				},
+			},
+			expected: []*spdx.OtherLicense{
+				{
+					LicenseIdentifier: "LicenseRef-un-known",
+					LicenseName:       "un known",
+					ExtractedText:     NONE,
+				},
+			},
+		},
+		{
+			name: "multiple licenseRef",
+			pkg: pkg.Package{
+				Licenses: []string{
+					"un known",
+					"not known %s",
+					"MIT",
+				},
+			},
+			expected: []*spdx.OtherLicense{
+				{
+					LicenseIdentifier: "LicenseRef-un-known",
+					LicenseName:       "un known",
+					ExtractedText:     NONE,
+				},
+				{
+					LicenseIdentifier: "LicenseRef-not-known--s",
+					LicenseName:       "not known %s",
+					ExtractedText:     NONE,
+				},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			catalog := pkg.NewCatalog(test.pkg)
+			otherLicenses := toOtherLicenses(catalog)
+			require.Len(t, otherLicenses, len(test.expected))
+			require.Equal(t, test.expected, otherLicenses)
+		})
+	}
+}