[vuln] internal/vulncheck: move code for filtering call stacks
3 views
Skip to first unread message
Zvonimir Pavlinovic (Gerrit)
Jun 20, 2023, 5:56:00 PM6/20/23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Zvonimir Pavlinovic has uploaded this change for review.
internal/vulncheck: move code for filtering call stacks
The code is now closer to call stack computation since
vulncheck.Callstacks is not exported anymore.
Change-Id: I90412fd2598c400358e3eb62e2ab353a3b790c84
---
M internal/scan/source.go
M internal/vulncheck/witness.go
2 files changed, 50 insertions(+), 50 deletions(-)
diff --git a/internal/scan/source.go b/internal/scan/source.go
index 2b02378..a1d8ff0 100644
--- a/internal/scan/source.go
+++ b/internal/scan/source.go
@@ -53,38 +53,9 @@
return err
}
callStacks := vulncheck.CallStacks(vr)
- filterCallStacks(callStacks)
return emitResult(handler, vr, callStacks)
}
-func filterCallStacks(callstacks map[*vulncheck.Vuln][]vulncheck.CallStack) {
- type key struct {
- id string
- pkg string
- mod string
- }
- // Collect all called symbols for a package.
- // Needed for creating unique call stacks.
- vulnsPerPkg := make(map[key][]*vulncheck.Vuln)
- for vv := range callstacks {
- if vv.CallSink != nil {
- k := key{id: vv.OSV.ID, pkg: vv.ImportSink.PkgPath, mod: vv.ImportSink.Module.Path}
- vulnsPerPkg[k] = append(vulnsPerPkg[k], vv)
- }
- }
- for vv, stacks := range callstacks {
- var filtered []vulncheck.CallStack
- if vv.CallSink != nil {
- k := key{id: vv.OSV.ID, pkg: vv.ImportSink.PkgPath, mod: vv.ImportSink.Module.Path}
- vcs := uniqueCallStack(vv, stacks, vulnsPerPkg[k])
- if vcs != nil {
- filtered = []vulncheck.CallStack{vcs}
- }
- }
- callstacks[vv] = filtered
- }
-}
-
func emitResult(handler govulncheck.Handler, vr *vulncheck.Result, callstacks map[*vulncheck.Vuln][]vulncheck.CallStack) error {
osvs := map[string]*osv.Entry{}
// first deal with all the affected vulnerabilities
@@ -338,23 +309,3 @@
// positive integer. Implicit inits are named simply "init".
return f.Name == "init" || strings.HasPrefix(f.Name, "init#")
}
-
-// uniqueCallStack returns the first unique call stack among css, if any.
-// Unique means that the call stack does not go through symbols of vg.
-func uniqueCallStack(v *vulncheck.Vuln, css []vulncheck.CallStack, vg []*vulncheck.Vuln) vulncheck.CallStack {
- vulnFuncs := make(map[*vulncheck.FuncNode]bool)
- for _, v := range vg {
- vulnFuncs[v.CallSink] = true
- }
-
-callstack:
- for _, cs := range css {
- for _, e := range cs {
- if e.Function != v.CallSink && vulnFuncs[e.Function] {
- continue callstack
- }
- }
- return cs
- }
- return nil
-}
diff --git a/internal/vulncheck/witness.go b/internal/vulncheck/witness.go
index bf01882..0a1c4da 100644
--- a/internal/vulncheck/witness.go
+++ b/internal/vulncheck/witness.go
@@ -58,8 +58,9 @@
wg.Done()
}()
}
-
wg.Wait()
+
+ filterCallStacks(stacksPerVuln)
return stacksPerVuln
}
@@ -269,3 +270,51 @@
// should happen only for inits
return f1.String() < f2.String()
}
+
+func filterCallStacks(callstacks map[*Vuln][]CallStack) {
+ type key struct {
+ id string
+ pkg string
+ mod string
+ }
+ // Collect all called symbols for a package.
+ // Needed for creating unique call stacks.
+ vulnsPerPkg := make(map[key][]*Vuln)
+ for vv := range callstacks {
+ if vv.CallSink != nil {
+ k := key{id: vv.OSV.ID, pkg: vv.ImportSink.PkgPath, mod: vv.ImportSink.Module.Path}
+ vulnsPerPkg[k] = append(vulnsPerPkg[k], vv)
+ }
+ }
+ for vv, stacks := range callstacks {
+ var filtered []CallStack
+ if vv.CallSink != nil {
+ k := key{id: vv.OSV.ID, pkg: vv.ImportSink.PkgPath, mod: vv.ImportSink.Module.Path}
+ vcs := uniqueCallStack(vv, stacks, vulnsPerPkg[k])
+ if vcs != nil {
+ filtered = []CallStack{vcs}
+ }
+ }
+ callstacks[vv] = filtered
+ }
+}
+
+// uniqueCallStack returns the first unique call stack among css, if any.
+// Unique means that the call stack does not go through symbols of vg.
+func uniqueCallStack(v *Vuln, css []CallStack, vg []*Vuln) CallStack {
+ vulnFuncs := make(map[*FuncNode]bool)
+ for _, v := range vg {
+ vulnFuncs[v.CallSink] = true
+ }
+
+callstack:
+ for _, cs := range css {
+ for _, e := range cs {
+ if e.Function != v.CallSink && vulnFuncs[e.Function] {
+ continue callstack
+ }
+ }
+ return cs
+ }
+ return nil
+}
To view, visit change 504716. To unsubscribe, or for help writing mail filters, visit settings.
Zvonimir Pavlinovic (Gerrit)
Jun 20, 2023, 6:13:19 PM6/20/23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Zvonimir Pavlinovic uploaded patch set #2 to this change.
internal/vulncheck: move code for filtering call stacks
The code is now closer to call stack computation since
vulncheck.Callstacks is not exported anymore.
Change-Id: I90412fd2598c400358e3eb62e2ab353a3b790c84
---
M internal/scan/source.go
M internal/scan/source_test.go
M internal/vulncheck/witness.go
M internal/vulncheck/witness_test.go
4 files changed, 103 insertions(+), 96 deletions(-)
To view, visit change 504716. To unsubscribe, or for help writing mail filters, visit settings.
Zvonimir Pavlinovic (Gerrit)
Jun 20, 2023, 6:14:00 PM6/20/23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Patch set 2:Run-TryBot +1
Zvonimir Pavlinovic (Gerrit)
Jun 20, 2023, 6:40:53 PM6/20/23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Zvonimir Pavlinovic uploaded patch set #3 to this change.
internal/vulncheck: move code for filtering call stacks
The code is now closer to call stack computation since
vulncheck.Callstacks is not exported anymore.
More importantly, this code now makes it more explicit
that govulncheck actually computes one call stack for
vulnerable symbol.
Change-Id: I90412fd2598c400358e3eb62e2ab353a3b790c84
---
M internal/scan/source.go
M internal/scan/source_test.go
M internal/vulncheck/witness.go
M internal/vulncheck/witness_test.go
4 files changed, 103 insertions(+), 96 deletions(-)
To view, visit change 504716. To unsubscribe, or for help writing mail filters, visit settings.
Zvonimir Pavlinovic (Gerrit)
Jun 20, 2023, 6:41:13 PM6/20/23
to goph...@pubsubhelper.golang.org, Ian Cottrell, Gopher Robot, golang-co...@googlegroups.com
Ian Cottrell (Gerrit)
Jun 22, 2023, 6:25:09 PM6/22/23
to Zvonimir Pavlinovic, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Zvonimir Pavlinovic.
Patch set 3:Code-Review +2
Zvonimir Pavlinovic (Gerrit)
Jun 22, 2023, 6:41:33 PM6/22/23
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Ian Cottrell, Gopher Robot, golang-co...@googlegroups.com
Zvonimir Pavlinovic submitted this change.
Approvals:
Zvonimir Pavlinovic: Run TryBots
Gopher Robot: TryBots succeeded
Ian Cottrell: Looks good to me, approved
internal/vulncheck: move code for filtering call stacks
The code is now closer to call stack computation since
vulncheck.Callstacks is not exported anymore.
More importantly, this code now makes it more explicit
that govulncheck actually computes one call stack for
vulnerable symbol.
Change-Id: I90412fd2598c400358e3eb62e2ab353a3b790c84
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/504716
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Zvonimir Pavlinovic <zpavl...@google.com>
Reviewed-by: Ian Cottrell <ianco...@google.com>
---
M internal/scan/source.go
M internal/scan/source_test.go
M internal/vulncheck/witness.go
M internal/vulncheck/witness_test.go
4 files changed, 103 insertions(+), 96 deletions(-)
diff --git a/internal/scan/source_test.go b/internal/scan/source_test.go
index ec0eb98..a637656 100644
--- a/internal/scan/source_test.go
+++ b/internal/scan/source_test.go
@@ -20,48 +20,6 @@
"golang.org/x/vuln/internal/vulncheck"
)
-func TestUniqueCallStack(t *testing.T) {
- a := &vulncheck.FuncNode{Name: "A"}
- b := &vulncheck.FuncNode{Name: "B"}
- v1 := &vulncheck.FuncNode{Name: "V1"}
- v2 := &vulncheck.FuncNode{Name: "V2"}
- v3 := &vulncheck.FuncNode{Name: "V3"}
-
- vuln1 := &vulncheck.Vuln{Symbol: "V1", CallSink: v1}
- vuln2 := &vulncheck.Vuln{Symbol: "V2", CallSink: v2}
- vuln3 := &vulncheck.Vuln{Symbol: "V3", CallSink: v3}
-
- callStack := func(fs ...*vulncheck.FuncNode) vulncheck.CallStack {
- var cs vulncheck.CallStack
- for _, f := range fs {
- cs = append(cs, vulncheck.StackEntry{Function: f})
- }
- return cs
- }
-
- // V1, V2, and V3 are vulnerable symbols
- skip := []*vulncheck.Vuln{vuln1, vuln2, vuln3}
- for _, test := range []struct {
- vuln *vulncheck.Vuln
- css []vulncheck.CallStack
- want vulncheck.CallStack
- }{
- // [A -> B -> V3 -> V1, A -> V1] ==> A -> V1 since the first stack goes through V3
- {vuln1, []vulncheck.CallStack{callStack(a, b, v3, v1), callStack(a, v1)}, callStack(a, v1)},
- // [A -> V1 -> V2] ==> nil since the only candidate call stack goes through V1
- {vuln2, []vulncheck.CallStack{callStack(a, v1, v2)}, nil},
- // [A -> V1 -> V3, A -> B -> v3] ==> A -> B -> V3 since the first stack goes through V1
- {vuln3, []vulncheck.CallStack{callStack(a, v1, v3), callStack(a, b, v3)}, callStack(a, b, v3)},
- } {
- t.Run(test.vuln.Symbol, func(t *testing.T) {
- got := uniqueCallStack(test.vuln, test.css, skip)
- if diff := cmp.Diff(test.want, got); diff != "" {
- t.Fatalf("mismatch (-want, +got):\n%s", diff)
- }
- })
- }
-}
-
func TestSummarizeCallStack(t *testing.T) {
for _, test := range []struct {
in, want string
diff --git a/internal/vulncheck/witness_test.go b/internal/vulncheck/witness_test.go
index 8f8010b..6636c70 100644
--- a/internal/vulncheck/witness_test.go
+++ b/internal/vulncheck/witness_test.go
@@ -8,6 +8,10 @@
"reflect"
"strings"
"testing"
+
+ "github.com/google/go-cmp/cmp"
+ "golang.org/x/tools/go/packages"
+ "golang.org/x/vuln/internal/osv"
)
// stacksToString converts map *Vuln:stacks to Vuln.Symbol:["f1->...->fN", ...]
@@ -37,22 +41,25 @@
// | interm2(interface)
// | / |
// vuln1 vuln2
+ o := &osv.Entry{ID: "o"}
e1 := &FuncNode{Name: "entry1"}
e2 := &FuncNode{Name: "entry2"}
i1 := &FuncNode{Name: "interm1", CallSites: []*CallSite{{Parent: e1, Resolved: true}}}
i2 := &FuncNode{Name: "interm2", CallSites: []*CallSite{{Parent: e2, Resolved: true}, {Parent: i1, Resolved: true}}}
v1 := &FuncNode{Name: "vuln1", CallSites: []*CallSite{{Parent: i1, Resolved: true}, {Parent: i2, Resolved: false}}}
v2 := &FuncNode{Name: "vuln2", CallSites: []*CallSite{{Parent: i2, Resolved: false}}}
- vuln1 := &Vuln{CallSink: v1, Symbol: "vuln1"}
- vuln2 := &Vuln{CallSink: v2, Symbol: "vuln2"}
+
+ vp := &packages.Package{PkgPath: "v1", Module: &packages.Module{Path: "m1"}}
+ vuln1 := &Vuln{CallSink: v1, ImportSink: vp, OSV: o, Symbol: "vuln1"}
+ vuln2 := &Vuln{CallSink: v2, ImportSink: vp, OSV: o, Symbol: "vuln2"}
res := &Result{
EntryFunctions: []*FuncNode{e1, e2},
Vulns: []*Vuln{vuln1, vuln2},
}
want := map[string][]string{
- "vuln1": {"entry1->interm1->vuln1", "entry2->interm2->vuln1"},
- "vuln2": {"entry2->interm2->vuln2", "entry1->interm1->interm2->vuln2"},
+ "vuln1": {"entry1->interm1->vuln1"},
+ "vuln2": {"entry2->interm2->vuln2"},
}
stacks := CallStacks(res)
@@ -60,3 +67,45 @@
t.Errorf("want %v; got %v", want, got)
}
}
+
+func TestUniqueCallStack(t *testing.T) {
+ a := &FuncNode{Name: "A"}
+ b := &FuncNode{Name: "B"}
+ v1 := &FuncNode{Name: "V1"}
+ v2 := &FuncNode{Name: "V2"}
+ v3 := &FuncNode{Name: "V3"}
+
+ vuln1 := &Vuln{Symbol: "V1", CallSink: v1}
+ vuln2 := &Vuln{Symbol: "V2", CallSink: v2}
+ vuln3 := &Vuln{Symbol: "V3", CallSink: v3}
+
+ callStack := func(fs ...*FuncNode) CallStack {
+ var cs CallStack
+ for _, f := range fs {
+ cs = append(cs, StackEntry{Function: f})
+ }
+ return cs
+ }
+
+ // V1, V2, and V3 are vulnerable symbols
+ skip := []*Vuln{vuln1, vuln2, vuln3}
+ for _, test := range []struct {
+ vuln *Vuln
+ css []CallStack
+ want CallStack
+ }{
+ // [A -> B -> V3 -> V1, A -> V1] ==> A -> V1 since the first stack goes through V3
+ {vuln1, []CallStack{callStack(a, b, v3, v1), callStack(a, v1)}, callStack(a, v1)},
+ // [A -> V1 -> V2] ==> nil since the only candidate call stack goes through V1
+ {vuln2, []CallStack{callStack(a, v1, v2)}, nil},
+ // [A -> V1 -> V3, A -> B -> v3] ==> A -> B -> V3 since the first stack goes through V1
+ {vuln3, []CallStack{callStack(a, v1, v3), callStack(a, b, v3)}, callStack(a, b, v3)},
+ } {
+ t.Run(test.vuln.Symbol, func(t *testing.T) {
+ got := uniqueCallStack(test.vuln, test.css, skip)
+ if diff := cmp.Diff(test.want, got); diff != "" {
+ t.Fatalf("mismatch (-want, +got):\n%s", diff)
+ }
+ })
+ }
+}
To view, visit change 504716. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages