internal/govulncheck: do not show anonymous functions in call stacks
Their names are likely to be confusing to users. Instead, replace a
call to an anonymous function A$1 with "B calls A", where B is the
previous non-anonymous function in the call stack leading to call of
A$1 and A is the function creating A$1. This is motivated by real
world example that can be simplified to
func B() {
A()()
}
func A() {
return func() {
//...
vuln()
}
}
Change-Id: Ie353b7cfa8d96e2e7a8d67dfb4e7f4619145a56d
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/454195
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
diff --git a/cmd/govulncheck/testdata/manystacks.ct b/cmd/govulncheck/testdata/manystacks.ct
index b35ce36..0348de0 100644
--- a/cmd/govulncheck/testdata/manystacks.ct
+++ b/cmd/govulncheck/testdata/manystacks.ct
@@ -11,7 +11,7 @@
Call stacks in your code:
.../main.go:16:14: example.com/manystacks.main calls github.com/shiyanhui/dht.New
- .../main.go:30:28: example.com/manystacks.main$2 calls example.com/manystacks/otherpkg.GetPeers, which eventually calls github.com/shiyanhui/dht.DHT.GetPeers
+ .../main.go:27:2: example.com/manystacks.main calls example.com/manystacks/otherpkg.GetPeers, which eventually calls github.com/shiyanhui/dht.DHT.GetPeers
.../main.go:44:7: example.com/manystacks.main calls github.com/shiyanhui/dht.DHT.Run
Found in: github.com/shiyanhui/dht@v0.0.0-20201219151056-5a20f3199263
diff --git a/internal/govulncheck/callstacks.go b/internal/govulncheck/callstacks.go
index f97e1bc..b66c4a8 100644
--- a/internal/govulncheck/callstacks.go
+++ b/internal/govulncheck/callstacks.go
@@ -116,41 +116,148 @@
}
// summarizeCallStack returns a short description of the call stack.
-// It uses one of two forms, depending on what the lowest function F in topPkgs
-// calls:
-// - If it calls a function V from the vulnerable package, then summarizeCallStack
-// returns "F calls V".
-// - If it calls a function G in some other package, which eventually calls V,
-// it returns "F calls G, which eventually calls V".
+// It uses one of four forms, depending on what the lowest function F
+// in topPkgs calls and what is the highest function V of vulnPkg:
+// - If F calls V directly and F as well as V are not anonymous functions:
+// "F calls V"
+// - The same case as above except F calls function G in some other package:
+// "F calls G, which eventually calls V"
+// - If F is an anonymous function, created by function G, and H is the
+// lowest non-anonymous function in topPkgs:
+// "H calls G, which eventually calls V"
+// - If V is an anonymous function, created by function W:
+// "F calls W, which eventually calls V"
//
// If it can't find any of these functions, summarizeCallStack returns the empty string.
func summarizeCallStack(cs CallStack, topPkgs map[string]bool, vulnPkg string) string {
- // Find the lowest function in the top packages.
- iTop := lowest(cs.Frames, func(e *StackFrame) bool {
- return topPkgs[e.PkgPath]
- })
+ iTop, iTopEnd, topFunc, topEndFunc := summarizeTop(cs.Frames, topPkgs)
if iTop < 0 {
return ""
}
- // Find the highest function in the vulnerable package that is below iTop.
- iVuln := highest(cs.Frames[iTop+1:], func(e *StackFrame) bool {
- return e.PkgPath == vulnPkg
- })
- if iVuln < 0 {
+
+ iVulnStart, vulnStartFunc, vulnFunc := summarizeVuln(cs.Frames, iTopEnd, vulnPkg)
+ if iVulnStart < 0 {
return ""
}
- iVuln += iTop + 1 // adjust for slice in call to highest.
- topName := cs.Frames[iTop].Name()
+
topPos := internal.AbsRelShorter(cs.Frames[iTop].Pos())
if topPos != "" {
topPos += ": "
}
- vulnName := cs.Frames[iVuln].Name()
- if iVuln == iTop+1 {
- return fmt.Sprintf("%s%s calls %s", topPos, topName, vulnName)
+
+ // The invariant is that the summary will always mention at most three functions
+ // and never mention an anonymous function. It prioritizes summarizing top of the
+ // stack as that is what the user has the most control of. For instance, if both
+ // the top and vuln portions of the stack are each summarized with two functions,
+ // then the final summary will mention the two functions of the top segment and
+ // only one from the vuln segment.
+ if topFunc != topEndFunc {
+ // The last function of the top segment is anonymous.
+ return fmt.Sprintf("%s%s calls %s, which eventually calls %s", topPos, topFunc, topEndFunc, vulnFunc)
}
- return fmt.Sprintf("%s%s calls %s, which eventually calls %s",
- topPos, topName, cs.Frames[iTop+1].Name(), vulnName)
+ if iVulnStart != iTopEnd+1 {
+ // If there is something in between top and vuln segments of
+ // the stack, then also summarize that intermediate segment.
+ return fmt.Sprintf("%s%s calls %s, which eventually calls %s", topPos, topFunc, cs.Frames[iTopEnd+1].Name(), vulnFunc)
+ }
+ if vulnStartFunc != vulnFunc {
+ // The first function of the vuln segment is anonymous.
+ return fmt.Sprintf("%s%s calls %s, which eventually calls %s", topPos, topFunc, vulnStartFunc, vulnFunc)
+ }
+ return fmt.Sprintf("%s%s calls %s", topPos, topFunc, vulnFunc)
+}
+
+// summarizeTop returns summary information for the beginning segment
+// of call stack frames that belong to topPkgs. It returns the latest,
+// e.g., lowest function in this segment and its index in frames. If
+// that function is anonymous, then summarizeTop also returns the
+// lowest non-anonymous function and its index in frames. In that case,
+// the anonymous function is replaced by the function that created it.
+//
+// [p.V p.W q.Q ...] -> (1, 1, p.W, p.W)
+// [p.V p.W p.Z$1 q.Q ...] -> (1, 2, p.W, p.Z)
+func summarizeTop(frames []*StackFrame, topPkgs map[string]bool) (iTop, iTopEnd int, topFunc, topEndFunc string) {
+ iTopEnd = lowest(frames, func(e *StackFrame) bool {
+ return topPkgs[e.PkgPath]
+ })
+ if iTopEnd < 0 {
+ return -1, -1, "", ""
+ }
+
+ topEndFunc = frames[iTopEnd].Name()
+ if !isAnonymousFunction(topEndFunc) {
+ iTop = iTopEnd
+ topFunc = topEndFunc
+ return
+ }
+
+ topEndFunc = creatorName(topEndFunc)
+
+ iTop = lowest(frames, func(e *StackFrame) bool {
+ return topPkgs[e.PkgPath] && !isAnonymousFunction(e.FuncName)
+ })
+ if iTop < 0 {
+ iTop = iTopEnd
+ topFunc = topEndFunc // for sanity
+ return
+ }
+
+ topFunc = frames[iTop].Name()
+ return
+}
+
+// summarizeVuln returns summary information for the final segment
+// of call stack frames that belong to vulnPkg. It returns the earliest,
+// e.g., highest function in this segment and its index in frames. If
+// that function is anonymous, then summarizeVuln also returns the
+// highest non-anonymous function. In that case, the anonymous function
+// is replaced by the function that created it.
+//
+// [x x q.Q v.V v.W] -> (3, v.V, v.V)
+// [x x q.Q v.V$1 v.W] -> (3, v.V, v.W)
+func summarizeVuln(frames []*StackFrame, iTop int, vulnPkg string) (iVulnStart int, vulnStartFunc, vulnFunc string) {
+ iVulnStart = highest(frames[iTop+1:], func(e *StackFrame) bool {
+ return e.PkgPath == vulnPkg
+ })
+ if iVulnStart < 0 {
+ return -1, "", ""
+ }
+ iVulnStart += iTop + 1 // adjust for slice in call to highest.
+
+ vulnStartFunc = frames[iVulnStart].Name()
+ if !isAnonymousFunction(vulnStartFunc) {
+ vulnFunc = vulnStartFunc
+ return
+ }
+
+ vulnStartFunc = creatorName(vulnStartFunc)
+
+ iVuln := highest(frames[iVulnStart:], func(e *StackFrame) bool {
+ return e.PkgPath == vulnPkg && !isAnonymousFunction(e.FuncName)
+ })
+ if iVuln < 0 {
+ vulnFunc = vulnStartFunc // for sanity
+ return
+ }
+
+ vulnFunc = frames[iVuln+iVulnStart].Name()
+ return
+}
+
+// creatorName returns the name of the function that created
+// the anonymous function anonFuncName. Assumes anonFuncName
+// is of the form <name>$1...
+func creatorName(anonFuncName string) string {
+ vs := strings.Split(anonFuncName, "$")
+ if len(vs) == 1 {
+ return anonFuncName
+ }
+ return vs[0]
+}
+
+func isAnonymousFunction(funcName string) bool {
+ // anonymous functions have $ sign in their name (naming done by ssa)
+ return strings.ContainsRune(funcName, '$')
}
// uniqueCallStack returns the first unique call stack among css, if any.
diff --git a/internal/govulncheck/callstacks_test.go b/internal/govulncheck/callstacks_test.go
index 8e6aef8..2ff2e6e 100644
--- a/internal/govulncheck/callstacks_test.go
+++ b/internal/govulncheck/callstacks_test.go
@@ -87,9 +87,25 @@
"t2.G calls v.V1",
},
{
+ "t1.F t2.G v.V$1 v.V1",
+ "t2.G calls v.V, which eventually calls v.V1",
+ },
+ {
+ "t1.F t2.G$1 v.V1",
+ "t1.F calls t2.G, which eventually calls v.V1",
+ },
+ {
+ "t1.F t2.G$1 v.V$1 v.V1",
+ "t1.F calls t2.G, which eventually calls v.V1",
+ },
+ {
"t1.F x.Y t2.G a.H b.I c.J v.V",
"t2.G calls a.H, which eventually calls v.V",
},
+ {
+ "t1.F x.Y t2.G a.H b.I c.J v.V$1 v.V1",
+ "t2.G calls a.H, which eventually calls v.V1",
+ },
} {
in := stringToCallStack(test.in)
got := summarizeCallStack(in, topPkgs, vulnPkg)
diff --git a/internal/govulncheck/util.go b/internal/govulncheck/util.go
index aba3adb..25a10c6 100644
--- a/internal/govulncheck/util.go
+++ b/internal/govulncheck/util.go
@@ -73,7 +73,7 @@
return -1
}
-// lowest returns the lowest (one with the largets index) entry in the call
+// lowest returns the lowest (one with the largest index) entry in the call
// stack for which f returns true.
func lowest(cs []*StackFrame, f func(e *StackFrame) bool) int {
for i := len(cs) - 1; i >= 0; i-- {
