Skip to content

Commit a9d9eed

Browse files
tatianabtatianab
committed
internal/{frontend,vuln}: make vuln search case-insensitive
Vulnerability search (by Go ID, CVE or GHSA) is now case-insensitive. Also fixes the regular expression for GHSAs which was too permissive. Change-Id: I2d53bfe41a1f8f78f6dd8774d3c9accc4461d101 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/506517 Run-TryBot: Tatiana Bradley <[email protected]> TryBot-Result: kokoro <[email protected]> Reviewed-by: Jamal Carvalho <[email protected]> Reviewed-by: Damien Neil <[email protected]>
1 parent d85a217 commit a9d9eed

File tree

6 files changed

+151
-54
lines changed

6 files changed

+151
-54
lines changed

‎internal/frontend/search.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -351,8 +351,8 @@ func searchRequestRedirectPath(ctx context.Context, ds internal.DataSource, quer
351351
if urlSchemeIdx > -1 {
352352
query = query[urlSchemeIdx+3:]
353353
}
354-
if vulnSupport && vuln.IsGoID(query) {
355-
return fmt.Sprintf("/vuln/%s?q", query)
354+
if id, ok := vuln.CanonicalGoID(query); vulnSupport && ok {
355+
return fmt.Sprintf("/vuln/%s?q", id)
356356
}
357357
requestedPath := path.Clean(query)
358358
if !strings.Contains(requestedPath, "/") || mode == searchModeVuln {
@@ -388,14 +388,15 @@ func searchVulnModule(ctx context.Context, mode, query string, client *vuln.Clie
388388
func searchVulnAlias(ctx context.Context, mode, cq string, vc *vuln.Client) (_ *searchAction, err error) {
389389
defer derrors.Wrap(&err, "searchVulnAlias(%q, %q)", mode, cq)
390390

391-
if mode != searchModeVuln || !vuln.IsAlias(cq) || vc == nil {
391+
alias, ok := vuln.CanonicalAlias(cq)
392+
if mode != searchModeVuln || !ok || vc == nil {
392393
return nil, nil
393394
}
394-
id, err := vc.ByAlias(ctx, cq)
395+
goID, err := vc.ByAlias(ctx, alias)
395396
if err != nil {
396397
return nil, &serverError{status: derrors.ToStatus(err)}
397398
}
398-
return &searchAction{redirectURL: "/vuln/" + id}, nil
399+
return &searchAction{redirectURL: "/vuln/" + goID}, nil
399400
}
400401

401402
// searchMode reports whether the search performed should be in package or
@@ -413,7 +414,7 @@ func searchMode(r *http.Request) string {
413414
case searchModeVuln:
414415
return searchModeVuln
415416
default:
416-
if vuln.IsAlias(q) {
417+
if _, ok := vuln.CanonicalAlias(q); ok {
417418
return searchModeVuln
418419
}
419420
if shouldDefaultToSymbolSearch(q) {

‎internal/frontend/search_test.go

+13-6
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func TestDetermineSearchAction(t *testing.T) {
108108
// See TestSearchVulnAlias in this file for more tests.
109109
{
110110
name: "vuln alias",
111-
query: "q=GHSA-aaaa-bbbb-cccc&m=vuln",
111+
query: "q=GHSA-cccc-ffff-gggg&m=vuln",
112112
wantRedirect: "/vuln/GO-1990-0001",
113113
},
114114
{
@@ -119,18 +119,18 @@ func TestDetermineSearchAction(t *testing.T) {
119119
{
120120
// We turn on vuln mode if the query matches a vuln alias.
121121
name: "vuln alias not vuln mode",
122-
query: "q=GHSA-aaaa-bbbb-cccc",
122+
query: "q=GHSA-cccc-ffff-gggg",
123123
wantRedirect: "/vuln/GO-1990-0001",
124124
},
125125
{
126126
name: "vuln alias with no match",
127-
query: "q=GHSA-aaaa-bbbb-dddd",
127+
query: "q=GHSA-cccc-ffff-xxxx",
128128
wantStatus: http.StatusNotFound,
129129
},
130130
{
131131
// An explicit mode overrides that.
132132
name: "vuln alias symbol mode",
133-
query: "q=GHSA-aaaa-bbbb-cccc?m=symbol",
133+
query: "q=GHSA-cccc-ffff-gggg?m=symbol",
134134
wantTemplate: "search",
135135
},
136136
{
@@ -502,7 +502,7 @@ func TestNewSearchResult(t *testing.T) {
502502
got := newSearchResult(&test.in, false, pr)
503503
test.want.CommitTime = "unknown"
504504
if diff := cmp.Diff(&test.want, got); diff != "" {
505-
t.Errorf("mimatch (-want, +got):\n%s", diff)
505+
t.Errorf("mismatch (-want, +got):\n%s", diff)
506506
}
507507
})
508508
}
@@ -540,6 +540,7 @@ func TestSearchRequestRedirectPath(t *testing.T) {
540540
{"non-existent path does not redirect", "github.com/non-existent", "", ""},
541541
{"trim URL scheme from query", "https://golang.org/x/tools", "/golang.org/x/tools", ""},
542542
{"Go vuln redirects", "GO-1969-0720", "/vuln/GO-1969-0720?q", ""},
543+
{"Lower-case Go vuln redirects", "go-1969-0720", "/vuln/GO-1969-0720?q", ""},
543544
{"not a Go vuln", "somepkg/GO-1969-0720", "", ""},
544545
// Just setting the search mode to vuln does not cause a redirect.
545546
{"search mode is vuln", "searchmodevuln", "", searchModeVuln},
@@ -593,7 +594,13 @@ func TestSearchVulnAlias(t *testing.T) {
593594
{
594595
name: "one match",
595596
mode: searchModeVuln,
596-
query: "GHSA-aaaa-bbbb-cccc",
597+
query: "GHSA-cccc-ffff-gggg",
598+
wantURL: "/vuln/GO-1990-0001",
599+
},
600+
{
601+
name: "one match - case insensitive",
602+
mode: searchModeVuln,
603+
query: "gHSa-ccCc-fFFF-gGgG",
597604
wantURL: "/vuln/GO-1990-0001",
598605
},
599606
} {

‎internal/frontend/vulns.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ func newVulnPage(ctx context.Context, url *url.URL, vc *vuln.Client) (*vulnPage,
8989
template: "vuln/list",
9090
title: "Vulnerability Reports"}, nil
9191
default: // the path should be "/<ID>", e.g. "/GO-2021-0001".
92-
id := strings.TrimPrefix(path, "/")
93-
if !vuln.IsGoID(id) {
92+
id, ok := vuln.CanonicalGoID(strings.TrimPrefix(path, "/"))
93+
if !ok {
9494
if url.Query().Has("q") {
9595
return nil, derrors.NotFound
9696
}

‎internal/frontend/vulns_test.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
)
1717

1818
var testEntries = []*osv.Entry{
19-
{ID: "GO-1990-0001", Details: "a", Aliases: []string{"CVE-2000-1", "GHSA-aaaa-bbbb-cccc"}},
19+
{ID: "GO-1990-0001", Details: "a", Aliases: []string{"CVE-2000-1", "GHSA-cccc-ffff-gggg"}},
2020
{ID: "GO-1990-0002", Details: "b", Aliases: []string{"CVE-2000-1", "GHSA-1111-2222-3333"}},
2121
{ID: "GO-1990-0010", Details: "c"},
2222
{ID: "GO-1991-0001", Details: "d"},
@@ -104,6 +104,18 @@ func TestNewVulnPage(t *testing.T) {
104104
title: "GO-1990-0002",
105105
},
106106
},
107+
{
108+
name: "vuln entry page - case insensitive",
109+
url: "https://pkg.go.dev/vuln/go-1990-0002",
110+
want: &vulnPage{
111+
page: &VulnEntryPage{
112+
Entry: testEntries[1],
113+
AliasLinks: aliasLinks(testEntries[1]),
114+
},
115+
template: "vuln/entry",
116+
title: "GO-1990-0002",
117+
},
118+
},
107119
}
108120

109121
for _, tc := range tcs {

‎internal/vuln/regexp.go

+38-11
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,48 @@
44

55
package vuln
66

7-
import "regexp"
7+
import (
8+
"regexp"
9+
"strings"
10+
)
11+
12+
const (
13+
ci = "(?i)" // case-insensitive
14+
goRE = "^GO-[0-9]{4}-[0-9]{4,}$"
15+
cveRE = "^CVE-[0-9]{4}-[0-9]+$"
16+
// Regexp adapted from https://github.com/github/advisory-database.
17+
ghsaRE = "^(GHSA)((-[23456789cfghjmpqrvwx]{4}){3})$"
18+
)
819

20+
// Case-insensitive regexps for vuln IDs/aliases.
921
var (
10-
goRegexp = regexp.MustCompile("^GO-[0-9]{4}-[0-9]{4,}$")
11-
cveRegexp = regexp.MustCompile("^CVE-[0-9]{4}-[0-9]+$")
12-
ghsaRegexp = regexp.MustCompile("^GHSA-.{4}-.{4}-.{4}$")
22+
goID = regexp.MustCompile(ci + goRE)
23+
cveID = regexp.MustCompile(ci + cveRE)
24+
ghsaID = regexp.MustCompile(ci + ghsaRE)
1325
)
1426

15-
// IsGoID returns whether s is a valid Go vulnerability ID.
16-
func IsGoID(s string) bool {
17-
return goRegexp.MatchString(s)
27+
// Canonical returns the canonical form of the given Go ID string
28+
// by correcting the case.
29+
//
30+
// If no canonical form can be found, returns false.
31+
func CanonicalGoID(id string) (_ string, ok bool) {
32+
if goID.MatchString(id) {
33+
return strings.ToUpper(id), true
34+
}
35+
return "", false
1836
}
1937

20-
// IsAlias returns whether s is a valid vulnerability alias
21-
// (CVE or GHSA).
22-
func IsAlias(s string) bool {
23-
return cveRegexp.MatchString(s) || ghsaRegexp.MatchString(s)
38+
// Canonical returns the canonical form of the given alias ID string
39+
// (a CVE or GHSA id) by correcting the case.
40+
//
41+
// If no canonical form can be found, returns false.
42+
func CanonicalAlias(id string) (_ string, ok bool) {
43+
if cveID.MatchString(id) {
44+
return strings.ToUpper(id), true
45+
}
46+
parts := ghsaID.FindStringSubmatch(id)
47+
if len(parts) != 4 {
48+
return "", false
49+
}
50+
return strings.ToUpper(parts[1]) + strings.ToLower(parts[2]), true
2451
}

‎internal/vuln/regexp_test.go

+78-28
Original file line numberDiff line numberDiff line change
@@ -6,65 +6,115 @@ package vuln
66

77
import "testing"
88

9-
func TestIsGoID(t *testing.T) {
9+
func TestCanonicalGoID(t *testing.T) {
1010
tests := []struct {
11-
id string
12-
want bool
11+
id string
12+
wantID string
13+
wantOK bool
1314
}{
1415
{
15-
id: "GO-1999-0001",
16-
want: true,
16+
id: "GO-1999-0001",
17+
wantID: "GO-1999-0001",
18+
wantOK: true,
1719
},
1820
{
19-
id: "GO-2023-12345678",
20-
want: true,
21+
id: "GO-1999-000111",
22+
wantID: "GO-1999-000111",
23+
wantOK: true,
2124
},
2225
{
23-
id: "GO-2023-123",
24-
want: false,
26+
id: "go-1999-0001",
27+
wantID: "GO-1999-0001",
28+
wantOK: true,
2529
},
2630
{
27-
id: "GO-abcd-0001",
28-
want: false,
31+
id: "GO-1999",
32+
wantID: "",
33+
wantOK: false,
2934
},
3035
{
31-
id: "CVE-1999-0001",
32-
want: false,
36+
id: "GHSA-cfgh-2345-rwxq",
37+
wantID: "",
38+
wantOK: false,
39+
},
40+
{
41+
id: "CVE-1999-000123",
42+
wantID: "",
43+
wantOK: false,
44+
},
45+
{
46+
id: "ghsa-Cfgh-2345-Rwxq",
47+
wantID: "",
48+
wantOK: false,
49+
},
50+
{
51+
id: "cve-1999-000123",
52+
wantID: "",
53+
wantOK: false,
54+
},
55+
{
56+
id: "cve-ghsa-go",
57+
wantID: "",
58+
wantOK: false,
3359
},
3460
}
3561
for _, tt := range tests {
3662
t.Run(tt.id, func(t *testing.T) {
37-
got := IsGoID(tt.id)
38-
if got != tt.want {
39-
t.Errorf("IsGoID(%s) = %t, want %t", tt.id, got, tt.want)
63+
gotID, gotOK := CanonicalGoID(tt.id)
64+
if gotID != tt.wantID || gotOK != tt.wantOK {
65+
t.Errorf("CanonicalGoID(%s) = (%s, %t), want (%s, %t)", tt.id, gotID, gotOK, tt.wantID, tt.wantOK)
4066
}
4167
})
4268
}
4369
}
4470

45-
func TestIsAlias(t *testing.T) {
71+
func TestCanonicalAlias(t *testing.T) {
4672
tests := []struct {
47-
id string
48-
want bool
73+
id string
74+
wantID string
75+
wantOK bool
4976
}{
5077
{
51-
id: "GO-1999-0001",
52-
want: false,
78+
id: "GO-1999-0001",
79+
wantID: "",
80+
wantOK: false,
81+
},
82+
{
83+
id: "GHSA-cfgh-2345-rwxq",
84+
wantID: "GHSA-cfgh-2345-rwxq",
85+
wantOK: true,
86+
},
87+
{
88+
id: "CVE-1999-000123",
89+
wantID: "CVE-1999-000123",
90+
wantOK: true,
91+
},
92+
{
93+
id: "go-1999-0001",
94+
wantID: "",
95+
wantOK: false,
96+
},
97+
{
98+
id: "ghsa-Cfgh-2345-Rwxq",
99+
wantID: "GHSA-cfgh-2345-rwxq",
100+
wantOK: true,
53101
},
54102
{
55-
id: "GHSA-abcd-1234-efgh",
56-
want: true,
103+
id: "cve-1999-000123",
104+
wantID: "CVE-1999-000123",
105+
wantOK: true,
57106
},
58107
{
59-
id: "CVE-1999-000123",
60-
want: true,
108+
id: "abc-CVE-1999-0001",
109+
wantID: "",
110+
wantOK: false,
61111
},
62112
}
63113
for _, tt := range tests {
64114
t.Run(tt.id, func(t *testing.T) {
65-
got := IsAlias(tt.id)
66-
if got != tt.want {
67-
t.Errorf("IsAlias(%s) = %t, want %t", tt.id, got, tt.want)
115+
gotID, gotOK := CanonicalAlias(tt.id)
116+
if gotID != tt.wantID || gotOK != tt.wantOK {
117+
t.Errorf("CanonicalAlias(%s) = (%s, %t), want (%s, %t)", tt.id, gotID, gotOK, tt.wantID, tt.wantOK)
68118
}
69119
})
70120
}

0 commit comments

Comments
 (0)