Repository: google/go-flow-levee
Pull request #17 (2020-05-14 21:41:22)

Extract source identification as a separate analyzer
This requires #13.  The first two commits belong to that PR.




Patch:
diff --git a/internal/levee.go b/internal/levee.go
index 632fe12..b665bbd 100644
--- a/internal/levee.go
+++ b/internal/levee.go
@@ -16,16 +16,13 @@ package internal
 
 import (
 	"fmt"
-	"go/types"
 	"strings"
 
 	"github.com/google/go-flow-levee/internal/pkg/config"
 	"golang.org/x/tools/go/analysis"
-	"golang.org/x/tools/go/analysis/passes/buildssa"
 	"golang.org/x/tools/go/ssa"
 
 	"github.com/google/go-flow-levee/internal/pkg/source"
-	"github.com/google/go-flow-levee/internal/pkg/utils"
 )
 
 var Analyzer = &analysis.Analyzer{
@@ -33,7 +30,7 @@ var Analyzer = &analysis.Analyzer{
 	Run:      run,
 	Flags:    config.FlagSet,
 	Doc:      "reports attempts to source data to sinks",
-	Requires: []*analysis.Analyzer{buildssa.Analyzer},
+	Requires: []*analysis.Analyzer{source.Analyzer},
 }
 
 // varargs represents a variable length argument.
@@ -111,9 +108,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	}
 	// TODO: respect configuration scope
 
-	ssaInput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
-
-	sourcesMap := identifySources(conf, ssaInput)
+	sourcesMap := pass.ResultOf[source.Analyzer].(source.ResultType)
 
 	// Only examine functions that have sources
 	for fn, sources := range sourcesMap {
@@ -169,80 +164,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return nil, nil
 }
 
-func identifySources(conf *config.Config, ssaInput *buildssa.SSA) map[*ssa.Function][]*source.Source {
-	sourceMap := make(map[*ssa.Function][]*source.Source)
-
-	for _, fn := range ssaInput.SrcFuncs {
-		var sources []*source.Source
-		sources = append(sources, sourcesFromParams(fn, conf)...)
-		sources = append(sources, sourcesFromClosure(fn, conf)...)
-		sources = append(sources, sourcesFromBlocks(fn, conf)...)
-
-		if len(sources) > 0 {
-			sourceMap[fn] = sources
-		}
-	}
-	return sourceMap
-}
-
-func sourcesFromParams(fn *ssa.Function, conf *config.Config) []*source.Source {
-	var sources []*source.Source
-	for _, p := range fn.Params {
-		switch t := p.Type().(type) {
-		case *types.Pointer:
-			if n, ok := t.Elem().(*types.Named); ok && conf.IsSource(n) {
-				sources = append(sources, source.New(p, conf))
-			}
-			// TODO Handle the case where sources arepassed by value: func(c sourceType)
-			// TODO Handle cases where PII is wrapped in struct/slice/map
-		}
-	}
-	return sources
-}
-
-func sourcesFromClosure(fn *ssa.Function, conf *config.Config) []*source.Source {
-	var sources []*source.Source
-	for _, p := range fn.FreeVars {
-		switch t := p.Type().(type) {
-		case *types.Pointer:
-			// FreeVars (variables from a closure) appear as double-pointers
-			// Hence, the need to dereference them recursively.
-			if s, ok := utils.Dereference(t).(*types.Named); ok && conf.IsSource(s) {
-				sources = append(sources, source.New(p, conf))
-			}
-		}
-	}
-	return sources
-}
-
-func sourcesFromBlocks(fn *ssa.Function, conf *config.Config) []*source.Source {
-	var sources []*source.Source
-	for _, b := range fn.Blocks {
-		if b == fn.Recover {
-			// TODO Handle calls to log in a recovery block.
-			continue
-		}
-
-		for _, instr := range b.Instrs {
-			switch v := instr.(type) {
-			// Looking for sources of PII allocated within the body of a function.
-			case *ssa.Alloc:
-				if conf.IsSource(utils.Dereference(v.Type())) {
-					sources = append(sources, source.New(v, conf))
-				}
-
-				// Handling the case where PII may be in a receiver
-				// (ex. func(b *something) { log.Info(something.PII) }
-			case *ssa.FieldAddr:
-				if conf.IsSource(utils.Dereference(v.Type())) {
-					sources = append(sources, source.New(v, conf))
-				}
-			}
-		}
-	}
-	return sources
-}
-
 func report(pass *analysis.Pass, source *source.Source, sink ssa.Node) {
 	var b strings.Builder
 	b.WriteString("a source has reached a sink")
diff --git a/internal/pkg/source/analyzer.go b/internal/pkg/source/analyzer.go
new file mode 100644
index 0000000..995cfe7
--- /dev/null
+++ b/internal/pkg/source/analyzer.go
@@ -0,0 +1,59 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package source
+
+import (
+	"reflect"
+
+	"github.com/google/go-flow-levee/internal/pkg/config"
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/buildssa"
+	"golang.org/x/tools/go/ssa"
+)
+
+type ResultType = map[*ssa.Function][]*Source
+
+var Analyzer = &analysis.Analyzer{
+	Name:       "source",
+	Doc:        "This analyzer identifies ssa.Values as dataflow sources.",
+	Flags:      config.FlagSet,
+	Run:        run,
+	Requires:   []*analysis.Analyzer{buildssa.Analyzer},
+	ResultType: reflect.TypeOf(new(ResultType)).Elem(),
+}
+
+// When reporting is true, report findings to pass.Report.
+// TODO This should be a flag passable to the common config.
+var reporting bool
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	ssaInput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
+	conf, err := config.ReadConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	sourceMap := identify(conf, ssaInput)
+
+	if reporting {
+		for _, srcs := range sourceMap {
+			for _, s := range srcs {
+				pass.Reportf(s.node.Pos(), "source identified")
+			}
+		}
+	}
+
+	return sourceMap, nil
+}
diff --git a/internal/pkg/source/source.go b/internal/pkg/source/source.go
index e0a876b..f503d7d 100644
--- a/internal/pkg/source/source.go
+++ b/internal/pkg/source/source.go
@@ -17,10 +17,14 @@ package source
 
 import (
 	"fmt"
+	"go/types"
 	"strings"
 
 	"github.com/eapache/queue"
+	"github.com/google/go-flow-levee/internal/pkg/config"
 	"github.com/google/go-flow-levee/internal/pkg/sanitizer"
+	"github.com/google/go-flow-levee/internal/pkg/utils"
+	"golang.org/x/tools/go/analysis/passes/buildssa"
 	"golang.org/x/tools/go/ssa"
 )
 
