diff --git a/vulnfeeds/cves/versions.go b/vulnfeeds/cves/versions.go index a69733e7547..911bc366308 100644 --- a/vulnfeeds/cves/versions.go +++ b/vulnfeeds/cves/versions.go @@ -515,44 +515,51 @@ func ValidateAndCanonicalizeLink(link string, httpClient *http.Client) (canonica } // For URLs referencing commits in supported Git repository hosts, return a cloneable AffectedCommit. -func ExtractGitCommit(link string, commitType models.CommitType, httpClient *http.Client) (ac models.AffectedCommit, err error) { - r, err := Repo(link) +func extractGitAffectedCommit(link string, commitType models.CommitType, httpClient *http.Client) (models.AffectedCommit, error) { + var ac models.AffectedCommit + c, r, err := ExtractGitCommit(link, httpClient, 0) if err != nil { return ac, err } + ac.SetRepo(r) + + models.SetCommitByType(&ac, commitType, c) + + return ac, nil +} + +func ExtractGitCommit(link string, httpClient *http.Client, depth int) (string, string, error) { + if depth > 10 { + return "", "", fmt.Errorf("max recursion depth exceeded for %s", link) + } + var commit string + r, err := Repo(link) + if err != nil { + return "", "", err + } + c, err := Commit(link) if err != nil { - return ac, err + return "", "", err } + commit = c + // If URL doesn't validate, treat it as linkrot. possiblyDifferentLink, err := ValidateAndCanonicalizeLink(link, httpClient) if err != nil { - return ac, err + return "", "", err } // restart the entire extraction process when the URL changes (i.e. handle a // redirect to a completely different host, instead of a redirect within // GitHub) if possiblyDifferentLink != link { - return ExtractGitCommit(possiblyDifferentLink, commitType, httpClient) + return ExtractGitCommit(possiblyDifferentLink, httpClient, depth+1) } - ac.SetRepo(r) - - switch commitType { - case models.Introduced: - ac.SetIntroduced(c) - case models.LastAffected: - ac.SetLastAffected(c) - case models.Limit: - ac.SetLimit(c) - case models.Fixed: - ac.SetFixed(c) - } - - return ac, nil + return commit, r, nil } func HasVersion(validVersions []string, version string) bool { @@ -676,7 +683,7 @@ func deduplicateAffectedCommits(commits []models.AffectedCommit) []models.Affect func ExtractVersionInfo(cve CVE, validVersions []string, httpClient *http.Client) (v models.VersionInfo, notes []string) { for _, reference := range cve.References { // (Potentially faulty) Assumption: All viable Git commit reference links are fix commits. - if commit, err := ExtractGitCommit(reference.URL, models.Fixed, httpClient); err == nil { + if commit, err := extractGitAffectedCommit(reference.URL, models.Fixed, httpClient); err == nil { v.AffectedCommits = append(v.AffectedCommits, commit) } } diff --git a/vulnfeeds/cves/versions_test.go b/vulnfeeds/cves/versions_test.go index 0890ba81149..ec7d3f87ef4 100644 --- a/vulnfeeds/cves/versions_test.go +++ b/vulnfeeds/cves/versions_test.go @@ -686,20 +686,20 @@ func TestExtractGitCommit(t *testing.T) { t.Skipf("test %q: running on Cloud Build", tc.description) } if time.Now().Before(tc.disableExpiryDate) { - t.Skipf("test %q: extractGitCommit for %q (%q) has been skipped due to known outage and will be reenabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) + t.Skipf("test %q: extractGitAffectedCommit for %q (%q) has been skipped due to known outage and will be reenabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) } if !tc.disableExpiryDate.IsZero() && time.Now().After(tc.disableExpiryDate) { - t.Logf("test %q: extractGitCommit(%q, %q) has been enabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) + t.Logf("test %q: extractGitAffectedCommit(%q, %q) has been enabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) } - got, err := ExtractGitCommit(tc.inputLink, tc.inputCommitType, client) + got, err := extractGitAffectedCommit(tc.inputLink, tc.inputCommitType, client) if err != nil && !tc.expectFailure { - t.Errorf("test %q: extractGitCommit for %q (%q) errored unexpectedly: %#v", tc.description, tc.inputLink, tc.inputCommitType, err) + t.Errorf("test %q: extractGitAffectedCommit for %q (%q) errored unexpectedly: %#v", tc.description, tc.inputLink, tc.inputCommitType, err) } if err == nil && tc.expectFailure { - t.Errorf("test %q: extractGitCommit for %q (%q) did not error as unexpected!", tc.description, tc.inputLink, tc.inputCommitType) + t.Errorf("test %q: extractGitAffectedCommit for %q (%q) did not error as unexpected!", tc.description, tc.inputLink, tc.inputCommitType) } if !reflect.DeepEqual(got, tc.expectedAffectedCommit) { - t.Errorf("test %q: extractGitCommit for %q was incorrect, got: %#v, expected: %#v", tc.description, tc.inputLink, got, tc.expectedAffectedCommit) + t.Errorf("test %q: extractGitAffectedCommit for %q was incorrect, got: %#v, expected: %#v", tc.description, tc.inputLink, got, tc.expectedAffectedCommit) } }) } diff --git a/vulnfeeds/git/versions.go b/vulnfeeds/git/versions.go index ccee3c8e8f3..bcc3404dc0d 100644 --- a/vulnfeeds/git/versions.go +++ b/vulnfeeds/git/versions.go @@ -25,20 +25,6 @@ import ( var versionRangeRegex = regexp.MustCompile(`^(>=|<=|~|\^|>|<|=)\s*([0-9a-zA-Z\.\-]+)(?:,\s*(>=|<=|~|\^|>|<|=)\s*([0-9a-zA-Z\.\-]+))?$`) // Used to parse version strings from the GitHub CNA. -// setCommitByType sets the appropriate commit field on an AffectedCommit based on the CommitType. -func setCommitByType(ac *models.AffectedCommit, commitType models.CommitType, commitHash string) { - switch commitType { - case models.Introduced: - ac.SetIntroduced(commitHash) - case models.LastAffected: - ac.SetLastAffected(commitHash) - case models.Limit: - ac.SetLimit(commitHash) - case models.Fixed: - ac.SetFixed(commitHash) - } -} - // findFuzzyCommit takes an already normalized version and the mapping of repo tags to // normalized tags and commits, and performs fuzzy matching to find a commit hash. func findFuzzyCommit(normalizedVersion string, normalizedTags map[string]NormalizedTag) (string, bool) { @@ -87,7 +73,7 @@ func VersionToAffectedCommit(version string, repo string, commitType models.Comm return ac, err } ac.SetRepo(repo) - setCommitByType(&ac, commitType, commitHash) + models.SetCommitByType(&ac, commitType, commitHash) return ac, nil } diff --git a/vulnfeeds/models/types.go b/vulnfeeds/models/types.go index 26e8880450a..b48c3eefab8 100644 --- a/vulnfeeds/models/types.go +++ b/vulnfeeds/models/types.go @@ -15,6 +15,20 @@ type AffectedCommit struct { LastAffected string `json:"last_affected,omitempty" yaml:"last_affected,omitempty"` } +// SetCommitByType sets the appropriate commit field on an AffectedCommit based on the CommitType. +func SetCommitByType(ac *AffectedCommit, commitType CommitType, commitHash string) { + switch commitType { + case Introduced: + ac.SetIntroduced(commitHash) + case LastAffected: + ac.SetLastAffected(commitHash) + case Limit: + ac.SetLimit(commitHash) + case Fixed: + ac.SetFixed(commitHash) + } +} + func (ac *AffectedCommit) SetRepo(repo string) { // GitHub.com repos are demonstrably case-insensitive, and frequently // expressed in URLs with varying cases, so normalize them to lowercase.