internal/vulncheck: add position for sinks in findings' trace

The sink is the vulnerable function. Before, we wouldn't show anything as we are only showing positions of calls. Now, we include the position where the vulnerable symbol is defined. This will not have effect on default text output. It will though on -show traces output. The main beneficiary of this change are integration points that will now be able to jump to the definition of the vulnerable symbol. Change-Id: Ie156bf5d05dc1c743f118f4d14dba6e2c263549b Reviewed-on: https://go-review.googlesource.com/c/vuln/+/559275 Reviewed-by: Maceo Thompson <maceothompson@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
golang · Feb 6, 2024 · 0b50c25 · 0b50c25
1 parent f50d9a6
commit 0b50c25
Show file tree

Hide file tree

Showing 8 changed files with 85 additions and 27 deletions.
diff --git a/cmd/govulncheck/testdata/testfiles/source-call/source_call_json.ct b/cmd/govulncheck/testdata/testfiles/source-call/source_call_json.ct
@@ -249,7 +249,13 @@ $ govulncheck -C ${moddir}/vuln -json ./...
         "version": "v1.6.5",
         "package": "github.com/tidwall/gjson",
         "function": "Get",
-        "receiver": "Result"
+        "receiver": "Result",
+        "position": {
+          "filename": ".../gjson.go",
+          "offset": 5744,
+          "line": 296,
+          "column": 17
+        }
       },
       {
         "module": "golang.org/vuln",
@@ -364,7 +370,13 @@ $ govulncheck -C ${moddir}/vuln -json ./...
         "module": "golang.org/x/text",
         "version": "v0.3.0",
         "package": "golang.org/x/text/language",
-        "function": "Parse"
+        "function": "Parse",
+        "position": {
+          "filename": ".../parse.go",
+          "offset": 5808,
+          "line": 228,
+          "column": 6
+        }
       },
       {
         "module": "golang.org/vuln",
@@ -478,7 +490,13 @@ $ govulncheck -C ${moddir}/vuln -json ./...
         "version": "v1.6.5",
         "package": "github.com/tidwall/gjson",
         "function": "ForEach",
-        "receiver": "Result"
+        "receiver": "Result",
+        "position": {
+          "filename": ".../gjson.go",
+          "offset": 4415,
+          "line": 220,
+          "column": 17
+        }
       },
       {
         "module": "github.com/tidwall/gjson",

diff --git a/cmd/govulncheck/testdata/testfiles/source-call/source_call_text.ct b/cmd/govulncheck/testdata/testfiles/source-call/source_call_text.ct
@@ -61,7 +61,7 @@ Vulnerability #1: GO-2021-0265
     Example traces found:
       #1: for function github.com/tidwall/gjson.Result.Get
         .../vuln.go:14:20: golang.org/vuln.main
-        github.com/tidwall/gjson.Result.Get
+        .../gjson.go:296:17: github.com/tidwall/gjson.Result.Get
 
 Vulnerability #2: GO-2021-0113
     Due to improper index calculation, an incorrectly formatted language tag can
@@ -75,7 +75,7 @@ Vulnerability #2: GO-2021-0113
     Example traces found:
       #1: for function golang.org/x/text/language.Parse
         .../vuln.go:13:16: golang.org/vuln.main
-        golang.org/x/text/language.Parse
+        .../parse.go:228:6: golang.org/x/text/language.Parse
 
 Vulnerability #3: GO-2021-0054
     Due to improper bounds checking, maliciously crafted JSON objects can cause
@@ -92,7 +92,7 @@ Vulnerability #3: GO-2021-0054
         .../gjson.go:1881:36: github.com/tidwall/gjson.Get
         .../gjson.go:2587:21: github.com/tidwall/gjson.execModifier
         .../gjson.go:2631:21: github.com/tidwall/gjson.modPretty
-        github.com/tidwall/gjson.Result.ForEach
+        .../gjson.go:220:17: github.com/tidwall/gjson.Result.ForEach
 
 Your code is affected by 3 vulnerabilities from 2 modules.
 This scan also found 0 vulnerabilities in packages you import and 2

diff --git a/cmd/govulncheck/testdata/testfiles/source-call/source_multientry_json.ct b/cmd/govulncheck/testdata/testfiles/source-call/source_multientry_json.ct
@@ -240,7 +240,13 @@ $ govulncheck -json -C ${moddir}/multientry .
         "module": "golang.org/x/text",
         "version": "v0.3.5",
         "package": "golang.org/x/text/language",
-        "function": "MustParse"
+        "function": "MustParse",
+        "position": {
+          "filename": ".../tags.go",
+          "offset": 427,
+          "line": 13,
+          "column": 6
+        }
       },
       {
         "module": "golang.org/multientry",
@@ -287,7 +293,13 @@ $ govulncheck -json -C ${moddir}/multientry .
         "module": "golang.org/x/text",
         "version": "v0.3.5",
         "package": "golang.org/x/text/language",
-        "function": "Parse"
+        "function": "Parse",
+        "position": {
+          "filename": ".../parse.go",
+          "offset": 1121,
+          "line": 33,
+          "column": 6
+        }
       },
       {
         "module": "golang.org/multientry",

diff --git a/cmd/govulncheck/testdata/testfiles/source-call/source_multientry_text.ct b/cmd/govulncheck/testdata/testfiles/source-call/source_multientry_text.ct
@@ -45,11 +45,11 @@ Vulnerability #1: GO-2021-0113
         .../main.go:26:3: golang.org/multientry.main
         .../main.go:48:8: golang.org/multientry.D
         .../main.go:99:20: golang.org/multientry.foobar
-        golang.org/x/text/language.MustParse
+        .../tags.go:13:6: golang.org/x/text/language.MustParse
       #2: for function golang.org/x/text/language.Parse
         .../main.go:22:3: golang.org/multientry.main
         .../main.go:44:23: golang.org/multientry.C
-        golang.org/x/text/language.Parse
+        .../parse.go:33:6: golang.org/x/text/language.Parse
 
 === Package Results ===
 

diff --git a/cmd/govulncheck/testdata/testfiles/source-call/source_stdlib_text.ct b/cmd/govulncheck/testdata/testfiles/source-call/source_stdlib_text.ct
@@ -39,7 +39,7 @@ Vulnerability #1: GO-2022-0969
     Example traces found:
       #1: for function net/http.ListenAndServe
         .../stdlib.go:17:31: golang.org/stdlib.main
-        net/http.ListenAndServe
+        .../server.go:3439:6: net/http.ListenAndServe
 
 Your code is affected by 1 vulnerability from the Go standard library.
 This scan found no other vulnerabilities in packages you import or modules you

diff --git a/cmd/govulncheck/testdata/testfiles/source-call/source_subdir_text.ct b/cmd/govulncheck/testdata/testfiles/source-call/source_subdir_text.ct
@@ -42,7 +42,7 @@ Vulnerability #1: GO-2021-0113
     Example traces found:
       #1: for function golang.org/x/text/language.Parse
         .../subdir.go:8:16: golang.org/vuln/subdir.Foo
-        golang.org/x/text/language.Parse
+        .../parse.go:228:6: golang.org/x/text/language.Parse
 
 Your code is affected by 1 vulnerability from 1 module.
 This scan also found 0 vulnerabilities in packages you import and 2

diff --git a/cmd/govulncheck/testdata/testfiles/source-call/source_vendored_json.ct b/cmd/govulncheck/testdata/testfiles/source-call/source_vendored_json.ct
@@ -249,7 +249,13 @@ $ govulncheck -C ${moddir}/vendored -json ./...
         "version": "v1.6.5",
         "package": "github.com/tidwall/gjson",
         "function": "Get",
-        "receiver": "Result"
+        "receiver": "Result",
+        "position": {
+          "filename": ".../gjson.go",
+          "offset": 81,
+          "line": 7,
+          "column": 15
+        }
       },
       {
         "module": "private.com/privateuser/fakemod",
@@ -376,7 +382,13 @@ $ govulncheck -C ${moddir}/vendored -json ./...
         "module": "golang.org/x/text",
         "version": "v0.3.0",
         "package": "golang.org/x/text/language",
-        "function": "Parse"
+        "function": "Parse",
+        "position": {
+          "filename": ".../language.go",
+          "offset": 53,
+          "line": 5,
+          "column": 6
+        }
       },
       {
         "module": "golang.org/vendored",

diff --git a/internal/vulncheck/emit.go b/internal/vulncheck/emit.go
@@ -5,6 +5,7 @@
 package vulncheck
 
 import (
+	"go/token"
 	"sort"
 
 	"golang.org/x/tools/go/packages"
@@ -76,39 +77,54 @@ func emitCallFindings(handler govulncheck.Handler, callstacks map[*Vuln]CallStac
 		if err := handler.Finding(&govulncheck.Finding{
 			OSV:          vuln.OSV.ID,
 			FixedVersion: fixed,
-			Trace:        tracefromEntries(stack),
+			Trace:        traceFromEntries(stack),
 		}); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-// tracefromEntries creates a sequence of
+// traceFromEntries creates a sequence of
 // frames from vcs. Position of a Frame is the
 // call position of the corresponding stack entry.
-func tracefromEntries(vcs CallStack) []*govulncheck.Frame {
+func traceFromEntries(vcs CallStack) []*govulncheck.Frame {
 	var frames []*govulncheck.Frame
 	for i := len(vcs) - 1; i >= 0; i-- {
 		e := vcs[i]
 		fr := frameFromPackage(e.Function.Package)
 		fr.Function = e.Function.Name
 		fr.Receiver = e.Function.Receiver()
-		if e.Call == nil || e.Call.Pos == nil {
-			fr.Position = nil
-		} else {
-			fr.Position = &govulncheck.Position{
-				Filename: e.Call.Pos.Filename,
-				Offset:   e.Call.Pos.Offset,
-				Line:     e.Call.Pos.Line,
-				Column:   e.Call.Pos.Column,
-			}
-		}
+		isSink := i == (len(vcs) - 1)
+		fr.Position = posFromStackEntry(e, isSink)
 		frames = append(frames, fr)
 	}
 	return frames
 }
 
+func posFromStackEntry(e StackEntry, sink bool) *govulncheck.Position {
+	var p *token.Position
+	if sink && e.Function != nil && e.Function.Pos != nil {
+		// For sinks, i.e., vulns we take the position
+		// of the symbol.
+		p = e.Function.Pos
+	} else if e.Call != nil && e.Call.Pos != nil {
+		// Otherwise, we take the position of
+		// the call statement.
+		p = e.Call.Pos
+	}
+
+	if p == nil {
+		return nil
+	}
+	return &govulncheck.Position{
+		Filename: p.Filename,
+		Offset:   p.Offset,
+		Line:     p.Line,
+		Column:   p.Column,
+	}
+}
+
 func frameFromPackage(pkg *packages.Package) *govulncheck.Frame {
 	fr := &govulncheck.Frame{}
 	if pkg != nil {