Repository: google/go-flow-levee
Pull request #49 (2020-08-17 14:59:58)

Use fieldtags analyzer in fieldpropagator analyzer
## This PR

This PR enhances the `fieldpropagator` analyzer's ability to detect field propagators by allowing it to use the results of the `fieldtags` analyzer. (The `fieldtags` analyzer was unused up to now.)

## Future PRs (in order)
1. Make this configurable, with the current setting of `levee:"source"` as a default.
2. Use the `fieldtags` analyzer in the `source` analyzer.

## Outstanding
* Should we mark a tagged field as a Source even if it is not on a Source type?
  - I would say no.
  - Currently, we do. This would be easy to change, however.
* Should we produce a warning when we detect a field tagged as a Source that is not on a Source type?
  - I would say yes, e.g. "warning: field tagged as source on non-source type; this field will not be used as a source"

- [x] Tests pass
- [x] Appropriate changes to README are included in PR


Patch:
diff --git a/.travis.yml b/.travis.yml
index b38b8f4..9cfc008 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -10,9 +10,9 @@ script:
 - go test -v ./...
 - >
   echo "Checking goimports..." && (
-    if [[ -n $(goimports -l internal cmd) ]]; then
-      goimports -d internal cmd
-      echo 'Please run `goimports -w cmd internal`.'
+    if [[ -n $(goimports -l cmd internal pkg) ]]; then
+      goimports -d cmd internal pkg
+      echo 'Please run `goimports -w cmd internal pkg`.'
       false
     fi
   )