@@ -136,3 +140,77 @@ func (a *Source) String() string {
 
 	return b.String()
 }
+
+func identify(conf *config.Config, ssaInput *buildssa.SSA) map[*ssa.Function][]*Source {
+	sourceMap := make(map[*ssa.Function][]*Source)
+
+	for _, fn := range ssaInput.SrcFuncs {
+		var sources []*Source
+		sources = append(sources, sourcesFromParams(fn, conf)...)
+		sources = append(sources, sourcesFromClosure(fn, conf)...)
+		sources = append(sources, sourcesFromBlocks(fn, conf)...)
+
+		if len(sources) > 0 {
+			sourceMap[fn] = sources
+		}
+	}
+	return sourceMap
+}
+
+func sourcesFromParams(fn *ssa.Function, conf *config.Config) []*Source {
+	var sources []*Source
+	for _, p := range fn.Params {
+		switch t := p.Type().(type) {
+		case *types.Pointer:
+			if n, ok := t.Elem().(*types.Named); ok && conf.IsSource(n) {
+				sources = append(sources, New(p, conf))
+			}
+			// TODO Handle the case where sources arepassed by value: func(c sourceType)
+			// TODO Handle cases where PII is wrapped in struct/slice/map
+		}
+	}
+	return sources
+}
+
+func sourcesFromClosure(fn *ssa.Function, conf *config.Config) []*Source {
+	var sources []*Source
+	for _, p := range fn.FreeVars {
+		switch t := p.Type().(type) {
+		case *types.Pointer:
+			// FreeVars (variables from a closure) appear as double-pointers
+			// Hence, the need to dereference them recursively.
+			if s, ok := utils.Dereference(t).(*types.Named); ok && conf.IsSource(s) {
+				sources = append(sources, New(p, conf))
+			}
+		}
+	}
+	return sources
+}
+
+func sourcesFromBlocks(fn *ssa.Function, conf *config.Config) []*Source {
+	var sources []*Source
+	for _, b := range fn.Blocks {
+		if b == fn.Recover {
+			// TODO Handle calls to log in a recovery block.
+			continue
+		}
+
+		for _, instr := range b.Instrs {
+			switch v := instr.(type) {
+			// Looking for sources of PII allocated within the body of a function.
+			case *ssa.Alloc:
+				if conf.IsSource(utils.Dereference(v.Type())) {
+					sources = append(sources, New(v, conf))
+				}
+
+				// Handling the case where PII may be in a receiver
+				// (ex. func(b *something) { log.Info(something.PII) }
+			case *ssa.FieldAddr:
+				if conf.IsSource(utils.Dereference(v.Type())) {
+					sources = append(sources, New(v, conf))
+				}
+			}
+		}
+	}
+	return sources
+}


