Skip to content

Commit 992f209

Browse files
committed
internal/proxy, cmd/vulnreport: fix infinite loop on root module paths
Prevent FindModule from looping indefinitely when path is "/" or contains a leading slash where path.Dir returns itself. Also ignore standalone slashes in vulnreport issue title parsing. Change-Id: I76f19ab826d4816a949686079fdd7f25009c0bba Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/821280 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com>
1 parent 9e978c8 commit 992f209

4 files changed

Lines changed: 77 additions & 7 deletions

File tree

cmd/vulnreport/creator.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"fmt"
1010
"net/http"
1111
"os"
12+
"regexp"
1213
"slices"
1314
"strings"
1415

@@ -319,21 +320,22 @@ func excludedReason(iss *issues.Issue) report.ExcludedType {
319320
return ""
320321
}
321322

323+
var modulePathRegexp = regexp.MustCompile(`^"?([[:alpha:]][\w./-]*[./][\w-]+)"?:?$`)
324+
322325
func modulePath(iss *issues.Issue) string {
323-
for _, p := range strings.Fields(iss.Title) {
326+
for p := range strings.FieldsSeq(iss.Title) {
324327
if p == "x/vulndb:" {
325328
continue
326329
}
327-
if strings.HasSuffix(p, ":") || strings.Contains(p, "/") {
328-
// Remove backslashes.
329-
return strings.ReplaceAll(strings.TrimSuffix(p, ":"), "\"", "")
330+
if m := modulePathRegexp.FindStringSubmatch(p); len(m) > 1 {
331+
return m[1]
330332
}
331333
}
332334
return ""
333335
}
334336

335337
func aliases(iss *issues.Issue) (aliases []string) {
336-
for _, p := range strings.Fields(iss.Title) {
338+
for p := range strings.FieldsSeq(iss.Title) {
337339
if idstr.IsAliasType(p) {
338340
aliases = append(aliases, strings.TrimSuffix(p, ","))
339341
}
@@ -417,7 +419,8 @@ func addReferenceTODOs(r *yamlReport) {
417419
todos := []*report.Reference{
418420
{Type: osv.ReferenceTypeAdvisory, URL: "TODO: canonical security advisory"},
419421
{Type: osv.ReferenceTypeReport, URL: "TODO: issue tracker link"},
420-
{Type: osv.ReferenceTypeFix, URL: "TODO: PR or commit (commit preferred)"}}
422+
{Type: osv.ReferenceTypeFix, URL: "TODO: PR or commit (commit preferred)"},
423+
}
421424

422425
types := make(map[osv.ReferenceType]bool)
423426
for _, r := range r.References {

cmd/vulnreport/vulnreport_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ package main
66

77
import (
88
"testing"
9+
10+
"golang.org/x/vulndb/internal/issues"
911
)
1012

1113
func TestCreate(t *testing.T) {
@@ -30,6 +32,51 @@ func TestCreate(t *testing.T) {
3032
}
3133
}
3234

35+
func TestModulePath(t *testing.T) {
36+
testCases := []struct {
37+
title string
38+
want string
39+
}{
40+
{
41+
title: "x/vulndb: potential Go vuln in github.com/foo/bar: GHSA-xxxx",
42+
want: "github.com/foo/bar",
43+
},
44+
{
45+
title: "x/vulndb: update fixed versions for GO-2026-4513 / duplicate GO-2026-4740",
46+
want: "",
47+
},
48+
{
49+
title: "x/vulndb: potential Go vuln in crypto/tls: CVE-2025-0001",
50+
want: "crypto/tls",
51+
},
52+
{
53+
title: `x/vulndb: potential Go vuln in "github.com/foo/bar": GHSA-xxxx`,
54+
want: "github.com/foo/bar",
55+
},
56+
{
57+
title: "x/vulndb: potential Go vuln in collectd.org: CVE-2021-0000",
58+
want: "collectd.org",
59+
},
60+
{
61+
title: "x/vulndb: potential Go vuln in 1234/foo: GHSA-xxxx",
62+
want: "",
63+
},
64+
{
65+
title: "x/vulndb: potential Go vuln in 4.15.2/foo: GHSA-xxxx",
66+
want: "",
67+
},
68+
}
69+
70+
for _, tc := range testCases {
71+
t.Run(tc.title, func(t *testing.T) {
72+
iss := &issues.Issue{Title: tc.title}
73+
if got := modulePath(iss); got != tc.want {
74+
t.Errorf("modulePath(%q) = %q, want %q", tc.title, got, tc.want)
75+
}
76+
})
77+
}
78+
}
79+
3380
func TestCreateExcluded(t *testing.T) {
3481
for _, tc := range []*testCase{
3582
// TODO(tatianabradley): add test cases

internal/proxy/proxy.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,15 @@ var errNoModuleFound = errors.New("no module found")
258258
func (c *Client) FindModule(path string) (modPath string, err error) {
259259
derrors.Wrap(&err, "FindModule(%s)", path)
260260

261-
for candidate := path; candidate != "."; candidate = urlpath.Dir(candidate) {
261+
for candidate := path; candidate != "." && candidate != "/"; {
262262
if c.ModuleExists(candidate) {
263263
return candidate, nil
264264
}
265+
next := urlpath.Dir(candidate)
266+
if next == candidate {
267+
break
268+
}
269+
candidate = next
265270
}
266271

267272
return "", errNoModuleFound

internal/proxy/proxy_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,21 @@ func TestFindModule(t *testing.T) {
286286
path: "github.com/RobotsAndPencils/go-saml/util",
287287
want: "github.com/RobotsAndPencils/go-saml",
288288
},
289+
{
290+
name: "slash path",
291+
path: "/",
292+
wantErr: errNoModuleFound,
293+
},
294+
{
295+
name: "leading slash path",
296+
path: "/foo/bar",
297+
wantErr: errNoModuleFound,
298+
},
299+
{
300+
name: "empty path",
301+
path: "",
302+
wantErr: errNoModuleFound,
303+
},
289304
}
290305

291306
for _, tc := range tcs {

0 commit comments

Comments
 (0)