@@ -27,7 +27,7 @@ script:
   )
 - >
   echo "Checking for license headers..." && (
-    FILES=$(grep -rL 'https://www.apache.org/licenses/LICENSE-2.0' internal cmd | grep -v ".json")
+    FILES=$(grep -rL 'https://www.apache.org/licenses/LICENSE-2.0' cmd internal pkg | grep -v ".json")
     if [[ -n ${FILES} ]]; then
       echo "The following files are missing the Apache header:"
       echo ${FILES}
diff --git a/README.md b/README.md
index 823dec7..7243c0f 100644
--- a/README.md
+++ b/README.md
@@ -50,9 +50,9 @@ if *debugging {
 In the `dot` output:
 * A **red** edge points to a **Referrer** of an `ssa.Node`
 * An **orange** edges points to an **Operand** of an `ssa.Node`
-* **Diamond**-shaped nodes represent `ssa.Node`s that are both `ssa.Instruction`s and `ssa.Value`s
-* **Square**-shaped node represent `ssa.Node`s that are only `ssa.Instruction`s
-* **Ellipse**-shaped nodes that are either only `ssa.Value`s, or are `ssa.Member`s.
+* **Rectangle**-shaped nodes represent `ssa.Node`s that are both `ssa.Instruction`s and `ssa.Value`s
+* **Diamond**-shaped nodes represent `ssa.Node`s that are only `ssa.Instruction`s
+* **Ellipse**-shaped nodes represent `ssa.Node`s that are either only `ssa.Value`s, or are `ssa.Member`s.
 
 ## Source Code Headers
 
diff --git a/configuration/README.md b/configuration/README.md
index c8383e3..98fa684 100644
--- a/configuration/README.md
+++ b/configuration/README.md
@@ -28,7 +28,7 @@ Sources are identified via regexp according to package, type, and field names.
 }
 ```
 
-Sinks, sanitizers, and some propagators (below) are identified via regexp according to package, method, and (optional) receiver name.
+Sinks and sanitizers are identified via regexp according to package, method, and (optional) receiver name.
 
 ```json
 {
@@ -49,33 +49,7 @@ Sinks, sanitizers, and some propagators (below) are identified via regexp accord
 }
 ```
 
-We separate propagators into the following types:
-
-A *transforming propagator* produces a tainted value which may contain source data.
-For instance, a struct's `String()` method or a serializable's `Marshal()` method are transforming propagators.
-The value returned by either can contain source data without being identifiable as the specified source's type.
-Transforming propagators are identified via regexp according to package and function name.
-
-An *argument propagator* is a propagator which taints an input argument rather than a return value.
-This is done, e.g., by buffer writers such as `fmt.Fprintf`, though could conceivably apply to any function that takes a source and any other reference argument.
-At time of writing, argument propagators only applies to the first argument of a method, expecting a pattern similar to buffer writers.
-Configuration currently only allows for one argument propagator to be specified.
-Argument propagators are identified via regexp matching the fully-qualified type name.
-
-```json
-{
-  "TransformingPropagators": [
-    {
-      "PackageRE": "<package path regexp>",
-      "ReceiverRE": "<type name regexp>",
-      "MethodRE": "<method name regexp>"
-    }
-  ],
-  "PropagatorArgs": {
-    "ArgumentTypeRE": "^io\\.(?:Writer|ReadWriter|WriteCloser|ReadWriteCloser)$"
-  }
-}
-```
+Taint propagation is performed automatically and does not need to be explicitly configured.
 
 For matchers that accept a `ReceiverRE` regexp matcher, an unspecified string will match any (or no) receiver.
 To match only methods without any receiver (i.e., a top-level function), use the matcher `^$` to match an empty-string receiver name.
diff --git a/configuration/example-config.json b/configuration/example-config.json
index 5a2b883..4a86565 100644
--- a/configuration/example-config.json
+++ b/configuration/example-config.json
@@ -29,14 +29,4 @@
     }
   ],
   "Sanitizers": [],
-  "TransformingPropagators": [
-    {
-      "PackageName": ".*proto$",
-      "ReceiverRE": "",
-      "MethodRE": "^Clone$|^Marshal"
-    }
-  ],
-  "PropagatorArgs": {
-    "ArgumentTypeRE": "^io\\.(?:Writer|ReadWriter|WriteCloser|ReadWriteCloser)$"
-  }
 }
diff --git a/internal/pkg/call/call.go b/internal/pkg/call/call.go
deleted file mode 100644
index df01c92..0000000
--- a/internal/pkg/call/call.go
+++ /dev/null
@@ -1,55 +0,0 @@
-// 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 call contains functionality for determining whether a function
-// call refers to a given referrer.
-package call
-
-import (
-	"golang.org/x/tools/go/ssa"
-)
-
-// A Call is a wrapper for *ssa.Call instructions, allowing Variadic and
-// non-Variadic calls to be handled polymorphically
-type Call interface {
-	// ReferredBy determines if the call is referred to by the referrer
-	ReferredBy(r Referrer) bool
-}
-
-// A Referrer is something that can be queried to determine whether it refers to
-// a given ssa Node.
-type Referrer interface {
-	RefersTo(node ssa.Node) bool
-}
-
-// A RegularCall is a wrapper around a non-variadic ssa Call.
-type RegularCall struct {
-	*ssa.Call
-}
-
-// Regular creates a RegularCall from an ssa Call.
-func Regular(c *ssa.Call) *RegularCall {
-	return &RegularCall{c}
-}
-
-// ReferredBy determines if the supplied referrer refers one of the Call's arguments.
-func (c *RegularCall) ReferredBy(r Referrer) bool {
-	for _, a := range c.Call.Call.Args {
-		if r.RefersTo(a.(ssa.Node)) {
-			return true
-		}
-	}
-
-	return false
-}
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index 7be108b..6d3851a 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -37,40 +37,9 @@ func init() {
 
 // config contains matchers and analysis scope information
 type Config struct {
-	Sources                 []sourceMatcher
-	Sinks                   []callMatcher
-	Sanitizers              []callMatcher
-	TransformingPropagators []transformingPropagatorMatcher
-	PropagatorArgs          argumentPropagatorMatcher
-	Allowlist               []packageMatcher
-	AnalysisScope           []packageMatcher
-}
-
-// shouldSkip returns true for any function that is outside analysis scope,
-// that is allowlisted,
-// whose containing package imports "testing"
-// or whose containing package does not import any package containing a source or a sink.
-func (c Config) shouldSkip(pkg *types.Package) bool {
-	if isTestPkg(pkg) || !c.isInScope(pkg) || c.isAllowlisted(pkg) {
-		return true
-	}
-
-	// TODO Does this skip packages that own sources/sinks but don't import others?
-	for _, im := range pkg.Imports() {
-		for _, s := range c.Sinks {
-			if s.matchPackage(im) {
-				return false
-			}
-		}
-
-		for _, s := range c.Sources {
-			if s.PackageRE.MatchString(im.Path()) {
-				return false
-			}
-		}
-	}
-
-	return true
+	Sources    []sourceMatcher
+	Sinks      []callMatcher
+	Sanitizers []callMatcher
 }
 
 func (c Config) IsSink(call *ssa.Call) bool {
@@ -150,54 +119,6 @@ func (c Config) IsSourceFieldAddr(fa *ssa.FieldAddr) bool {
 	return false
 }
 
-func (c Config) IsPropagator(call *ssa.Call) bool {
-	return c.isTransformingPropagator(call)
-}
-
-// A call is a transforming propagator if its name matches a pattern in the config
-// and at least one of its arguments is a Source.
-func (c Config) isTransformingPropagator(call *ssa.Call) bool {
-	for _, p := range c.TransformingPropagators {
-		if !p.match(call) {
-			continue
-		}
-
-		for _, a := range call.Call.Args {
-			// TODO Handle ChangeInterface case.
-			switch t := a.(type) {
-			case *ssa.MakeInterface:
-				if c.IsSource(utils.Dereference(t.X.Type())) {
-					return true
-				}
-			case *ssa.Parameter:
-				if c.IsSource(utils.Dereference(t.Type())) {
-					return true
-				}
-			}
-		}
-	}
-
-	return false
-}
-
-func (c Config) isAllowlisted(pkg *types.Package) bool {
-	for _, w := range c.Allowlist {
-		if w.match(pkg) {
-			return true
-		}
-	}
-	return false
-}
-
-func (c Config) isInScope(pkg *types.Package) bool {
-	for _, s := range c.AnalysisScope {
-		if s.match(pkg) {
-			return true
-		}
-	}
-	return false
-}
-
 // A sourceMatcher defines what types are or contain sources.
 // Within a given type, specific field access can be specified as the actual source data
 // via the fieldRE.
@@ -216,33 +137,6 @@ func (s sourceMatcher) match(n *types.Named) bool {
 	return s.PackageRE.MatchString(n.Obj().Pkg().Path()) && s.TypeRE.MatchString(n.Obj().Name())
 }
 
-type transformingPropagatorMatcher struct {
-	PackageName string
-	MethodRE    regexp.Regexp
-}
-
-func (t transformingPropagatorMatcher) match(call *ssa.Call) bool {
-	if call.Call.StaticCallee() == nil ||
-		call.Call.StaticCallee().Pkg == nil ||
-		call.Call.StaticCallee().Pkg.Pkg.Path() != t.PackageName {
-		return false
-	}
-
-	return t.MethodRE.MatchString(call.Call.StaticCallee().Name())
-}
-
-type argumentPropagatorMatcher struct {
-	ArgumentTypeRE regexp.Regexp
-}
-
-type packageMatcher struct {
-	PackageNameRE regexp.Regexp
-}
-
-func (pm packageMatcher) match(pkg *types.Package) bool {
-	return pm.PackageNameRE.MatchString(pkg.Path())
-}
-
 type callMatcher struct {
 	PackageRE  regexp.Regexp
 	ReceiverRE regexp.Regexp
@@ -299,12 +193,3 @@ func ReadConfig() (*Config, error) {
 	_ = loadedFromCache
 	return readConfigCached, readConfigCachedErr
 }
-
-func isTestPkg(p *types.Package) bool {
-	for _, im := range p.Imports() {
-		if im.Name() == "testing" {
-			return true
-		}
-	}
-	return false
-}
diff --git a/internal/pkg/debug/render/dot.go b/internal/pkg/debug/render/dot.go
index 33b84f6..ba15a9e 100644
--- a/internal/pkg/debug/render/dot.go
+++ b/internal/pkg/debug/render/dot.go
@@ -102,9 +102,9 @@ func nodeShape(n ssa.Node) string {
 	_, isInstr := n.(ssa.Instruction)
 	switch {
 	case isValue && isInstr:
-		return "diamond"
+		return "rectangle"
 	case isInstr:
-		return "square"
+		return "diamond"
 	default:
 		return "ellipse"
 	}
diff --git a/internal/pkg/fieldpropagator/analyzer.go b/internal/pkg/fieldpropagator/analyzer.go
index 0743841..c5f25d7 100644
--- a/internal/pkg/fieldpropagator/analyzer.go
+++ b/internal/pkg/fieldpropagator/analyzer.go
@@ -21,6 +21,7 @@ import (
 	"reflect"
 
 	"github.com/google/go-flow-levee/internal/pkg/config"
+	"github.com/google/go-flow-levee/internal/pkg/fieldtags"
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/buildssa"
 	"golang.org/x/tools/go/ssa"
@@ -44,12 +45,13 @@ var Analyzer = &analysis.Analyzer{
 A field propagator is a function that returns a source field.`,
 	Flags:      config.FlagSet,
 	Run:        run,
-	Requires:   []*analysis.Analyzer{buildssa.Analyzer},
+	Requires:   []*analysis.Analyzer{buildssa.Analyzer, fieldtags.Analyzer},
 	ResultType: reflect.TypeOf(new(ResultType)).Elem(),
 	FactTypes:  []analysis.Fact{new(isFieldPropagator)},
 }
 
 func run(pass *analysis.Pass) (interface{}, error) {
+	taggedFields := pass.ResultOf[fieldtags.Analyzer].(fieldtags.ResultType)
 	ssaInput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
 
 	conf, err := config.ReadConfig()
@@ -66,7 +68,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		methods := ssaProg.MethodSets.MethodSet(ssaType.Type())
 		for i := 0; i < methods.Len(); i++ {
 			meth := ssaProg.MethodValue(methods.At(i))
-			analyzeBlocks(pass, conf, meth)
+			analyzeBlocks(pass, conf, taggedFields, meth)
 		}
 	}
 
