Add additional validation for CVE data and test (go mod tidy with gol…

…ang 1.16)

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

go.mod

@@ -33,6 +33,7 @@ require (
 	github.com/sirupsen/logrus v1.7.0
 	github.com/spf13/cobra v1.1.1
 	github.com/spf13/pflag v1.0.5
+	github.com/spiegel-im-spiegel/go-cvss v0.4.0
 	github.com/stretchr/testify v1.7.0
 	github.com/yuin/goldmark v1.3.1
 	golang.org/x/net v0.0.0-20201224014010-6772e930b67b

go.sum

@@ -676,6 +676,10 @@ github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DM
 github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE=
 github.com/spf13/viper v1.6.1/go.mod h1:t3iDnF5Jlj76alVNuyFBk5oUMCvsrkbvZK0WQdfDi5k=
 github.com/spf13/viper v1.7.0/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5qpdg=
+github.com/spiegel-im-spiegel/errs v1.0.2 h1:v4amEwRDqRWjKHOILQnJSovYhZ4ZttEnBBXNXEzS6Sc=
+github.com/spiegel-im-spiegel/errs v1.0.2/go.mod h1:UoasJYYujMcdkbT9USv8dfZWoMyaY3btqQxoLJImw0A=
+github.com/spiegel-im-spiegel/go-cvss v0.4.0 h1:A+S9I01wkiEo6e66xFbxtz9LfXdsGUr5FppYozUAmmI=
+github.com/spiegel-im-spiegel/go-cvss v0.4.0/go.mod h1:VrcmtyfJ7OJF3/O08MQnxq+kqyPMjstExKdv02TPx+M=
 github.com/src-d/gcfg v1.4.0 h1:xXbNR5AlLSA315x2UO+fTSSAXCDf+Ar38/6oyGbDKQ4=
 github.com/src-d/gcfg v1.4.0/go.mod h1:p/UMsR43ujA89BJY9duynAwIpvqEujIH/jFlfL7jWoI=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=

pkg/notes/document/template.go

@@ -72,8 +72,11 @@ This release contains changes that address the following vulnerabilities:
 
 {{.Description}}
 
-**CVSS Rating:** {{.CVSSRating}} ({{.CVSSScore}}) [{{.CVSSVector}}]({{.CalcLink}})<br>
+**CVSS Rating:** {{.CVSSRating}} ({{.CVSSScore}}) [{{.CVSSVector}}]({{.CalcLink}})
+{{- if .TrackingIssue -}}
+<br>
 **Tracking Issue:** {{.TrackingIssue}}
+{{- end }}
 
 {{ end }}
 {{- end -}}

pkg/notes/notes.go

@@ -33,6 +33,7 @@ import (
 	"github.com/nozzle/throttler"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	cvss "github.com/spiegel-im-spiegel/go-cvss/v3/metric"
 	"gopkg.in/yaml.v2"
 
 	"k8s.io/release/pkg/github"
@@ -51,6 +52,9 @@ const (
 	// maxParallelRequests is the maximum parallel requests we shall make to the
 	// GitHub API
 	maxParallelRequests = 10
+
+	// Regexp to check CVE IDs
+	cveIDRegExp = `^CVE-\d{4}-\d+$`
 )
 
 type (
@@ -63,12 +67,12 @@ type CVEData struct {
 	ID            string  `json:"id"`          // CVE ID, eg CVE-2019-1010260
 	Title         string  `json:"title"`       // Title of the vulnerability
 	Description   string  `json:"description"` // Description text of the vulnerability
-	TrackingIssue string  `json:"issue"`       // Link to the vulnerability tracking issue
+	TrackingIssue string  `json:"issue"`       // Link to the vulnerability tracking issue (url, optional)
 	CVSSVector    string  `json:"vector"`      // Full CVSS vector string, CVSS:3.1/AV:N/AC:H/PR:H/UI:R/S:U/C:H/I:H/A:H
 	CVSSScore     float32 `json:"score"`       // Numeric CVSS score (eg 6.2)
 	CVSSRating    string  `json:"rating"`      // Severity bucket (eg Medium)
 	CalcLink      string  // Link to the CVE calculator (automatic)
-	LinkedPRs     []int   `json:"linkedPRs"` // List of linked PRs (to remove them from the release notes doc)
+	LinkedPRs     []int   `json:"pullrequests"` // List of linked PRs (to remove them from the release notes doc)
 }
 
 const (
@@ -1090,31 +1094,51 @@ func (rn *ReleaseNote) ContentHash() (string, error) {
 
 // Validate checks the data defined in a CVE map is complete and valid
 func (cve *CVEData) Validate() error {
+	// Verify that rating is defined and a known string
 	if cve.CVSSRating == "" {
 		return errors.New("CVSS rating missing from CVE data")
 	}
 
 	// Check rating is a valid string
-	if cve.CVSSRating != "None" &&
-		cve.CVSSRating != "Low" &&
-		cve.CVSSRating != "Medium" &&
-		cve.CVSSRating != "High" &&
-		cve.CVSSRating != "Critical" {
+	if _, ok := map[string]bool{
+		"None": true, "Low": true, "Medium": true, "High": true, "Critical": true,
+	}[cve.CVSSRating]; !ok {
 		return errors.New("Invalid CVSS rating")
 	}
 
+	// Check vector string is not empty
 	if cve.CVSSVector == "" {
 		return errors.New("CVSS vector string missing from CVE data")
 	}
 
+	// Parse the vector string to make sure it is well formed
+	bm, err := cvss.NewBase().Decode(cve.CVSSVector)
+	if err != nil {
+		return errors.Wrap(err, "parsing CVSS vector string")
+	}
+	cve.CalcLink = fmt.Sprintf(
+		"https://www.first.org/cvss/calculator/%s#%s", bm.Ver.String(), cve.CVSSVector,
+	)
+
 	if cve.CVSSScore == 0 {
 		return errors.New("CVSS score missing from CVE data")
 	}
+	if cve.CVSSScore < 0 || cve.CVSSScore > 10 {
+		return errors.New("CVSS score pit of range, should be 0-10")
+	}
 
+	// Check that the CVE ID is not empty
 	if cve.ID == "" {
 		return errors.New("ID missing from CVE data")
 	}
 
+	// Verify that the CVE ID is well formed
+	cvsre := regexp.MustCompile(cveIDRegExp)
+	if !cvsre.MatchString(cve.ID) {
+		return errors.New("CVS ID is not well formed")
+	}
+
+	// Title and description must not be empty
 	if cve.Title == "" {
 		return errors.New("Title missing from CVE data")
 	}
@@ -1123,14 +1147,5 @@ func (cve *CVEData) Validate() error {
 		return errors.New("CVE description missing from CVE data")
 	}
 
-	// Since we're checking the vector string with a regex, use the effort to
-	// add a link to the CVE calculator
-	re := regexp.MustCompile(`^CVSS:(\d+\.\d+)/`)
-	cvssVer := re.FindStringSubmatch(cve.CVSSVector)
-	if len(cvssVer) == 0 {
-		return errors.New("CVSS vector in not properly formed: version missing")
-	}
-	cve.CalcLink = fmt.Sprintf("https://www.first.org/cvss/calculator/%s#%s", cvssVer[1], cve.CVSSVector)
-
 	return nil
 }

pkg/notes/notes_test.go

@@ -430,3 +430,74 @@ func TestApplyMap(t *testing.T) {
 		}
 	}
 }
+
+func TestCVEDataValidation(t *testing.T) {
+	var sut CVEData
+	cve := CVEData{
+		ID:            "CVE-2020-8559",
+		Title:         "Privilege escalation from compromised node to cluster",
+		Description:   "If an attacker is able to intercept certain requests to the Kubelet, they",
+		TrackingIssue: "https://github.com/kubernetes/kubernetes/issues/92914",
+		CVSSVector:    "CVSS:3.1/AV:N/AC:H/PR:H/UI:R/S:U/C:H/I:H/A:H",
+		CVSSScore:     6.4,
+		CVSSRating:    "Medium",
+		LinkedPRs:     []int{92941, 92969, 92970, 92971},
+	}
+
+	// As is, the CVE should validate
+	require.Nil(t, cve.Validate())
+
+	// Check each value
+	sut = cve
+	sut.ID = "CVE-123"
+	require.NotNil(t, sut.Validate(), "checking cve id")
+
+	sut = cve
+	sut.Title = ""
+	require.NotNil(t, sut.Validate(), "checking title")
+
+	sut = cve
+	sut.Description = ""
+	require.NotNil(t, sut.Validate(), "checking description")
+
+	sut = cve
+	for _, testVector := range []string{
+		"CVSS:3.1/AV:N/AC:H/P", //  too short
+		"",                     // empty
+		"CVSS:3.1/AV:N/AC:Á/PR:H/UI:R/S:U/C:H/I:H/A:H", //  invalid value
+	} {
+		sut.CVSSVector = testVector // too short
+		require.NotNil(t, sut.Validate(), "checking vector string")
+	}
+
+	sut = cve
+	for _, testScore := range []float32{
+		0,    // cannot be 0 (nil value)
+		-1,   // under
+		10.1, // over
+	} {
+		sut.CVSSScore = testScore
+		require.NotNil(t, sut.Validate(), "checking vector string")
+	}
+
+	sut = cve
+	for _, tc := range []struct {
+		Valid bool
+		Value string
+	}{
+		{true, "None"},
+		{true, "Low"},
+		{true, "Medium"},
+		{true, "High"},
+		{true, "Critical"},
+		{false, ""},
+		{false, "Superbad"},
+	} {
+		sut.CVSSRating = tc.Value // too short
+		if tc.Valid {
+			require.Nil(t, sut.Validate(), "checking valid rating string")
+		} else {
+			require.NotNil(t, sut.Validate(), "checking invalid rating string")
+		}
+	}
+}