From 4f5e697b0d891243a2fa0d9805e7e5ac1db12a07 Mon Sep 17 00:00:00 2001 From: Peter Date: Fri, 29 Jul 2022 23:31:31 -0400 Subject: [PATCH] Declare result types for analyzers. This change declares each of the analyzers in the analyzers package to have a result type of `[]github.com/praetorian-inc/gokart/util.Finding`. A test has also been included that runs a minimal `*analysis.Pass` through each analyzer's `Run` function, reflectively determines the type of the result value, and compares it to the declared result type of the analyzer. Resolves #76. --- analyzers/cmdi.go | 9 +++++---- analyzers/generic.go | 9 +++++---- analyzers/result_type_test.go | 26 ++++++++++++++++++++++++++ analyzers/rsa.go | 9 +++++---- analyzers/scan.go | 3 +++ analyzers/sqli.go | 9 +++++---- analyzers/ssrf.go | 9 +++++---- analyzers/traversal.go | 9 +++++---- test/testutil/run.go | 12 ++++++++++++ 9 files changed, 71 insertions(+), 24 deletions(-) create mode 100644 analyzers/result_type_test.go diff --git a/analyzers/cmdi.go b/analyzers/cmdi.go index 7bbb5d9..d23d661 100644 --- a/analyzers/cmdi.go +++ b/analyzers/cmdi.go @@ -25,10 +25,11 @@ import ( // converts all variables to SSA form to construct a call graph and performs // recursive taint analysis to search for input sources of user-controllable data var CommandInjectionAnalyzer = &analysis.Analyzer{ - Name: "command_injection", - Doc: "reports when command injection can occur", - Run: cmdInjectionRun, - Requires: []*analysis.Analyzer{buildssa.Analyzer}, + Name: "command_injection", + Doc: "reports when command injection can occur", + Run: cmdInjectionRun, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, + ResultType: resultType, } // vulnCmdInjectionFuncs() returns a map of command injection functions that may be vulnerable when used with user controlled input diff --git a/analyzers/generic.go b/analyzers/generic.go index d234125..0a76c62 100644 --- a/analyzers/generic.go +++ b/analyzers/generic.go @@ -71,10 +71,11 @@ func LoadGenericAnalyzers() []*analysis.Analyzer { return genericFunctionRun(pass, vulnCalls, analyzerName, message) } analysisRun := analysis.Analyzer{ - Name: analyzerName, - Doc: analyzerDict.Doc, - Run: analyzerFunc, - Requires: []*analysis.Analyzer{buildssa.Analyzer}, + Name: analyzerName, + Doc: analyzerDict.Doc, + Run: analyzerFunc, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, + ResultType: resultType, } analyzers = append(analyzers, &analysisRun) } diff --git a/analyzers/result_type_test.go b/analyzers/result_type_test.go new file mode 100644 index 0000000..c131495 --- /dev/null +++ b/analyzers/result_type_test.go @@ -0,0 +1,26 @@ +package analyzers + +import ( + "reflect" + "testing" + + "github.com/praetorian-inc/gokart/test/testutil" +) + +func TestResultTypes(t *testing.T) { + for _, a := range Analyzers { + t.Run(a.Name, func(t *testing.T) { + in := testutil.MinimalPass(a) + x, _ := a.Run(in) + if got := reflect.TypeOf(x); got != a.ResultType { + t.Errorf( + "x, _ := %v.Run(%v); reflect.TypeOf(x) = %v, want %v", + a.Name, + in, + got, + a.ResultType, + ) + } + }) + } +} diff --git a/analyzers/rsa.go b/analyzers/rsa.go index b00613d..e64b031 100644 --- a/analyzers/rsa.go +++ b/analyzers/rsa.go @@ -31,10 +31,11 @@ import ( // all variables are converted to SSA form and a call graph is constructed // recursive analysis is then used to resolve variables used as a key length to a final constant value at the callsite var RsaKeylenAnalyzer = &analysis.Analyzer{ - Name: "rsa_keylen", - Doc: "reports when rsa keys are too short", - Run: rsaRun, - Requires: []*analysis.Analyzer{buildssa.Analyzer}, + Name: "rsa_keylen", + Doc: "reports when rsa keys are too short", + Run: rsaRun, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, + ResultType: resultType, } const RECOMMENDED_KEYLEN = 2048 diff --git a/analyzers/scan.go b/analyzers/scan.go index 3141f4e..3a4cf45 100644 --- a/analyzers/scan.go +++ b/analyzers/scan.go @@ -25,6 +25,7 @@ import ( "log" "os" "path/filepath" + "reflect" "strings" "time" @@ -33,6 +34,8 @@ import ( "golang.org/x/tools/go/analysis" ) +var resultType = reflect.TypeOf([]util.Finding(nil)) + var Analyzers = []*analysis.Analyzer{ RsaKeylenAnalyzer, PathTraversalAnalyzer, diff --git a/analyzers/sqli.go b/analyzers/sqli.go index 5676377..07d798a 100644 --- a/analyzers/sqli.go +++ b/analyzers/sqli.go @@ -26,10 +26,11 @@ import ( // all variables are converted to SSA form and a call graph is constructed // recursive taint analysis is then used to search from a given Sink up the callgraph for Sources of user-controllable data var SQLInjectionAnalyzer = &analysis.Analyzer{ - Name: "sql_injection", - Doc: "reports when SQL injection can occur", - Run: sqlRun, - Requires: []*analysis.Analyzer{buildssa.Analyzer}, + Name: "sql_injection", + Doc: "reports when SQL injection can occur", + Run: sqlRun, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, + ResultType: resultType, } // grab_vulnerable_sql_functions() creates map of vulnerable functions that the scanner will check diff --git a/analyzers/ssrf.go b/analyzers/ssrf.go index b8ae17d..2f0260e 100644 --- a/analyzers/ssrf.go +++ b/analyzers/ssrf.go @@ -26,10 +26,11 @@ import ( // converts all variables to SSA form to construct a call graph and performs // recursive taint analysis to search for input sources of user-controllable data var SSRFAnalyzer = &analysis.Analyzer{ - Name: "SSRF", - Doc: "reports when SSRF vulnerabilities can occur", - Run: ssrfRun, - Requires: []*analysis.Analyzer{buildssa.Analyzer}, + Name: "SSRF", + Doc: "reports when SSRF vulnerabilities can occur", + Run: ssrfRun, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, + ResultType: resultType, } // vulnerable_ssrf_funcs() returns a map of networking functions that may be vulnerable when used with user controlled input diff --git a/analyzers/traversal.go b/analyzers/traversal.go index 4c95aec..c5d6cce 100644 --- a/analyzers/traversal.go +++ b/analyzers/traversal.go @@ -24,10 +24,11 @@ import ( // all variables are converted to SSA form and a call graph is constructed // recursive taint analysis is then used to search from a given Sink up the callgraph for Sources of user-controllable data var PathTraversalAnalyzer = &analysis.Analyzer{ - Name: "path_traversal", - Doc: "reports when path traversal can occur", - Run: traversalRun, - Requires: []*analysis.Analyzer{buildssa.Analyzer}, + Name: "path_traversal", + Doc: "reports when path traversal can occur", + Run: traversalRun, + Requires: []*analysis.Analyzer{buildssa.Analyzer}, + ResultType: resultType, } // getVulnerableInjectionFunctions() returns a map of functions that may be vulnerable to path traversal when used with user controlled input diff --git a/test/testutil/run.go b/test/testutil/run.go index 971e5d2..ccaaf4d 100644 --- a/test/testutil/run.go +++ b/test/testutil/run.go @@ -15,14 +15,26 @@ package testutil import ( + "go/types" "path/filepath" "runtime" "testing" "github.com/praetorian-inc/gokart/run" "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/buildssa" ) +func MinimalPass(a *analysis.Analyzer) *analysis.Pass { + return &analysis.Pass{ + Analyzer: a, + ResultOf: map[*analysis.Analyzer]interface{}{ + buildssa.Analyzer: new(buildssa.SSA), + }, + Pkg: types.NewPackage("./foo", "bar"), + } +} + func RunTest(file string, numResults int, resultsType string, analyzer *analysis.Analyzer, t *testing.T) { _, b, _, _ := runtime.Caller(0) basepath := filepath.Dir(b)