Skip to content

Commit

Permalink
all: move cve/ghsa utils to their own package
Browse files Browse the repository at this point in the history
- Move some functions related to CVE/GHSA regex matching
to a new "idstr" package, as they are not related specifically to the
CVE5 or GHSA GraphQL format.

- Move all logic related to the cve5, cve4 and legacyGHSA formats in
the "internal/report" package to their own files, so it is easier to
(potentially) move these to their own packages in the future.

The goal of this CL is to reduce the risk of import cycles
for some upcoming refactors.

Change-Id: I7e14c31c17882230b783cc62e1ecdf43dcb98995
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/581717
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
  • Loading branch information
tatianab committed May 15, 2024
1 parent 3b6f478 commit 685ac19
Show file tree
Hide file tree
Showing 24 changed files with 460 additions and 422 deletions.
3 changes: 2 additions & 1 deletion cmd/cve/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"golang.org/x/vulndb/internal/cveclient"
"golang.org/x/vulndb/internal/cveschema5"
"golang.org/x/vulndb/internal/database"
"golang.org/x/vulndb/internal/idstr"
"golang.org/x/vulndb/internal/report"
)

Expand Down Expand Up @@ -200,7 +201,7 @@ func validateID(id string) (string, error) {
if id == "" {
return "", errors.New("CVE ID must be provided")
}
if !cveschema5.IsCVE(id) {
if !idstr.IsCVE(id) {
return "", fmt.Errorf("%q is not a valid CVE ID", id)
}
return id, nil
Expand Down
4 changes: 2 additions & 2 deletions cmd/issue/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ import (
"strings"

"golang.org/x/vulndb/internal"
"golang.org/x/vulndb/internal/cveschema5"
"golang.org/x/vulndb/internal/ghsa"
"golang.org/x/vulndb/internal/gitrepo"
"golang.org/x/vulndb/internal/idstr"
"golang.org/x/vulndb/internal/issues"
"golang.org/x/vulndb/internal/proxy"
"golang.org/x/vulndb/internal/report"
Expand Down Expand Up @@ -101,7 +101,7 @@ func createExcluded(ctx context.Context, c *issues.Client, ghsaClient *ghsa.Clie

func createPlaceholder(ctx context.Context, c *issues.Client, args []string) error {
for _, arg := range args {
if !cveschema5.IsCVE(arg) {
if !idstr.IsCVE(arg) {
return fmt.Errorf("%q is not a CVE", arg)
}
aliases := []string{arg}
Expand Down
8 changes: 4 additions & 4 deletions cmd/vulnreport/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import (
"time"

"golang.org/x/vulndb/cmd/vulnreport/log"
"golang.org/x/vulndb/internal/cveschema5"
"golang.org/x/vulndb/internal/genai"
"golang.org/x/vulndb/internal/genericosv"
"golang.org/x/vulndb/internal/ghsa"
"golang.org/x/vulndb/internal/gitrepo"
"golang.org/x/vulndb/internal/idstr"
"golang.org/x/vulndb/internal/issues"
"golang.org/x/vulndb/internal/osv"
"golang.org/x/vulndb/internal/proxy"
Expand Down Expand Up @@ -207,13 +207,13 @@ func createReport(ctx context.Context, iss *issues.Issue, pc *proxy.Client, gc *
func fetch(ctx context.Context, alias string, gc *ghsa.Client) report.Source {
var f report.Fetcher
switch {
case ghsa.IsGHSA(alias):
case idstr.IsGHSA(alias):
if *graphQL {
f = report.LegacyGHSAFetcher(gc)
} else {
f = genericosv.NewFetcher()
}
case cveschema5.IsCVE(alias):
case idstr.IsCVE(alias):
f = report.CVE5Fetcher()
default:
log.Warnf("alias %s is not supported, creating basic report", alias)
Expand Down Expand Up @@ -305,7 +305,7 @@ func parseGithubIssue(iss *issues.Issue, pc *proxy.Client) (*parsedIssue, error)
switch {
case p == "x/vulndb:":
continue
case cveschema5.IsCVE(p) || ghsa.IsGHSA(p):
case idstr.IsAliasType(p):
parsed.aliases = append(parsed.aliases, strings.TrimSuffix(p, ","))
case strings.HasSuffix(p, ":") || strings.Contains(p, "/"):
// Remove backslashes.
Expand Down
10 changes: 5 additions & 5 deletions cmd/vulnreport/find_aliases.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (

"golang.org/x/exp/slices"
"golang.org/x/vulndb/cmd/vulnreport/log"
"golang.org/x/vulndb/internal/cveschema5"
"golang.org/x/vulndb/internal/ghsa"
"golang.org/x/vulndb/internal/idstr"
"golang.org/x/vulndb/internal/report"
)

Expand Down Expand Up @@ -40,9 +40,9 @@ func removeRelated(all, related []string) []string {
func allAliases(ctx context.Context, knownAliases []string, gc *ghsa.Client) []string {
aliasesFor := func(ctx context.Context, alias string) ([]string, error) {
switch {
case ghsa.IsGHSA(alias):
case idstr.IsGHSA(alias):
return aliasesForGHSA(ctx, alias, gc)
case cveschema5.IsCVE(alias):
case idstr.IsCVE(alias):
return aliasesForCVE(ctx, alias, gc)
default:
return nil, fmt.Errorf("unsupported alias %s", alias)
Expand Down Expand Up @@ -109,8 +109,8 @@ func aliasesForCVE(ctx context.Context, cve string, gc *ghsa.Client) (aliases []
// If "preferCVE" is true, it prefers CVEs instead.
// If no GHSAs or CVEs are present, it returns ("", false).
func pickBestAlias(aliases []string, preferCVE bool) (_ string, ok bool) {
firstChoice := ghsa.IsGHSA
secondChoice := cveschema5.IsCVE
firstChoice := idstr.IsGHSA
secondChoice := idstr.IsCVE
if preferCVE {
firstChoice, secondChoice = secondChoice, firstChoice
}
Expand Down
3 changes: 2 additions & 1 deletion internal/cvelistrepo/cvelistrepo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"golang.org/x/vulndb/internal/cveschema"
"golang.org/x/vulndb/internal/cveschema5"
"golang.org/x/vulndb/internal/gitrepo"
"golang.org/x/vulndb/internal/idstr"
)

var update = flag.Bool("update", false, "update the .txtar files with real CVE data (this takes a while)")
Expand Down Expand Up @@ -183,7 +184,7 @@ func TestParse(t *testing.T) {
if err := Parse(repo, file, cve); err != nil {
t.Fatal(err)
}
want := cveschema5.FindCVE(file.Filename)
want := idstr.FindCVE(file.Filename)
if got := cveID(cve); got != want {
t.Errorf("ParseCVE(%s) ID = %s, want %s", file.Filename, got, want)
t.Log(cve)
Expand Down
4 changes: 2 additions & 2 deletions internal/cvelistrepo/txtar.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (

"github.com/go-git/go-git/v5/plumbing"
"golang.org/x/tools/txtar"
"golang.org/x/vulndb/internal/cveschema5"
"golang.org/x/vulndb/internal/gitrepo"
"golang.org/x/vulndb/internal/idstr"
"golang.org/x/vulndb/internal/test"
)

Expand Down Expand Up @@ -49,7 +49,7 @@ func WriteTxtarRepo(ctx context.Context, url string, filename string, cveIDs []s
idToFile := make(map[string]*File)
for _, f := range files {
f := f
id := cveschema5.FindCVE(f.Filename)
id := idstr.FindCVE(f.Filename)
if id != "" {
if _, ok := idToFile[id]; ok {
return fmt.Errorf("found duplicate record files for %s", id)
Expand Down
16 changes: 0 additions & 16 deletions internal/cveschema5/cveschema5.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ package cveschema5
import (
"encoding/json"
"os"
"regexp"
)

type CVERecord struct {
Expand Down Expand Up @@ -137,18 +136,3 @@ func ReadForPublish(filename string) (cveID string, toPublish *Containers, err e
}
return record.Metadata.ID, &record.Containers, nil
}

const RegexStr = `CVE-\d{4}-\d{4,}`

var (
Regex = regexp.MustCompile(RegexStr)
RegexStrict = regexp.MustCompile(`^` + RegexStr + `$`)
)

func IsCVE(s string) bool {
return RegexStrict.MatchString(s)
}

func FindCVE(s string) string {
return Regex.FindString(s)
}
8 changes: 0 additions & 8 deletions internal/cveschema5/cveschema5_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,3 @@ func TestRead(t *testing.T) {
t.Errorf("Read(%s) = %v\n want %v", f, got, want)
}
}

func TestFindCVE(t *testing.T) {
s := "something/CVE-1999-0004.json"
got, want := FindCVE(s), "CVE-1999-0004"
if got != want {
t.Errorf("FindCVE(%s) = %s, want %s", s, got, want)
}
}
9 changes: 4 additions & 5 deletions internal/cveutils/triage.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ import (
"errors"
"fmt"
"net/url"
"regexp"
"strings"

"golang.org/x/vulndb/internal/cveschema"
"golang.org/x/vulndb/internal/derrors"
"golang.org/x/vulndb/internal/ghsa"
"golang.org/x/vulndb/internal/idstr"
"golang.org/x/vulndb/internal/pkgsite"
"golang.org/x/vulndb/internal/stdlib"
"golang.org/x/vulndb/internal/worker/log"
Expand Down Expand Up @@ -168,12 +167,12 @@ func triageV4CVE(ctx context.Context, c *cveschema.CVE, pc *pkgsite.Client) (res
return nil, nil
}

var ghsaRegex = regexp.MustCompile(ghsa.Regex)

func GetAliasGHSAs(c *cveschema.CVE) []string {
var ghsas []string
for _, r := range c.References.Data {
ghsas = append(ghsas, ghsaRegex.FindAllString(r.URL, 1)...)
if ghsa := idstr.FindGHSA(r.URL); ghsa != "" {
ghsas = append(ghsas, ghsa)
}
}
return ghsas
}
7 changes: 3 additions & 4 deletions internal/genericosv/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import (
"strings"

osvschema "github.com/google/osv-scanner/pkg/models"
"golang.org/x/vulndb/internal/cveschema5"
"golang.org/x/vulndb/internal/ghsa"
"golang.org/x/vulndb/internal/idstr"
"golang.org/x/vulndb/internal/osv"
"golang.org/x/vulndb/internal/report"
)
Expand All @@ -25,9 +24,9 @@ func (osv *Entry) ToReport(string) *report.Report {
}
addAlias := func(alias string) {
switch {
case cveschema5.IsCVE(alias):
case idstr.IsCVE(alias):
r.CVEs = append(r.CVEs, alias)
case ghsa.IsGHSA(alias):
case idstr.IsGHSA(alias):
r.GHSAs = append(r.GHSAs, alias)
default:
r.UnknownAliases = append(r.UnknownAliases, alias)
Expand Down
9 changes: 0 additions & 9 deletions internal/ghsa/ghsa.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package ghsa
import (
"context"
"fmt"
"regexp"
"time"

"github.com/shurcooL/githubv4"
Expand Down Expand Up @@ -257,11 +256,3 @@ func (c *Client) FetchGHSA(ctx context.Context, ghsaID string) (_ *SecurityAdvis
}
return query.SA.securityAdvisory()
}

const Regex = `GHSA-[^-]{4}-[^-]{4}-[^-]{4}`

var ghsaStrict = regexp.MustCompile(`^` + Regex + `$`)

func IsGHSA(s string) bool {
return ghsaStrict.MatchString(s)
}
63 changes: 63 additions & 0 deletions internal/idstr/ids.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2024 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// Package idstr provides utilities for working with vulnerability
// identifier strings.
package idstr

import "regexp"

const ghsaStr = `GHSA-[^-]{4}-[^-]{4}-[^-]{4}`

var (
ghsaRE, ghsaStrict = re(ghsaStr)
)

func IsGHSA(s string) bool {
return ghsaStrict.MatchString(s)
}

func FindGHSA(s string) string {
return ghsaRE.FindString(s)
}

const cveStr = `CVE-\d{4}-\d{4,}`

var (
cveRE, cveStrict = re(cveStr)
)

func IsCVE(s string) bool {
return cveStrict.MatchString(s)
}

func FindCVE(s string) string {
return cveRE.FindString(s)
}

const goIDStr = `GO-\d{4}-\d{4,}`

var (
_, goIDStrict = re(goIDStr)
)

func IsGoID(s string) bool {
return goIDStrict.MatchString(s)
}

func re(s string) (*regexp.Regexp, *regexp.Regexp) {
return regexp.MustCompile(s), regexp.MustCompile(`^` + s + `$`)
}

// IsIdentifier returns whether the given ID is a recognized identifier
// (currently, either a GHSA, CVE, or Go ID).
func IsIdentifier(id string) bool {
return IsAliasType(id) || IsGoID(id)
}

// IsAliasType returns whether the given ID is a recognized alias type
// (currently, either a GHSA or CVE).
func IsAliasType(id string) bool {
return IsGHSA(id) || IsCVE(id)
}
15 changes: 15 additions & 0 deletions internal/idstr/ids_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2024 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package idstr

import "testing"

func TestFindCVE(t *testing.T) {
s := "something/CVE-1999-0004.json"
got, want := FindCVE(s), "CVE-1999-0004"
if got != want {
t.Errorf("FindCVE(%s) = %s, want %s", s, got, want)
}
}
18 changes: 18 additions & 0 deletions internal/idstr/links.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2024 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package idstr

import "regexp"

var (
NISTLink = regexp.MustCompile(`^https://nvd.nist.gov/vuln/detail/(` + cveStr + `)$`)
GHSALink = regexp.MustCompile(`^https://github.com/.*/(` + ghsaStr + `)$`)
MITRELink = regexp.MustCompile(`^https://cve.mitre.org/.*(` + cveStr + `)$`)
goAdvisoryLink = regexp.MustCompile(`^https://pkg.go.dev/vuln/(` + goIDStr + `)$`)
)

func IsGoAdvisory(u string) bool {
return goAdvisoryLink.MatchString(u)
}
10 changes: 3 additions & 7 deletions internal/osvutils/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@ package osvutils
import (
"errors"
"fmt"
"regexp"
"strings"

"golang.org/x/vulndb/internal/cveschema5"
"golang.org/x/vulndb/internal/derrors"
"golang.org/x/vulndb/internal/ghsa"
"golang.org/x/vulndb/internal/idstr"
"golang.org/x/vulndb/internal/osv"
"golang.org/x/vulndb/internal/version"
)
Expand Down Expand Up @@ -71,8 +69,6 @@ var (
errNoIntroducedOrFixed = errors.New("introduced or fixed must be set")
errBothIntroducedAndFixed = errors.New("introduced and fixed cannot both be set in same event")
errInvalidSemver = errors.New("invalid or non-canonical semver version")

pkgsiteLinkRegex = regexp.MustCompile(`^https://pkg.go.dev/vuln/GO-\d{4}-\d{4,}$`)
)

func validate(e *osv.Entry, checkTimestamps bool) (err error) {
Expand Down Expand Up @@ -111,7 +107,7 @@ func validate(e *osv.Entry, checkTimestamps bool) (err error) {
}
}
for _, alias := range e.Aliases {
if !ghsa.IsGHSA(alias) && !cveschema5.IsCVE(alias) {
if !idstr.IsAliasType(alias) {
return fmt.Errorf("%w (found alias %s)", errInvalidAlias, alias)
}
}
Expand Down Expand Up @@ -250,7 +246,7 @@ func validateEcosystemSpecific(es *osv.EcosystemSpecific, module string) error {
}

func validateDatabaseSpecific(d *osv.DatabaseSpecific) error {
if !pkgsiteLinkRegex.MatchString(d.URL) {
if !idstr.IsGoAdvisory(d.URL) {
return fmt.Errorf("%w (found URL %q)", errInvalidPkgsiteURL, d.URL)
}
return nil
Expand Down

0 comments on commit 685ac19

Please sign in to comment.