Test patch:
diff --git a/internal/pkg/source/analyzer_test.go b/internal/pkg/source/analyzer_test.go
new file mode 100644
index 0000000..b57becb
--- /dev/null
+++ b/internal/pkg/source/analyzer_test.go
@@ -0,0 +1,33 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package source
+
+import (
+	"testing"
+
+	"github.com/google/go-flow-levee/internal/pkg/config"
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestSourceAnalysis(t *testing.T) {
+	reporting = true
+
+	testdata := analysistest.TestData()
+	if err := config.FlagSet.Set("config", testdata+"/src/analyzertest/test-config.json"); err != nil {
+		t.Error(err)
+	}
+
+	analysistest.Run(t, testdata, Analyzer, "analyzertest/sourcetest")
+}
diff --git a/internal/pkg/source/source_test.go b/internal/pkg/source/source_test.go
index d8b5be3..5a1fed6 100644
--- a/internal/pkg/source/source_test.go
+++ b/internal/pkg/source/source_test.go
@@ -57,13 +57,13 @@ type analyzerResult struct {
 
 var testAnalyzer = &analysis.Analyzer{
 	Name:       "source",
-	Run:        run,
+	Run:        runTest,
 	Doc:        "test harness for the logic related to sources",
 	Requires:   []*analysis.Analyzer{buildssa.Analyzer},
 	ResultType: reflect.TypeOf(analyzerResult{}),
 }
 
-func run(pass *analysis.Pass) (interface{}, error) {
+func runTest(pass *analysis.Pass) (interface{}, error) {
 	in := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
 	var result analyzerResult
 	for _, fn := range in.SrcFuncs {
diff --git a/internal/pkg/source/testdata/src/analyzertest/sourcetest/test.go b/internal/pkg/source/testdata/src/analyzertest/sourcetest/test.go
new file mode 100644
index 0000000..86f70f4
--- /dev/null
+++ b/internal/pkg/source/testdata/src/analyzertest/sourcetest/test.go
@@ -0,0 +1,42 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package sourcetest
+
+// source container
+type Source struct {
+	Data string // source field
+	ID   int    // public
+}
+
+// This function allows us to consume multiple arguments in a single line so this file can compile
+func noop(args ...interface{}) {}
+
+func TestSourceDeclarations() {
+	var varZeroVal Source                         // want "source identified"
+	declZeroVal := Source{}                       // want "source identified"
+	populatedVal := Source{Data: "secret", ID: 0} // want "source identified"
+
+	var ptr *Source                                       // TODO want "source identified"
+	newPtr := new(Source)                                 // want "source identified"
+	ptrToDeclZero := &Source{}                            // want "source identified"
+	ptrToDeclPopulataed := &Source{Data: "secret", ID: 1} // want "source identified"
+
+	noop(varZeroVal, declZeroVal, populatedVal, ptr, newPtr, ptrToDeclZero, ptrToDeclPopulataed)
+}
+
+// A report should be emitted for each parameter.
+func TestSourceParameters(val Source, ptr *Source) { // want "source identified" "source identified"
+
+}
diff --git a/internal/pkg/source/testdata/src/analyzertest/test-config.json b/internal/pkg/source/testdata/src/analyzertest/test-config.json
new file mode 100644
index 0000000..9de249b
--- /dev/null
+++ b/internal/pkg/source/testdata/src/analyzertest/test-config.json
@@ -0,0 +1,9 @@
+{
+  "Sources": [
+    {
+      "PackageRE": "",
+      "TypeRE": "^Source$",
+      "FieldRE": "^Data"
+    }
+  ]
+}
\ No newline at end of file