fix: UnmarshalText/MarshalText for VendorProduct should handle escaping colons (#4699)

also refactored cpe-repo-gen so that vendor-product info is defined in
one place

Author: jess-lowe

vulnfeeds/cmd/mirrors/cpe-repo-gen/main.go

@@ -77,37 +77,23 @@ type CPEFeed struct {
 	Products []CPEProduct `json:"products"`
 }
 
-// VendorProduct contains a CPE's Vendor and Product strings.
-type VendorProduct struct {
-	Vendor  string
-	Product string
-}
-
 // VendorProducts in this denylist are known non-OSS and/or have generic
 // product names, which cause undesired and incorrect repository attribution
 // when resolved via Debian copyright metadata.
-var DebianCopyrightDenylist = []VendorProduct{
-	{"apple", "pdfkit"},
-	{"f-secure", "safe"},
-	{"ibm", "workflow"},
-	{"inductiveautomation", "ignition"},
-	{"jetbrains", "hub"},
-	{"microsoft", "onedrive"},
-	{"mirametrix", "glance"},
-	{"nintext", "workflow"},
-	{"oracle", "workflow"},
-	{"thrivethemes", "ignition"},
-	{"vmware", "horizon"},
+var DebianCopyrightDenylist = []cves.VendorProduct{
+	{Vendor: "apple", Product: "pdfkit"},
+	{Vendor: "f-secure", Product: "safe"},
+	{Vendor: "ibm", Product: "workflow"},
+	{Vendor: "inductiveautomation", Product: "ignition"},
+	{Vendor: "jetbrains", Product: "hub"},
+	{Vendor: "microsoft", Product: "onedrive"},
+	{Vendor: "mirametrix", Product: "glance"},
+	{Vendor: "nintext", Product: "workflow"},
+	{Vendor: "oracle", Product: "workflow"},
+	{Vendor: "thrivethemes", Product: "ignition"},
+	{Vendor: "vmware", Product: "horizon"},
 }
 
-// MarshalText is a helper for JSON rendering of a map with a struct key.
-func (vp VendorProduct) MarshalText() ([]byte, error) { //nolint:unparam
-	return []byte(vp.Vendor + ":" + vp.Product), nil
-}
-
-// VendorProductToRepoMap maps a VendorProduct to a repo URL.
-type VendorProductToRepoMap map[VendorProduct][]string
-
 const (
 	OutputDirDefault = "."
 	projectID        = "oss-vdb"
@@ -158,7 +144,7 @@ func LoadCPEsFromJSONDir(dir string) ([]CPE, error) {
 }
 
 // Outputs a JSON file of the product-to-repo map.
-func outputProductToRepoMap(prm VendorProductToRepoMap, f io.Writer) error {
+func outputProductToRepoMap(prm cves.VendorProductToRepoMap, f io.Writer) error {
 	productsWithoutRepos := 0
 	for p := range prm {
 		if len(prm[p]) == 0 {
@@ -331,10 +317,10 @@ func MaybeGetSourceRepoFromDebian(mdir string, pkg string) string {
 }
 
 // Analyze CPE Dictionary and return a product-to-repo map and a reference description frequency table.
-func analyzeCPEDictionary(cpes []CPE) (productToRepo VendorProductToRepoMap, descriptionFrequency map[string]int) {
-	productToRepo = make(VendorProductToRepoMap)
+func analyzeCPEDictionary(cpes []CPE) (productToRepo cves.VendorProductToRepoMap, descriptionFrequency map[string]int) {
+	productToRepo = make(cves.VendorProductToRepoMap)
 	descriptionFrequency = make(map[string]int)
-	MaybeTryDebian := make(map[VendorProduct]bool)
+	MaybeTryDebian := make(map[cves.VendorProduct]bool)
 	for _, c := range cpes {
 		if c.Deprecated {
 			logger.Info("Skipping deprecated", slog.String("cpe", c.Name))
@@ -360,26 +346,26 @@ func analyzeCPEDictionary(cpes []CPE) (productToRepo VendorProductToRepoMap, des
 				repo = strings.ToLower(repo)
 			}
 			// If we already have an entry for this repo, don't add it again.
-			if slices.Contains(productToRepo[VendorProduct{parsedCPE.Vendor, parsedCPE.Product}], repo) {
+			if slices.Contains(productToRepo[cves.VendorProduct{Vendor: parsedCPE.Vendor, Product: parsedCPE.Product}], repo) {
 				continue
 			}
 			logger.Info("Liking", slog.String("repo", repo), slog.String("vendor", parsedCPE.Vendor), slog.String("product", parsedCPE.Product), slog.String("type", r.Type))
-			productToRepo[VendorProduct{parsedCPE.Vendor, parsedCPE.Product}] = append(productToRepo[VendorProduct{parsedCPE.Vendor, parsedCPE.Product}], repo)
+			productToRepo[cves.VendorProduct{Vendor: parsedCPE.Vendor, Product: parsedCPE.Product}] = append(productToRepo[cves.VendorProduct{Vendor: parsedCPE.Vendor, Product: parsedCPE.Product}], repo)
 			// If this was queued for trying to find via Debian, and subsequently found, dequeue it.
 			if *DebianMetadataPath != "" {
-				delete(MaybeTryDebian, VendorProduct{parsedCPE.Vendor, parsedCPE.Product})
+				delete(MaybeTryDebian, cves.VendorProduct{Vendor: parsedCPE.Vendor, Product: parsedCPE.Product})
 			}
 		}
 		// If we've arrived to this point, we've exhausted the
 		// references and not calculated any repos for the product,
 		// flag for trying Debian afterwards.
 		// We may encounter another CPE item that *does* have a viable reference in the meantime.
-		if len(productToRepo[VendorProduct{parsedCPE.Vendor, parsedCPE.Product}]) == 0 && *DebianMetadataPath != "" {
+		if len(productToRepo[cves.VendorProduct{Vendor: parsedCPE.Vendor, Product: parsedCPE.Product}]) == 0 && *DebianMetadataPath != "" {
 			// Check the denylist though.
-			if slices.Contains(DebianCopyrightDenylist, VendorProduct{parsedCPE.Vendor, parsedCPE.Product}) {
+			if slices.Contains(DebianCopyrightDenylist, cves.VendorProduct{Vendor: parsedCPE.Vendor, Product: parsedCPE.Product}) {
 				continue
 			}
-			MaybeTryDebian[VendorProduct{parsedCPE.Vendor, parsedCPE.Product}] = true
+			MaybeTryDebian[cves.VendorProduct{Vendor: parsedCPE.Vendor, Product: parsedCPE.Product}] = true
 		}
 	}
 	// Try any Debian possible ones as a last resort.
@@ -403,7 +389,7 @@ func analyzeCPEDictionary(cpes []CPE) (productToRepo VendorProductToRepoMap, des
 					logger.Info("Disregarding derived repo as unusable", slog.String("repo", repo), slog.String("vendor", vp.Vendor), slog.String("product", vp.Product))
 					continue
 				}
-				productToRepo[VendorProduct{vp.Vendor, vp.Product}] = append(productToRepo[VendorProduct{vp.Vendor, vp.Product}], repo)
+				productToRepo[cves.VendorProduct{Vendor: vp.Vendor, Product: vp.Product}] = append(productToRepo[cves.VendorProduct{Vendor: vp.Vendor, Product: vp.Product}], repo)
 			}
 		}
 	}
@@ -412,8 +398,8 @@ func analyzeCPEDictionary(cpes []CPE) (productToRepo VendorProductToRepoMap, des
 }
 
 // validateRepos takes a VendorProductToRepoMap and removes any entries where the repository fails remote validation.
-func validateRepos(prm VendorProductToRepoMap) (validated VendorProductToRepoMap) {
-	validated = make(VendorProductToRepoMap)
+func validateRepos(prm cves.VendorProductToRepoMap) (validated cves.VendorProductToRepoMap) {
+	validated = make(cves.VendorProductToRepoMap)
 	logger.Info("Validating repos", slog.Int("products", len(prm)))
 	// This is likely to be time consuming, so give an impatient log watcher something to gauge progress by.
 	entryCount := 0

vulnfeeds/cves/versions.go

@@ -895,15 +895,27 @@ func ParseCPE(formattedString string) (*models.CPEString, error) {
 
 func (vp *VendorProduct) UnmarshalText(text []byte) error {
 	s := strings.Split(string(text), ":")
-	if len(s) < 2 {
-		return fmt.Errorf("expected at least 2 parts, got %d", len(s))
+	if len(s) != 2 {
+		return fmt.Errorf("expected exactly 2 parts, got %d", len(s))
+	}
+	var err error
+	vp.Vendor, err = url.QueryUnescape(s[0])
+	if err != nil {
+		return err
+	}
+	vp.Product, err = url.QueryUnescape(s[1])
+	if err != nil {
+		return err
 	}
-	vp.Vendor = s[0]
-	vp.Product = s[1]
 
 	return nil
 }
 
+// MarshalText is a helper for JSON rendering of a map with a struct key.
+func (vp VendorProduct) MarshalText() ([]byte, error) {
+	return []byte(url.QueryEscape(vp.Vendor) + ":" + url.QueryEscape(vp.Product)), nil
+}
+
 func RefAcceptable(ref models.Reference, tagDenyList []string) bool {
 	for _, deniedTag := range tagDenyList {
 		if slices.Contains(ref.Tags, deniedTag) {

vulnfeeds/cves/versions_test.go

@@ -1677,3 +1677,125 @@ func TestBuildVersionRange(t *testing.T) {
 		})
 	}
 }
+
+func TestVendorProduct_MarshalText(t *testing.T) {
+	tests := []struct {
+		name    string
+		vp      VendorProduct
+		want    string
+		wantErr bool
+	}{
+		{
+			name: "simple",
+			vp: VendorProduct{
+				Vendor:  "google",
+				Product: "chrome",
+			},
+			want: "google:chrome",
+		},
+		{
+			name: "with spaces",
+			vp: VendorProduct{
+				Vendor:  "adobe",
+				Product: "acrobat reader",
+			},
+			want: "adobe:acrobat+reader",
+		},
+		{
+			name: "with colons",
+			vp: VendorProduct{
+				Vendor:  "foo:bar",
+				Product: "baz",
+			},
+			want: "foo%3Abar:baz",
+		},
+		{
+			name: "empty",
+			vp: VendorProduct{
+				Vendor:  "",
+				Product: "",
+			},
+			want: ":",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := tt.vp.MarshalText()
+			if (err != nil) != tt.wantErr {
+				t.Errorf("VendorProduct.MarshalText() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if string(got) != tt.want {
+				t.Errorf("VendorProduct.MarshalText() = %v, want %v", string(got), tt.want)
+			}
+		})
+	}
+}
+
+func TestVendorProduct_UnmarshalText(t *testing.T) {
+	tests := []struct {
+		name    string
+		text    string
+		want    VendorProduct
+		wantErr bool
+	}{
+		{
+			name: "simple",
+			text: "google:chrome",
+			want: VendorProduct{
+				Vendor:  "google",
+				Product: "chrome",
+			},
+		},
+		{
+			name: "with spaces",
+			text: "adobe:acrobat+reader",
+			want: VendorProduct{
+				Vendor:  "adobe",
+				Product: "acrobat reader",
+			},
+		},
+		{
+			name: "with colons encoded",
+			text: "foo%3Abar:baz",
+			want: VendorProduct{
+				Vendor:  "foo:bar",
+				Product: "baz",
+			},
+		},
+		{
+			name: "empty",
+			text: ":",
+			want: VendorProduct{
+				Vendor:  "",
+				Product: "",
+			},
+		},
+		{
+			name:    "no colon",
+			text:    "invalid",
+			wantErr: true,
+		},
+		{
+			name:    "too many colons",
+			text:    "too:many:colons",
+			wantErr: true,
+		},
+		{
+			name:    "bad encoding",
+			text:    "bad%encoding:foo",
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var vp VendorProduct
+			if err := vp.UnmarshalText([]byte(tt.text)); (err != nil) != tt.wantErr {
+				t.Errorf("VendorProduct.UnmarshalText() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if !tt.wantErr && vp != tt.want {
+				t.Errorf("VendorProduct.UnmarshalText() = %v, want %v", vp, tt.want)
+			}
+		})
+	}
+}