Mark disputed CVEs as withdrawn (#1531)

This consults the CVE 5.0 record in the CVE List, because the 4.0 schema in the NVD doesn't have a machine-readable way to determine dispute status.
google · Aug 7, 2023 · ac220d0 · ac220d0
1 parent b18a6d8
commit ac220d0
Show file tree

Hide file tree

Showing 4 changed files with 223 additions and 1 deletion.
diff --git a/vulnfeeds/cmd/combine-to-osv/main.go b/vulnfeeds/cmd/combine-to-osv/main.go
@@ -111,6 +111,14 @@ func combineIntoOSV(loadedCves map[string]cves.CVEItem, allParts map[string][]vu
 			continue
 		}
 		convertedCve, _ := vulns.FromCVE(cveId, cve)
+		// Best-effort attempt to mark a disputed CVE as withdrawn.
+		modified, err := vulns.CVEIsDisputed(convertedCve)
+		if err != nil {
+			Logger.Warnf("Unable to determine CVE dispute status of %s: %v", convertedCve.ID, err)
+		}
+		if err == nil && modified != "" {
+			convertedCve.Withdrawn = modified
+		}
 		for _, pkgInfo := range allParts[cveId] {
 			convertedCve.AddPkgInfo(pkgInfo)
 		}

diff --git a/vulnfeeds/cves/cve.go b/vulnfeeds/cves/cve.go
@@ -21,7 +21,8 @@ import (
 )
 
 const (
-	CVETimeFormat = "2006-01-02T15:04Z07:00"
+	CVETimeFormat  = "2006-01-02T15:04Z07:00"
+	CVE5TimeFormat = "2006-01-02T00:00:00"
 )
 
 type CVE struct {
@@ -36,6 +37,7 @@ type CVE struct {
 		} `json:"description_data"`
 	} `json:"description"`
 }