@@ -77,7 +79,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return FieldPropagators(isFieldPropagator), nil
 }
 
-func analyzeBlocks(pass *analysis.Pass, conf *config.Config, meth *ssa.Function) {
+func analyzeBlocks(pass *analysis.Pass, conf *config.Config, tf fieldtags.TaggedFields, meth *ssa.Function) {
 	// Function does not return anything
 	if res := meth.Signature.Results(); res == nil || (*res).Len() == 0 {
 		return
@@ -91,17 +93,17 @@ func analyzeBlocks(pass *analysis.Pass, conf *config.Config, meth *ssa.Function)
 		if !ok {
 			continue
 		}
-		analyzeResults(pass, conf, meth, ret.Results)
+		analyzeResults(pass, conf, tf, meth, ret.Results)
 	}
 }
 
-func analyzeResults(pass *analysis.Pass, conf *config.Config, meth *ssa.Function, results []ssa.Value) {
+func analyzeResults(pass *analysis.Pass, conf *config.Config, tf fieldtags.TaggedFields, meth *ssa.Function, results []ssa.Value) {
 	for _, r := range results {
 		fa, ok := fieldAddr(r)
 		if !ok {
 			continue
 		}
-		if conf.IsSourceFieldAddr(fa) {
+		if conf.IsSourceFieldAddr(fa) || tf.IsSource(fa) {
 			pass.ExportObjectFact(meth.Object(), &isFieldPropagator{})
 		}
 	}
diff --git a/internal/pkg/fieldtags/analyzer.go b/internal/pkg/fieldtags/analyzer.go
index 39b940c..22ff822 100644
--- a/internal/pkg/fieldtags/analyzer.go
+++ b/internal/pkg/fieldtags/analyzer.go
@@ -17,13 +17,18 @@
 package fieldtags
 
 import (
+	"fmt"
 	"go/ast"
+	"go/types"
 	"reflect"
 	"strings"
 
+	"github.com/google/go-flow-levee/internal/pkg/utils"
+
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/go/ssa"
 )
 
 type keyValue struct {
@@ -37,6 +42,17 @@ var patterns sourcePatterns = []keyValue{
 	{"levee", "source"},
 }
 
+// TaggedFields is named type representing a slice of TaggedFields.
+type TaggedFields []TaggedField
+
+// ResultType is a slice of TaggedFields.
+type ResultType = TaggedFields
+
+// A TaggedField contains the necessary information to identify a tagged struct field.
+type TaggedField struct {
+	pkg, typ, field string
+}
+
 var Analyzer = &analysis.Analyzer{
 	Name: "fieldtags",
 	Doc:  "This analyzer identifies Source fields based on their tags. Tags are expected to satisfy the `go vet -structtag` format.",
@@ -44,28 +60,39 @@ var Analyzer = &analysis.Analyzer{
 	Requires: []*analysis.Analyzer{
 		inspect.Analyzer,
 	},
-	ResultType: reflect.TypeOf(new([]*ast.Field)).Elem(),
+	ResultType: reflect.TypeOf(new(ResultType)).Elem(),
 }
 
 func run(pass *analysis.Pass) (interface{}, error) {
 	inspectResult := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
-	var results []*ast.Field
+	var taggedFields TaggedFields
 
 	nodeFilter := []ast.Node{
-		(*ast.Field)(nil),
+		(*ast.TypeSpec)(nil),
 	}
 
 	inspectResult.Preorder(nodeFilter, func(n ast.Node) {
-		field, ok := n.(*ast.Field)
+		t, ok := n.(*ast.TypeSpec)
 		if !ok {
 			return
 		}
-		if patterns.isSource(field) {
-			results = append(results, field)
-			pass.Reportf(field.Pos(), "tagged field")
+		s, ok := t.Type.(*ast.StructType)
+		if !ok || s.Fields == nil {
+			return
+		}
+		for _, f := range (*s).Fields.List {
+			if patterns.isSource(f) && len(f.Names) > 0 {
+				tf := TaggedField{
+					pkg:   pass.Pkg.Name(),
+					typ:   t.Name.Name,
+					field: f.Names[0].Name,
+				}
+				taggedFields = append(taggedFields, tf)
+				pass.Reportf(f.Pos(), "tagged field: %s", tf)
+			}
 		}
 	})
-	return results, nil
+	return taggedFields, nil
 }
 
 func (sp *sourcePatterns) isSource(field *ast.Field) bool {
@@ -119,3 +146,33 @@ func (sp *sourcePatterns) isSource(field *ast.Field) bool {
 
 	return false
 }
+
+// IsSource determines whether a FieldAddr is a source, that is whether it refers to a field previously identified as a source.
+func (t TaggedFields) IsSource(f *ssa.FieldAddr) bool {
+	n, ok := namedType(f)
+	if !ok {
+		return false
+	}
+	fName := utils.FieldName(f)
+	for _, tf := range t {
+		if tf.matches(n.String(), fName) {
+			return true
+		}
+	}
+	return false
+}
+
+func namedType(f *ssa.FieldAddr) (*types.Named, bool) {
+	structType := f.X.Type()
+	deref := utils.Dereference(structType)
+	named, ok := deref.(*types.Named)
+	return named, ok
+}
+
+func (t TaggedField) matches(typeName, fieldName string) bool {
+	return fmt.Sprintf("%s.%s", t.pkg, t.typ) == typeName && t.field == fieldName
+}
+
+func (t TaggedField) String() string {
+	return fmt.Sprintf("%s.%s.%s", t.pkg, t.typ, t.field)
+}
diff --git a/internal/pkg/levee/levee.go b/internal/pkg/levee/levee.go
index d947013..4eb300f 100644
--- a/internal/pkg/levee/levee.go
+++ b/internal/pkg/levee/levee.go
@@ -18,10 +18,8 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/google/go-flow-levee/internal/pkg/call"
 	"github.com/google/go-flow-levee/internal/pkg/config"
 	"github.com/google/go-flow-levee/internal/pkg/fieldpropagator"
-	"github.com/google/go-flow-levee/internal/pkg/varargs"
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/ssa"
 
@@ -63,25 +61,9 @@ func run(pass *analysis.Pass) (interface{}, error) {
 				case fieldPropagators.Contains(v):
 					sources = append(sources, source.New(v, conf))
 
-				case conf.IsPropagator(v):
-					// Handling the case where sources are propagated to io.Writer
-					// (ex. proto.MarshalText(&buf, c)
-					// In such cases, "buf" becomes a source, and not the return value of the propagator.
-					// TODO Do not hard-code logging sinks usecase
-					// TODO  Handle case of os.Stdout and os.Stderr.
-					// TODO  Do not hard-code the position of the argument, instead declaratively
-					//  specify the position of the propagated source.
-					// TODO  Consider turning propagators that take io.Writer into sinks.
-					if a := getArgumentPropagator(conf, v); a != nil {
-						sources = append(sources, source.New(a, conf))
-					} else {
-						sources = append(sources, source.New(v, conf))
-					}
-
 				case conf.IsSink(v):
-					c := makeCall(v)
 					for _, s := range sources {
-						if c.ReferredBy(s) && !s.IsSanitizedAt(v) {
+						if s.HasPathTo(instr.(ssa.Node)) && !s.IsSanitizedAt(v) {
 							report(pass, s, v)
 							break
 						}
@@ -94,31 +76,9 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return nil, nil
 }
 
-func makeCall(c *ssa.Call) call.Call {
-	if sinkVarargs := varargs.New(c); sinkVarargs != nil {
-		return sinkVarargs
-	}
-	return call.Regular(c)
-}
-
 func report(pass *analysis.Pass, source *source.Source, sink ssa.Node) {
 	var b strings.Builder
 	b.WriteString("a source has reached a sink")
 	fmt.Fprintf(&b, ", source: %v", pass.Fset.Position(source.Node().Pos()))
 	pass.Reportf(sink.Pos(), b.String())
 }
-
-func getArgumentPropagator(c *config.Config, call *ssa.Call) ssa.Node {
-	if call.Call.Signature().Params().Len() == 0 {
-		return nil
-	}
-
-	firstArg := call.Call.Signature().Params().At(0)
-	if c.PropagatorArgs.ArgumentTypeRE.MatchString(firstArg.Type().String()) {
-		if a, ok := call.Call.Args[0].(*ssa.MakeInterface); ok {
-			return a.X.(ssa.Node)
-		}
-	}
-
-	return nil
-}
diff --git a/internal/pkg/source/source.go b/internal/pkg/source/source.go
index eae79fa..8a8a93f 100644
--- a/internal/pkg/source/source.go
+++ b/internal/pkg/source/source.go
@@ -29,7 +29,6 @@ import (
 type classifier interface {
 	IsSource(types.Type) bool
 	IsSanitizer(*ssa.Call) bool
-	IsPropagator(*ssa.Call) bool
 	IsSourceFieldAddr(*ssa.FieldAddr) bool
 	IsSinkFunction(fn *ssa.Function) bool
 }
@@ -110,10 +109,19 @@ func (a *Source) visitOperands(operands []*ssa.Value) {
 		if !ok || a.marked[n] {
 			continue
 		}
-		al, ok := (*o).(*ssa.Alloc)
-		if !ok || al.Comment == "slicelit" || a.config.IsSource(al.Type()) {
-			a.dfs(n)
+		// An Alloc represents the allocation of space for a variable. If a Node is an Alloc,
+		// and the thing being allocated is not an array, then either:
+		// a) it is a Source value, in which case it will get its own traversal when sourcesFromBlocks
+		//    finds this Alloc
+		// b) it is not a Source value, in which case we should not visit it.
+		// However, if the Alloc is an array, then that means the source that we are visiting from
+		// is being placed into an array, slice or varags, so we do need to keep visiting.
+		if al, isAlloc := (*o).(*ssa.Alloc); isAlloc {
+			if _, isArray := utils.Dereference(al.Type()).(*types.Array); !isArray {
+				return
+			}
 		}
+		a.dfs(n)
 	}
 }
 
diff --git a/internal/pkg/varargs/varargs.go b/internal/pkg/varargs/varargs.go
deleted file mode 100644
index 5e8e29b..0000000
--- a/internal/pkg/varargs/varargs.go
+++ /dev/null
@@ -1,80 +0,0 @@
-// 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 varargs contains functionality for dealing with variable length arguments.
-package varargs
-
-import (
-	"github.com/google/go-flow-levee/internal/pkg/call"
-	"golang.org/x/tools/go/ssa"
-)
-
-// Varargs represents a variable length argument.
-// Concretely, it abstract over the fact that the varargs internally are represented by an ssa.Slice
-// which contains the underlying values of for vararg members.
-// Since many sink functions (ex. log.Info, fmt.Errorf) take a vararg argument, being able to
-// get the underlying values of the vararg members is important for the analysis.
-type Varargs struct {
-	stores []*ssa.Store
-}
-
-// New constructs varargs. SSA represents varargs as an ssa.Slice.
-func New(s *ssa.Call) *Varargs {
-	if !s.Call.Signature().Variadic() {
-		return nil
-	}
-	if len(s.Call.Args) == 0 {
-		return nil
-	}
-
-	lastArg := s.Call.Args[len(s.Call.Args)-1]
-	sl, ok := lastArg.(*ssa.Slice)
-	if !ok {
-		return nil
-	}
-
-	a, ok := sl.X.(*ssa.Alloc)
-	if !ok || (a.Comment != "varargs" && a.Comment != "slicelit") {
-		return nil
-	}
-
-	var stores []*ssa.Store
-
-	for _, r := range *a.Referrers() {
-		idx, ok := r.(*ssa.IndexAddr)
-		if !ok || idx.Referrers() == nil {
-			continue
-		}
-
-		// IndexAddr and Store instructions are inherently linked together.
-		// IndexAddr returns an address of an element within a Slice, which is followed by
-		// a Store instructions to place a value into the address provided by IndexAddr.
-		stores = append(stores, (*idx.Referrers())[0].(*ssa.Store))
-	}
-
-	return &Varargs{
-		stores: stores,
-	}
-}
-
-// ReferredBy determines if the supplied node refers the Vararg in question via a store instruction.
-func (v *Varargs) ReferredBy(r call.Referrer) bool {
-	for _, s := range v.stores {
-		if r.RefersTo(s.Val.(ssa.Node)) {
-			return true
-		}
-	}
-
-	return false
-}
diff --git a/pkg/levee/levee.go b/pkg/levee/levee.go
index 77d0d4e..84690f3 100644
--- a/pkg/levee/levee.go
+++ b/pkg/levee/levee.go
@@ -1,3 +1,17 @@
+// Copyright 2019 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 levee exports the levee Analyzer.
 package levee
 


Test patch:
diff --git a/internal/pkg/call/call_test.go b/internal/pkg/call/call_test.go
deleted file mode 100644
index 00d9e9d..0000000
--- a/internal/pkg/call/call_test.go
+++ /dev/null
@@ -1,113 +0,0 @@
-// 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 call
-
-import (
-	"reflect"
-	"testing"
-
-	"golang.org/x/tools/go/analysis"
-	"golang.org/x/tools/go/analysis/analysistest"
-	"golang.org/x/tools/go/analysis/passes/buildssa"
-	"golang.org/x/tools/go/ssa"
-)
-
-func TestRegularCallReferredBy(t *testing.T) {
-	cases := []struct {
-		desc    string
-		pattern string
-		r       Referrer
-		want    bool
-	}{
-		{
-			"call without args",
-			"noarg",
-			fakeReferrer{true},
-			false,
-		},
-		{
-			"call with one arg that is referred to by a referrer",
-			"onearg",
-			fakeReferrer{true},
-			true,
-		},
-		{
-			"call with one arg that isn't referred to",
-			"onearg",
-			fakeReferrer{false},
-			false,
-		},
-	}
-	for _, tt := range cases {
-		t.Run(tt.desc, func(t *testing.T) {
-			dir := analysistest.TestData()
-			r := analysistest.Run(t, dir, testAnalyzer, tt.pattern)
-			call := Regular(getCall(t, r))
-			got := call.ReferredBy(tt.r)
-			if got != tt.want {
-				t.Errorf("call.ReferredBy(%v) == %v, want %v", call, got, tt.want)
-			}
-		})
-	}
-}
-
-func getCall(t *testing.T, r []*analysistest.Result) *ssa.Call {
-	t.Helper()
-	if len(r) != 1 {
-		t.Fatalf("Got %d results, want one", len(r))
-	}
-	a, ok := r[0].Result.(analyzerResult)
-	if !ok {
-		t.Fatalf("Got result of type %T, want analyzerResult", a)
-	}
-	if len(a.calls) != 1 {
-		t.Fatalf("Got %d calls, want one", len(a.calls))
-	}
-	return a.calls[0]
-}
-
-type fakeReferrer struct {
-	hasPathTo bool
-}
-
-func (f fakeReferrer) RefersTo(node ssa.Node) bool { return f.hasPathTo }
-
-var testAnalyzer = &analysis.Analyzer{
-	Name:       "call",
-	Run:        run,
-	Doc:        "test harness for call",
-	Requires:   []*analysis.Analyzer{buildssa.Analyzer},
-	ResultType: reflect.TypeOf(analyzerResult{}),
-}
-
-type analyzerResult struct {
-	calls []*ssa.Call
-}
-
-func run(pass *analysis.Pass) (interface{}, error) {
-	in := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
-	var result analyzerResult
-	for _, fn := range in.SrcFuncs {
-		for _, b := range fn.Blocks {
-			for _, i := range b.Instrs {
-				switch v := i.(type) {
-				case *ssa.Call:
-					result.calls = append(result.calls, v)
-				}
-			}
-		}
-	}
-	return result, nil
-}
diff --git a/internal/pkg/call/testdata/src/noarg/test.go b/internal/pkg/call/testdata/src/noarg/test.go
deleted file mode 100644
index 4c3a6e5..0000000
--- a/internal/pkg/call/testdata/src/noarg/test.go
+++ /dev/null
@@ -1,21 +0,0 @@
-// 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 test
-
-func NoArgs() {}
-
-func CallNoArgs() {
-	NoArgs()
-}
diff --git a/internal/pkg/call/testdata/src/onearg/test.go b/internal/pkg/call/testdata/src/onearg/test.go
deleted file mode 100644
index ac662e6..0000000
--- a/internal/pkg/call/testdata/src/onearg/test.go
+++ /dev/null
@@ -1,21 +0,0 @@
-// 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 test
-
-func OneArg(one int) {}
-
-func CallOneArg() {
-	OneArg(0)
-}
diff --git a/internal/pkg/fieldpropagator/testdata/src/source/test.go b/internal/pkg/fieldpropagator/testdata/src/source/test.go
index 931c55a..6d1dbf2 100644
--- a/internal/pkg/fieldpropagator/testdata/src/source/test.go
+++ b/internal/pkg/fieldpropagator/testdata/src/source/test.go
@@ -15,6 +15,7 @@
 package source
 
 type Source struct {
+	secret  string `levee:"source"`
 	data    string
 	dataPtr *string
 	id      int
@@ -24,6 +25,10 @@ func (s Source) ID() int {
 	return s.id
 }
 
+func (s Source) Secret() string { // want Secret:"field propagator identified"
+	return s.secret
+}
+
 func (s Source) Data() string { // want Data:"field propagator identified"
 	return s.data
 }
diff --git a/internal/pkg/fieldtags/testdata/src/tests/test.go b/internal/pkg/fieldtags/testdata/src/tests/test.go
index 7a44e12..3c90bd8 100644
--- a/internal/pkg/fieldtags/testdata/src/tests/test.go
+++ b/internal/pkg/fieldtags/testdata/src/tests/test.go
@@ -15,9 +15,9 @@
 package fieldtags
 
 type Person struct {
-	password           string      `levee:"source"`               // want "tagged field"
-	secret             string      `json:"secret" levee:"source"` // want "tagged field"
-	another            interface{} "levee:\"source\""             // want "tagged field"
+	password           string      `levee:"source"`               // want "tagged field: fieldtags.Person.password"
+	secret             string      `json:"secret" levee:"source"` // want "tagged field: fieldtags.Person.secret"
+	another            interface{} "levee:\"source\""             // want "tagged field: fieldtags.Person.another"
 	name               string      `some_key:"non_secret"`
 	someNotTaggedField int
 }
diff --git a/internal/pkg/levee/testdata/test-config.json b/internal/pkg/levee/testdata/test-config.json
index 68e7f0c..0664e03 100644
--- a/internal/pkg/levee/testdata/test-config.json
+++ b/internal/pkg/levee/testdata/test-config.json
@@ -17,10 +17,5 @@
       "PackageRE": "^example.com/core$",
       "MethodRE": "^Sanitize"
     }
-  ],
-  "TransformingPropagators": [
-  ],
-  "PropagatorArgs": {},
-  "Allowlist": [],
-  "AnalysisScope": []
+  ]
 }
diff --git a/internal/pkg/source/source_test.go b/internal/pkg/source/source_test.go
index 57d7acf..38e250d 100644
--- a/internal/pkg/source/source_test.go
+++ b/internal/pkg/source/source_test.go
@@ -28,11 +28,10 @@ import (
 )
 
 type testConfig struct {
-	sourcePattern      string
-	propagatorsPattern string
-	fieldsPattern      string
-	sanitizerPattern   string
-	sinkPattern        string
+	sourcePattern    string
+	fieldsPattern    string
+	sanitizerPattern string
+	sinkPattern      string
 }
 
 func (c *testConfig) IsSource(t types.Type) bool {
@@ -51,11 +50,6 @@ func (c *testConfig) IsSanitizer(call *ssa.Call) bool {
 	return match
 }
 
-func (c *testConfig) IsPropagator(call *ssa.Call) bool {
-	match, _ := regexp.MatchString(c.propagatorsPattern, call.String())
-	return match
-}
-
 func (c *testConfig) IsSourceFieldAddr(field *ssa.FieldAddr) bool {
 	match, _ := regexp.MatchString(c.fieldsPattern, utils.FieldName(field))
 	return match
@@ -76,11 +70,10 @@ var testAnalyzer = &analysis.Analyzer{
 func runTest(pass *analysis.Pass) (interface{}, error) {
 	in := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
 	config := &testConfig{
-		sourcePattern:      `\.foo`,
-		propagatorsPattern: "propagator",
-		sanitizerPattern:   "sanitizer",
-		fieldsPattern:      "name",
-		sinkPattern:        "sink",
+		sourcePattern:    `\.foo`,
+		sanitizerPattern: "sanitizer",
+		fieldsPattern:    "name",
+		sinkPattern:      "sink",
 	}
 
 	sm := identify(config, in)
diff --git a/internal/pkg/varargs/testdata/src/base/test.go b/internal/pkg/varargs/testdata/src/base/test.go
deleted file mode 100644
index 24c8ffd..0000000
--- a/internal/pkg/varargs/testdata/src/base/test.go
+++ /dev/null
@@ -1,24 +0,0 @@
-// 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 base
-
-import "fmt"
-
-type source struct{}
-
-func f1() {
-	s := &source{}
-	fmt.Printf("%v", s)
-}
diff --git a/internal/pkg/varargs/testdata/src/empty/test.go b/internal/pkg/varargs/testdata/src/empty/test.go
deleted file mode 100644
index 3b68f95..0000000
--- a/internal/pkg/varargs/testdata/src/empty/test.go
+++ /dev/null
@@ -1,21 +0,0 @@
-// 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 empty
-
-import "fmt"
-
-func f1() {
-	fmt.Print()
-}
diff --git a/internal/pkg/varargs/testdata/src/multiple/test.go b/internal/pkg/varargs/testdata/src/multiple/test.go
deleted file mode 100644
index 936f40a..0000000
--- a/internal/pkg/varargs/testdata/src/multiple/test.go
+++ /dev/null
@@ -1,24 +0,0 @@
-// 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 multiple
-
-import "fmt"
-
-type source struct{}
-
-func f1() {
-	s := &source{}
-	fmt.Printf("%v, %v", s, s)
-}
diff --git a/internal/pkg/varargs/testdata/src/no-connection-to-source/test.go b/internal/pkg/varargs/testdata/src/no-connection-to-source/test.go
deleted file mode 100644
index 4aaa143..0000000
--- a/internal/pkg/varargs/testdata/src/no-connection-to-source/test.go
+++ /dev/null
@@ -1,30 +0,0 @@
-// 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 (
-	"fmt"
-	"reflect"
-)
-
-type source struct{}
-
-func f1() {
-	s := &source{}
-	// Touching "s" via reflect to keep the compiler happy.
-	reflect.TypeOf(s)
-
-	fmt.Printf("%v", "hello")
-}
diff --git a/internal/pkg/varargs/varargs_test.go b/internal/pkg/varargs/varargs_test.go
deleted file mode 100644
index 042eb31..0000000
--- a/internal/pkg/varargs/varargs_test.go
+++ /dev/null
@@ -1,159 +0,0 @@
-// 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 varargs
-
-import (
-	"go/types"
-	"reflect"
-	"testing"
-
-	"github.com/google/go-flow-levee/internal/pkg/source"
-
-	"golang.org/x/tools/go/analysis"
-	"golang.org/x/tools/go/analysis/analysistest"
-	"golang.org/x/tools/go/analysis/passes/buildssa"
-	"golang.org/x/tools/go/ssa"
-)
-
-type testConfig struct {
-	propagatorsPattern string
-	fieldsPattern      string
-	sanitizerPattern   string
-}
-
-func (c *testConfig) IsSource(t types.Type) bool {
-	return true
-}
-
-func (c *testConfig) IsSanitizer(call *ssa.Call) bool {
-	return false
-}
-
-func (c *testConfig) IsPropagator(call *ssa.Call) bool {
-	return false
-}
-
-func (c *testConfig) IsSourceFieldAddr(field *ssa.FieldAddr) bool {
-	return false
-}
-
-func (c *testConfig) IsSinkFunction(f *ssa.Function) bool {
-	return false
-}
-
-type analyzerResult struct {
-	allocations []*ssa.Alloc
-	calls       []*ssa.Call
-	store       []*ssa.Store
-}
-
-var testAnalyzer = &analysis.Analyzer{
-	Name:       "varargs",
-	Run:        run,
-	Doc:        "test harness for varargs",
-	Requires:   []*analysis.Analyzer{buildssa.Analyzer},
-	ResultType: reflect.TypeOf(analyzerResult{}),
-}
-
-func run(pass *analysis.Pass) (interface{}, error) {
-	in := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
-	var result analyzerResult
-	for _, fn := range in.SrcFuncs {
-		for _, b := range fn.Blocks {
-			for _, i := range b.Instrs {
-				switch v := i.(type) {
-				case *ssa.Alloc:
-					result.allocations = append(result.allocations, v)
-				case *ssa.Call:
-					result.calls = append(result.calls, v)
-				case *ssa.Store:
-					result.store = append(result.store, v)
-				}
-			}
-		}
-	}
-
-	return result, nil
-}
-
-func TestVarargs(t *testing.T) {
-	var testCases = []struct {
-		pattern               string
-		expectVararg          bool
-		callUnderTestIdx      int
-		allocUnderTestIdx     int
-		wantConnectionToAlloc bool
-		wantStores            int
-	}{
-		{
-			pattern:               "base",
-			wantConnectionToAlloc: true,
-			wantStores:            1,
-		},
-		{
-			pattern:               "empty",
-			wantConnectionToAlloc: true,
-			wantStores:            1,
-		},
-		{
-			pattern:               "multiple",
-			wantConnectionToAlloc: true,
-			wantStores:            2,
-		},
-		{
-			pattern:    "no-connection-to-source",
-			wantStores: 1,
-		},
-	}
-
-	for _, tt := range testCases {
-		t.Run(tt.pattern, func(t *testing.T) {
-			dir := analysistest.TestData()
-			r := analysistest.Run(t, dir, testAnalyzer, tt.pattern)
-			if len(r) != 1 {
-				t.Fatalf("Got %d results, wanted one", len(r))
-			}
-
-			a, ok := r[0].Result.(analyzerResult)
-			if !ok {
-				t.Fatalf("Got result of type %T, wanted analyzerResult", a)
-			}
-
-			got := New(a.calls[tt.callUnderTestIdx])
-			if got == nil && tt.expectVararg {
-				t.Fatal("Got nil wanted varargs")
-			}
-
-			if got == nil {
-				return
-			}
-
-			for i := 0; i < tt.wantStores; i++ {
-				if got.stores[i] != a.store[i] {
-					t.Fatalf("Expected %v == %v for store #%d. Referrers: %v", got.stores[i], a.store[i], i, got.stores[i].Referrers())
-				}
-			}
-
-			s := source.New(a.allocations[tt.allocUnderTestIdx], &testConfig{})
-			if s == nil {
-				t.Fatal("Expected a source got nil at a.allocations[0]")
-			}
-
-			if !got.ReferredBy(s) && tt.wantConnectionToAlloc {
-				t.Fatalf("Expected source %v to refer to vararg %v", s, got)
-			}
-		})
-	}
-}