+
 type CVEReferenceData struct {
 	URL       string   `json:"url"`
 	Name      string   `json:"name"`
@@ -96,6 +98,52 @@ func (n *NVDCVE2) ToJSON(w io.Writer) error {
 	return encoder.Encode(n)
 }
 
+type CVE5 struct {
+	DataType    string `json:"dataType"`
+	DataVersion string `json:"dataVersion"`
+	Metadata    struct {
+		State             string `json:"state"`
+		ID                string `json:"cveId"`
+		AssignerOrgId     string `json:"assignerOrgId"`
+		AssignerShortName string `json:"assignerShortName"`
+		DateUpdated       string `json:"dateUpdated"`
+		DateReserved      string `json:"dateReserved"`
+		DatePublished     string `json:"datePublished"`
+	}
+	Containers struct {
+		CNA struct {
+			ProviderMetadata struct {
+				OrgID       string `json:"orgId"`
+				ShortName   string `json:"shortName"`
+				DateUpdated string `json:"dateUpdated"`
+			}
+			Descriptions []struct {
+				Lang  string `json:"lang"`
+				Value string `json:"value"`
+			}
+			Tags     []string `json:"tags"`
+			Affected []struct {
+				Vendor   string `json:"vendor"`
+				Product  string `json:"product"`
+				Versions []struct {
+					Version string `json:"version"`
+					Status  string `json:"status"`
+				}
+			}
+			References []struct {
+				URL string `json:"url"`
+			}
+			ProblemTypes []struct {
+				Descriptions []struct {
+					Type        string `json:"type"`
+					Lang        string `json:"lang"`
+					Description string `json:"description"`
+				}
+			}
+		}
+	}
+}
+
 func EnglishDescription(cve CVE) string {
 	for _, desc := range cve.Description.DescriptionData {
 		if desc.Lang == "en" {
@@ -108,3 +156,7 @@ func EnglishDescription(cve CVE) string {
 func ParseTimestamp(timestamp string) (time.Time, error) {
 	return time.Parse(CVETimeFormat, timestamp)
 }
+
+func ParseCVE5Timestamp(timestamp string) (time.Time, error) {
+	return time.Parse(CVE5TimeFormat, timestamp)
+}
diff --git a/vulnfeeds/vulns/vulns.go b/vulnfeeds/vulns/vulns.go
@@ -15,18 +15,36 @@
 package vulns
 
 import (
+	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
+	"net/http"
 	"net/url"
 	"strings"
 	"time"
 
+	"golang.org/x/exp/slices"
 	"gopkg.in/yaml.v2"
 
 	"github.com/google/osv/vulnfeeds/cves"
+	"github.com/sethvargo/go-retry"
 )
 
+const CVEListBaseURL = "https://raw.githubusercontent.com/CVEProject/cvelistV5/main/cves"
+
+var ErrVulnNotACVE = errors.New("not a CVE")
+
+type VulnsCVEListError struct {
+	URL string
+	Err error
+}
+
+func (e *VulnsCVEListError) Error() string {
+	return e.URL + ": " + e.Err.Error()
+}
+
 type Event struct {
 	Introduced   string `json:"introduced,omitempty" yaml:"introduced,omitempty"`
 	Fixed        string `json:"fixed,omitempty" yaml:"fixed,omitempty"`
@@ -173,6 +191,7 @@ type Reference struct {
 
 type Vulnerability struct {
 	ID         string      `json:"id" yaml:"id"`
+	Withdrawn  string      `json:"withdrawn,omitempty" yaml:"modified,omitempty"`
 	Summary    string      `json:"summary,omitempty" yaml:"summary,omitempty"`
 	Severity   []Severity  `json:"severity,omitempty" yaml:"severity,omitempty"`
 	Details    string      `json:"details" yaml:"details"`
@@ -294,6 +313,15 @@ func timestampToRFC3339(timestamp string) (string, error) {
 	return t.Format(time.RFC3339), nil
 }
 
+func CVE5timestampToRFC3339(timestamp string) (string, error) {
+	t, err := cves.ParseCVE5Timestamp(timestamp)
+	if err != nil {
+		return "", err
+	}
+
+	return t.Format(time.RFC3339), nil
+}
+
 // For a given URL, infer the OSV schema's reference type of it.
 // See https://ossf.github.io/osv-schema/#references-field
 // Uses the tags first before resorting to inference by shape.
@@ -546,3 +574,83 @@ func FromJSON(r io.Reader) (*Vulnerability, error) {
 
 	return &vuln, nil
 }
+
+// CVEIsDisputed will return if the underlying CVE is disputed.
+// It returns the CVE's CNA container's dateUpdated value if it is disputed.
+// This can be used to set the Withdrawn field.
+func CVEIsDisputed(v *Vulnerability) (modified string, e error) {
+	// iff the v.ID starts with a CVE...
+	// 	Try to make an HTTP request for the CVE record in the CVE List
+	// 	iff .containers.cna.tags contains "disputed"
+	//		return .containers.cna.providerMetadata.dateUpdated, formatted for use in the Withdrawn field.
+	if !strings.HasPrefix(v.ID, "CVE-") {
+		return "", ErrVulnNotACVE
+	}
+
+	CVEParts := strings.Split(v.ID, "-")[1:3]
+	// Replace the last three digits of the CVE ID with "xxx".
+	CVEYear, CVEIndexShard := CVEParts[0], CVEParts[1][:len(CVEParts[1])-3]+"xxx"
+
+	// https://raw.githubusercontent.com/CVEProject/cvelistV5/main/cves/2023/23xxx/CVE-2023-23127.json
+	CVEListURL, err := url.JoinPath(CVEListBaseURL, CVEYear, CVEIndexShard, v.ID+".json")
+
+	if err != nil {
+		return "", &VulnsCVEListError{"", err}
+	}
+
+	gitHubClient := http.Client{
+		Timeout: 5 * time.Second,
+	}
+
+	req, err := http.NewRequest(http.MethodGet, CVEListURL, nil)
+
+	if err != nil {
+		return "", &VulnsCVEListError{CVEListURL, err}
+	}
+
+	req.Header.Set("User-Agent", "osv.dev")
+
+	// Retry on timeout or 5xx
+	ctx := context.Background()
+	backoff := retry.NewFibonacci(1 * time.Second)
+	CVE := &cves.CVE5{}
+	if err := retry.Do(ctx, retry.WithMaxRetries(3, backoff), func(ctx context.Context) error {
+		res, err := gitHubClient.Do(req)
+		if err != nil {
+			if err == http.ErrHandlerTimeout {
+				return retry.RetryableError(fmt.Errorf("timeout: %#v", err))
+			}
+			return err
+		}
+		if res.StatusCode/100 == 5 {
+			return retry.RetryableError(fmt.Errorf("bad response: %v", res.StatusCode))
+		}
+		if res.Body != nil {
+			defer res.Body.Close()
+		}
+
+		decoder := json.NewDecoder(res.Body)
+
+		for {
+			if err := decoder.Decode(&CVE); err == io.EOF {
+				break
+			} else if err != nil {
+				return err
+			}
+		}
+		return nil
+	}); err != nil {
+		return "", &VulnsCVEListError{CVEListURL, err}
+	}
+
+	if err != nil {
+		return "", &VulnsCVEListError{CVEListURL, err}
+	}
+
+	if slices.Contains(CVE.Containers.CNA.Tags, "disputed") {
+		modified, err = CVE5timestampToRFC3339(CVE.Containers.CNA.ProviderMetadata.DateUpdated)
+		return modified, err
+	}
+
+	return "", nil
+}
diff --git a/vulnfeeds/vulns/vulns_test.go b/vulnfeeds/vulns/vulns_test.go
@@ -2,6 +2,7 @@ package vulns
 
 import (
 	"encoding/json"
+	"errors"
 	"log"
 	"os"
 	"reflect"
@@ -230,3 +231,56 @@ func TestAddSeverity(t *testing.T) {
 		}
 	}
 }
+
+func TestCVEIsDisputed(t *testing.T) {
+	tests := []struct {
+		description       string
+		inputVulnId       string
+		expectedWithdrawn bool
+		expectedError     error
+	}{
+		{
+			description:       "A non-CVE vulnerability",
+			inputVulnId:       "OSV-1234",
+			expectedWithdrawn: false,
+			expectedError:     ErrVulnNotACVE,
+		},
+		{
+			description:       "A disputed CVE vulnerability",
+			inputVulnId:       "CVE-2023-23127",
+			expectedWithdrawn: true,
+			expectedError:     nil,
+		},
+		{
+			description:       "An undisputed CVE vulnerability",
+			inputVulnId:       "CVE-2023-38408",
+			expectedWithdrawn: false,
+			expectedError:     nil,
+		},
+	}
+
+	for _, tc := range tests {
+		inputVuln := &Vulnerability{
+			ID: tc.inputVulnId,
+		}
+
+		modified, err := CVEIsDisputed(inputVuln)
+
+		if err != nil && err != tc.expectedError {
+			var verr *VulnsCVEListError
+			if errors.As(err, &verr) {
+				t.Errorf("test %q: unexpected errored: %#v", tc.description, verr.Err)
+			} else {
+				t.Errorf("test %q: unexpectedly errored: %#v", tc.description, err)
+			}
+		}
+
+		if err == nil && tc.expectedError != nil {
+			t.Errorf("test %q: did not error as expected, wanted: %#v", tc.description, tc.expectedError)
+		}
+
+		if modified == "" && tc.expectedWithdrawn {
+			t.Errorf("test: %q: withdrawn (%s) not set as expected", tc.description, modified)
+		}
+	}
+}