Repository: google/go-flow-levee
Pull request #138 (2020-10-05 18:12:14)

Introduce issue template for false positive reports
In relation to #94. I intend to add something to the README with a link that points to a new issue using this template.


Patch:
diff --git a/.github/ISSUE_TEMPLATE/false-positive.md b/.github/ISSUE_TEMPLATE/false-positive.md
new file mode 100644
index 0000000..ef1be09
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/false-positive.md
@@ -0,0 +1,28 @@
+---
+name: False positive
+about: Report a false positive.
+title: "[FALSE POSITIVE]: "
+labels: false positive
+assignees: ''
+
+---
+
+## False positive report
+
+Use this issue template to describe a false positive report produced by the analyzer. A false positive report is a report produced by the analyzer on code that you consider to be safe. For example, if the analyzer were to produce a report on the following piece of code, then that would be a false positive:
+
+```go
+func NothingWrongHere() {
+    Sink(Safe{"not a secret"}) // we don't want a report here, since the value that reached the sink is not sensitive
+}
+```
+(We are assuming that `Sink` has been configured as a sink and that `Safe` has *not* been configured as a source.)
+
+**Describe the issue**
+Please include a clear and concise description of what happened and why you think it is a false positive.
+
+**To Reproduce**
+Please make it as easy as possible for us to reproduce what you observed. If possible, provide the exact configuration and code on which the analyzer failed to produce a report. If the code cannot be shared, please provide a simplified example and confirm that it also yields the false positive report.
+
+**Additional context**
+Add any other context about the problem here.
diff --git a/.travis.yml b/.travis.yml
index 178a5ba..68d42ab 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -27,7 +27,7 @@ script:
   )
 - >
   echo "Checking for license headers..." && (
-    FILES=$(grep -rL 'https://www.apache.org/licenses/LICENSE-2.0' cmd internal pkg | grep -v ".json")
+    FILES=$(grep -rL 'https://www.apache.org/licenses/LICENSE-2.0' cmd internal pkg | grep -vE ".(json|ssa)")
     if [[ -n ${FILES} ]]; then
       echo "The following files are missing the Apache header:"
       echo ${FILES}
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index 7062634..ba24abe 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -20,6 +20,8 @@ import (
 	"fmt"
 	"go/types"
 	"io/ioutil"
+	"reflect"
+	"strconv"
 	"strings"
 	"sync"
 
@@ -50,21 +52,25 @@ type fieldTagMatcher struct {
 	Val string
 }
 
-func (ftm fieldTagMatcher) matches(key, val string) bool {
-	return ftm.Key == key && ftm.Val == val
-}
-
 // IsSourceFieldTag determines whether a field tag made up of a key and value
 // is a Source.
-func (c Config) IsSourceFieldTag(key, val string) bool {
+func (c Config) IsSourceFieldTag(tag string) bool {
+	if unq, err := strconv.Unquote(tag); err == nil {
+		tag = unq
+	}
+	st := reflect.StructTag(tag)
+
 	// built in
-	if key == "levee" && val == "source" {
+	if st.Get("levee") == "source" {
 		return true
 	}
 	// configured
 	for _, ft := range c.FieldTags {
-		if ft.matches(key, val) {
-			return true
+		val := st.Get(ft.Key)
+		for _, v := range strings.Split(val, ",") {
+			if v == ft.Val {
+				return true
+			}
 		}
 	}
 	return false
diff --git a/internal/pkg/debug/render/ssa.go b/internal/pkg/debug/render/ssa.go
index f1b4e78..d8454c1 100644
--- a/internal/pkg/debug/render/ssa.go
+++ b/internal/pkg/debug/render/ssa.go
@@ -26,26 +26,38 @@ import (
 // SSA produces a human-readable representation of the SSA code for a function.
 func SSA(f *ssa.Function) string {
 	var b strings.Builder
+
+	b.WriteString("func ")
+	b.WriteString(f.Name())
+	b.WriteString(strings.TrimPrefix(f.Signature.String(), "func"))
+	b.WriteByte('\n')
+
+	maxBlockLength := 1
+	for _, blk := range f.Blocks {
+		length := len(strconv.Itoa(len(blk.Instrs)))
+		if length > maxBlockLength {
+			maxBlockLength = length
+		}
+	}
+
 	for i, blk := range f.Blocks {
-		renderBlock(&b, i, blk)
+		renderBlock(&b, i, blk, maxBlockLength)
 	}
+
 	for _, af := range f.AnonFuncs {
-		b.WriteString(af.Name())
-		b.WriteByte('\n')
-		for i, blk := range af.Blocks {
-			renderBlock(&b, i, blk)
-		}
+		b.WriteString(SSA(af))
 	}
+
 	return b.String()
 }
 
-func renderBlock(b *strings.Builder, i int, blk *ssa.BasicBlock) {
+func renderBlock(b *strings.Builder, i int, blk *ssa.BasicBlock, instructionIndexWidth int) {
 	b.WriteString(fmt.Sprintf("%d: %s\n", i, blk.Comment))
 	for j, instr := range blk.Instrs {
 		s := node.CanonicalName(instr.(ssa.Node))
 		b.WriteByte('\t')
-		b.WriteString(strconv.Itoa(j))
-		b.WriteString(fmt.Sprintf("(%-20T): %-20d", instr, instr.Pos()))
+		b.WriteString(fmt.Sprintf("%"+strconv.Itoa(instructionIndexWidth)+"d", j))
+		b.WriteString(fmt.Sprintf("(%-20T): ", instr))
 		b.WriteString(s)
 		b.WriteByte('\n')
 	}
diff --git a/internal/pkg/fieldtags/analyzer.go b/internal/pkg/fieldtags/analyzer.go
index 77f4a8e..542d93d 100644
--- a/internal/pkg/fieldtags/analyzer.go
+++ b/internal/pkg/fieldtags/analyzer.go
@@ -71,14 +71,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			return
 		}
 		for _, f := range (*s).Fields.List {
-			tags := extractTags(f)
-			isTaggedField := false
-			for _, ft := range tags {
-				if conf.IsSourceFieldTag(ft.key, ft.val) {
-					isTaggedField = true
-				}
-			}
-			if !isTaggedField || len(f.Names) == 0 {
+			if f.Tag == nil || len(f.Names) == 0 || !conf.IsSourceFieldTag(f.Tag.Value) {
 				continue
 			}
 			fNames := make([]string, len(f.Names))
diff --git a/internal/pkg/source/source.go b/internal/pkg/source/source.go
index 291cfb8..531a055 100644
--- a/internal/pkg/source/source.go
+++ b/internal/pkg/source/source.go
@@ -55,6 +55,16 @@ func (s *Source) Pos() token.Pos {
 	if e, ok := s.node.(*ssa.Extract); ok {
 		return e.Tuple.Pos()
 	}
+	// Fields don't *always* have a registered position in the source code,
+	// e.g. when accessing an embedded field.
+	if f, ok := s.node.(*ssa.Field); ok && f.Pos() == token.NoPos {
+		return f.X.Pos()
+	}
+	// FieldAddrs don't *always* have a registered position in the source code,
+	// e.g. when accessing an embedded field.
+	if f, ok := s.node.(*ssa.FieldAddr); ok && f.Pos() == token.NoPos {
+		return f.X.Pos()
+	}
 	return s.node.Pos()
 }
 
@@ -104,10 +114,7 @@ func (s *Source) dfs(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]int, lastBl
 
 	s.visitReferrers(n, mirCopy, lastBlockVisited)
 
-	operands := n.Operands(nil)
-	if operands != nil {
-		s.visitOperands(n, operands, mirCopy, lastBlockVisited)
-	}
+	s.visitOperands(n, n.Operands(nil), mirCopy, lastBlockVisited)
 }
 
 func (s *Source) visitReferrers(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]int, lastBlockVisited *ssa.BasicBlock) {
@@ -314,11 +321,8 @@ func identify(conf classifier, ssaInput *buildssa.SSA) map[*ssa.Function][]*Sour
 func sourcesFromParams(fn *ssa.Function, conf classifier) []*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))
-			}
+		if isSourceType(conf, p.Type()) {
+			sources = append(sources, New(p, conf))
 		}
 	}
 	return sources
@@ -382,10 +386,13 @@ func sourcesFromBlocks(fn *ssa.Function, conf classifier) []*Source {
 
 			// source obtained through a field or an index operation
 			case *ssa.Field, *ssa.FieldAddr, *ssa.IndexAddr, *ssa.Lookup:
+
+			// source chan or map (arrays and slices have regular Allocs)
+			case *ssa.MakeMap, *ssa.MakeChan:
 			}
 
 			// all of the instructions that the switch lets through are values as per ssa/doc.go
-			if v := instr.(ssa.Value); conf.IsSource(utils.Dereference(v.Type())) {
+			if v := instr.(ssa.Value); isSourceType(conf, v.Type()) {
 				sources = append(sources, New(v.(ssa.Node), conf))
 			}
 		}
@@ -393,6 +400,26 @@ func sourcesFromBlocks(fn *ssa.Function, conf classifier) []*Source {
 	return sources
 }
 
+func isSourceType(c classifier, t types.Type) bool {
+	deref := utils.Dereference(t)
+	switch tt := deref.(type) {
+	case *types.Array:
+		return isSourceType(c, tt.Elem())
+	case *types.Slice:
+		return isSourceType(c, tt.Elem())
+	case *types.Chan:
+		return isSourceType(c, tt.Elem())
+	case *types.Map:
+		key := isSourceType(c, tt.Key())
+		elem := isSourceType(c, tt.Elem())
+		return key || elem
+	case *types.Basic, *types.Interface, *types.Tuple, *types.Struct:
+		return false
+	default:
+		return c.IsSource(tt) || isSourceType(c, tt.Underlying())
+	}
+}
+
 func isProducedBySanitizer(v ssa.Value, conf classifier) bool {
 	for _, instr := range *v.Referrers() {
 		store, ok := instr.(*ssa.Store)


Test patch:
diff --git a/internal/pkg/config/fieldtags_test.go b/internal/pkg/config/fieldtags_test.go
index f1b73ad..c112449 100644
--- a/internal/pkg/config/fieldtags_test.go
+++ b/internal/pkg/config/fieldtags_test.go
@@ -30,41 +30,71 @@ func TestFieldTagsIdentification(t *testing.T) {
 
 	cases := []struct {
 		desc string
-		key  string
-		val  string
+		tag  string
 		want bool
 	}{
 		{
 			"built-in field tag",
-			"levee",
-			"source",
+			"`levee:\"source\"`",
 			true,
 		},
 		{
 			"custom field tag",
-			"example",
-			"sensitive",
+			"`example:\"sensitive\"`",
 			true,
 		},
 		{
 			"different tag key",
-			"notexample",
-			"sensitive",
+			"`notexample:\"sensitive\"`",
 			false,
 		},
 		{
 			"different tag value",
-			"example",
-			"notsensitive",
+			"`example:\"notsensitive\"`",
+			false,
+		},
+		{
+			"escaped tag accepted",
+			`"levee:\"source\""`,
+			true,
+		},
+		{
+			"multiple values, no target",
+			"`example:\"foo,bar,baz\"`",
+			false,
+		},
+		{
+			"multiple values, with target",
+			"`example:\"foo,sensitive,bar\"`",
+			true,
+		},
+		{
+			"multiple key value sets, no target",
+			"`foo:\"bar,baz\" example:\"foo,bar,baz\" fizz:\"bang\"`",
+			false,
+		},
+		{
+			"multiple key value sets, with target",
+			"`foo:\"bar,baz\" example:\"foo,sensitive,bar\"` fizz:\"bang\"",
+			true,
+		},
+		{
+			"empty",
+			"",
+			false,
+		},
+		{
+			"malformed",
+			"`noEndQuote:\"malform",
 			false,
 		},
 	}
 
 	for _, tt := range cases {
 		t.Run(tt.desc, func(t *testing.T) {
-			got := config.IsSourceFieldTag(tt.key, tt.val)
+			got := config.IsSourceFieldTag(tt.tag)
 			if got != tt.want {
-				t.Errorf("config.IsSourceFieldTag(%q, %q) == %v, want %v", tt.key, tt.val, got, tt.want)
+				t.Errorf("config.IsSourceFieldTag(%q) == %v, want %v", tt.tag, got, tt.want)
 			}
 		})
 	}
diff --git a/internal/pkg/debug/render/ssa_test.go b/internal/pkg/debug/render/ssa_test.go
new file mode 100644
index 0000000..399b822
--- /dev/null
+++ b/internal/pkg/debug/render/ssa_test.go
@@ -0,0 +1,79 @@
+// 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 render
+
+import (
+	"go/ast"
+	"go/importer"
+	"go/parser"
+	"go/token"
+	"go/types"
+	"io/ioutil"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"golang.org/x/tools/go/ssa"
+	"golang.org/x/tools/go/ssa/ssautil"
+)
+
+func TestSSA(t *testing.T) {
+	testdata, err := filepath.Abs("testdata")
+	if err != nil {
+		t.Fatal(err)
+	}
+	testfile := filepath.Join(testdata, "tests.go")
+	ssaFuncs := extractSSAFuncs(t, testfile)
+	for _, f := range ssaFuncs {
+		bytes, err := ioutil.ReadFile(filepath.Join(testdata, f.Name()) + ".ssa")
+		if err != nil {
+			t.Fatal(err)
+		}
+		want := string(bytes)
+
+		got := SSA(f)
+
+		if diff := cmp.Diff(want, got); diff != "" {
+			t.Errorf("render.SSA diff (-want +got):\n%s", diff)
+		}
+	}
+}
+
+func extractSSAFuncs(t *testing.T, testfile string) []*ssa.Function {
+	t.Helper()
+
+	fset := token.NewFileSet()
+	file, err := parser.ParseFile(fset, testfile, nil, parser.ParseComments)
+	if err != nil {
+		t.Fatal(err)
+	}
+	files := []*ast.File{file}
+
+	pkg := types.NewPackage(file.Name.Name, "")
+	ssaPkg, _, err := ssautil.BuildPackage(
+		&types.Config{Importer: importer.Default()}, fset, pkg, files, ssa.SanityCheckFunctions)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var functions []*ssa.Function
+	for _, m := range ssaPkg.Members {
+		if f, ok := m.(*ssa.Function); ok && !strings.HasPrefix(f.Name(), "init") {
+			functions = append(functions, f)
+		}
+	}
+	return functions
+}
diff --git a/internal/pkg/debug/render/testdata/TestClosure.ssa b/internal/pkg/debug/render/testdata/TestClosure.ssa
new file mode 100644
index 0000000..f8cf5d3
--- /dev/null
+++ b/internal/pkg/debug/render/testdata/TestClosure.ssa
@@ -0,0 +1,13 @@
+func TestClosure()
+0: entry
+	0(*ssa.Call           ): t0 = TestClosure$1(0:int)
+	1(*ssa.Return         ): return
+func TestClosure$1(x int)
+0: entry
+	0(*ssa.Alloc          ): t0 = new [1]interface{} (varargs)
+	1(*ssa.IndexAddr      ): t1 = &t0[0:int]
+	2(*ssa.MakeInterface  ): t2 = make interface{} <- int (x)
+	3(*ssa.Store          ): *t1 = t2
+	4(*ssa.Slice          ): t3 = slice t0[:]
+	5(*ssa.Call           ): t4 = fmt.Println(t3...)
+	6(*ssa.Return         ): return
diff --git a/internal/pkg/debug/render/testdata/TestMultiBlock.ssa b/internal/pkg/debug/render/testdata/TestMultiBlock.ssa
new file mode 100644
index 0000000..374c9f7
--- /dev/null
+++ b/internal/pkg/debug/render/testdata/TestMultiBlock.ssa
@@ -0,0 +1,41 @@
+func TestMultiBlock()
+0: entry
+	 0(*ssa.Alloc          ): t0 = local image.Point (p)
+	 1(*ssa.FieldAddr      ): t1 = &t0.X [#0]
+	 2(*ssa.FieldAddr      ): t2 = &t0.Y [#1]
+	 3(*ssa.Store          ): *t1 = 1:int
+	 4(*ssa.Store          ): *t2 = 2:int
+	 5(*ssa.FieldAddr      ): t3 = &t0.X [#0]
+	 6(*ssa.UnOp           ): t4 = *t3
+	 7(*ssa.BinOp          ): t5 = t4 > 0:int
+	 8(*ssa.If             ): if t5 goto 1 else 3
+1: if.then
+	 0(*ssa.FieldAddr      ): t6 = &t0.Y [#1]
+	 1(*ssa.UnOp           ): t7 = *t6
+	 2(*ssa.BinOp          ): t8 = t7 > 0:int
+	 3(*ssa.If             ): if t8 goto 4 else 2
+2: if.done
+	 0(*ssa.Return         ): return
+3: if.else
+	 0(*ssa.Alloc          ): t9 = new [1]interface{} (varargs)
+	 1(*ssa.IndexAddr      ): t10 = &t9[0:int]
+	 2(*ssa.MakeInterface  ): t11 = make interface{} <- string ("somewhere":string)
+	 3(*ssa.Store          ): *t10 = t11
+	 4(*ssa.Slice          ): t12 = slice t9[:]
+	 5(*ssa.Call           ): t13 = fmt.Println(t12...)
+	 6(*ssa.Jump           ): jump 2
+4: if.then
+	 0(*ssa.FieldAddr      ): t14 = &t0.X [#0]
+	 1(*ssa.UnOp           ): t15 = *t14
+	 2(*ssa.FieldAddr      ): t16 = &t0.Y [#1]
+	 3(*ssa.UnOp           ): t17 = *t16
+	 4(*ssa.Alloc          ): t18 = new [2]interface{} (varargs)
+	 5(*ssa.IndexAddr      ): t19 = &t18[0:int]
+	 6(*ssa.MakeInterface  ): t20 = make interface{} <- int (t15)
+	 7(*ssa.Store          ): *t19 = t20
+	 8(*ssa.IndexAddr      ): t21 = &t18[1:int]
+	 9(*ssa.MakeInterface  ): t22 = make interface{} <- int (t17)
+	10(*ssa.Store          ): *t21 = t22
+	11(*ssa.Slice          ): t23 = slice t18[:]
+	12(*ssa.Call           ): t24 = fmt.Printf("in top right quad...":string, t23...)
+	13(*ssa.Jump           ): jump 2
diff --git a/internal/pkg/debug/render/testdata/TestParams.ssa b/internal/pkg/debug/render/testdata/TestParams.ssa
new file mode 100644
index 0000000..6bd6eb7
--- /dev/null
+++ b/internal/pkg/debug/render/testdata/TestParams.ssa
@@ -0,0 +1,15 @@
+func TestParams(a int, b int, c string)
+0: entry
+	 0(*ssa.Alloc          ): t0 = new [3]interface{} (varargs)
+	 1(*ssa.IndexAddr      ): t1 = &t0[0:int]
+	 2(*ssa.MakeInterface  ): t2 = make interface{} <- int (a)
+	 3(*ssa.Store          ): *t1 = t2
+	 4(*ssa.IndexAddr      ): t3 = &t0[1:int]
+	 5(*ssa.MakeInterface  ): t4 = make interface{} <- int (b)
+	 6(*ssa.Store          ): *t3 = t4
+	 7(*ssa.IndexAddr      ): t5 = &t0[2:int]
+	 8(*ssa.MakeInterface  ): t6 = make interface{} <- string (c)
+	 9(*ssa.Store          ): *t5 = t6
+	10(*ssa.Slice          ): t7 = slice t0[:]
+	11(*ssa.Call           ): t8 = fmt.Println(t7...)
+	12(*ssa.Return         ): return
diff --git a/internal/pkg/debug/render/testdata/TestSingleBlock.ssa b/internal/pkg/debug/render/testdata/TestSingleBlock.ssa
new file mode 100644
index 0000000..906752c
--- /dev/null
+++ b/internal/pkg/debug/render/testdata/TestSingleBlock.ssa
@@ -0,0 +1,23 @@
+func TestSingleBlock()
+0: entry
+	 0(*ssa.Alloc          ): t0 = local image.Point (p)
+	 1(*ssa.FieldAddr      ): t1 = &t0.X [#0]
+	 2(*ssa.FieldAddr      ): t2 = &t0.Y [#1]
+	 3(*ssa.Store          ): *t1 = 1:int
+	 4(*ssa.Store          ): *t2 = 2:int
+	 5(*ssa.FieldAddr      ): t3 = &t0.X [#0]
+	 6(*ssa.Store          ): *t3 = 3:int
+	 7(*ssa.FieldAddr      ): t4 = &t0.Y [#1]
+	 8(*ssa.Store          ): *t4 = 4:int
+	 9(*ssa.FieldAddr      ): t5 = &t0.X [#0]
+	10(*ssa.UnOp           ): t6 = *t5
+	11(*ssa.FieldAddr      ): t7 = &t0.Y [#1]
+	12(*ssa.UnOp           ): t8 = *t7
+	13(*ssa.BinOp          ): t9 = t6 + t8
+	14(*ssa.Alloc          ): t10 = new [1]interface{} (varargs)
+	15(*ssa.IndexAddr      ): t11 = &t10[0:int]
+	16(*ssa.MakeInterface  ): t12 = make interface{} <- int (t9)
+	17(*ssa.Store          ): *t11 = t12
+	18(*ssa.Slice          ): t13 = slice t10[:]
+	19(*ssa.Call           ): t14 = fmt.Println(t13...)
+	20(*ssa.Return         ): return
diff --git a/internal/pkg/debug/render/testdata/tests.go b/internal/pkg/debug/render/testdata/tests.go
new file mode 100644
index 0000000..64dc3a2
--- /dev/null
+++ b/internal/pkg/debug/render/testdata/tests.go
@@ -0,0 +1,50 @@
+// 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 main
+
+import (
+	"fmt"
+	"image"
+)
+
+func TestSingleBlock() {
+	p := image.Point{1, 2}
+	p.X = 3
+	p.Y = 4
+	fmt.Println(p.X + p.Y)
+}
+
+func TestMultiBlock() {
+	p := image.Point{1, 2}
+	if p.X > 0 {
+		if p.Y > 0 {
+			fmt.Printf("in top right quadrant, at (%d, %d)\n", p.X, p.Y)
+		}
+	} else {
+		fmt.Println("somewhere")
+	}
+}
+
+func TestParams(a, b int, c string) {
+	fmt.Println(a, b, c)
+}
+
+func TestClosure() {
+	x := 0
+	f := func(x int) {
+		fmt.Println(x)
+	}
+	f(x)
+}
diff --git a/internal/pkg/fieldtags/testdata/src/tests/test.go b/internal/pkg/fieldtags/testdata/src/tests/test.go
index 4e244df..511ff3f 100644
--- a/internal/pkg/fieldtags/testdata/src/tests/test.go
+++ b/internal/pkg/fieldtags/testdata/src/tests/test.go
@@ -19,7 +19,7 @@ type Person struct {
 	secret                   string      `json:"secret" levee:"source"` // want "tagged field: secret"
 	another                  interface{} "levee:\"source\""             // want "tagged field: another"
 	hasCustomFieldTag        string      `example:"sensitive"`          // want "tagged field: hasCustomFieldTag"
-	hasTagWithMultipleValues string      `levee:"value0,source,value2"` // want "tagged field: hasTagWithMultipleValues"
+	hasTagWithMultipleValues string      `example:"val,sensitive,long"` // want "tagged field: hasTagWithMultipleValues"
 	name                     string      `some_key:"non_secret"`
 	spaceAfterFinalQuote     string      `key:"value" `
 	someNotTaggedField       int
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/collections/sources.go b/internal/pkg/levee/testdata/src/example.com/tests/collections/sources.go
new file mode 100644
index 0000000..c5451cc
--- /dev/null
+++ b/internal/pkg/levee/testdata/src/example.com/tests/collections/sources.go
@@ -0,0 +1,97 @@
+// 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 collections
+
+import (
+	"example.com/core"
+)
+
+func TestSourceMapIsSource() {
+	core.Sink(map[string]string{})
+	core.Sink(map[core.Source]string{})      // want "a source has reached a sink"
+	core.Sink(map[string]core.Source{})      // want "a source has reached a sink"
+	core.Sink(map[core.Source]core.Source{}) // want "a source has reached a sink"
+}
+
+func TestSourceArrayIsSource() {
+	core.Sink([1]string{})
+	core.Sink([1]core.Source{}) // want "a source has reached a sink"
+}
+
+func TestSourceSliceIsSource() {
+	core.Sink([]string{})
+	core.Sink([]core.Source{}) // want "a source has reached a sink"
+}
+
+func TestSourceChanIsSource() {
+	core.Sink(make(chan string))
+	core.Sink(make(chan core.Source)) // want "a source has reached a sink"
+}
+
+func TestSourcePointerCollectionIsSource() {
+	core.Sink([]*core.Source{}) // want "a source has reached a sink"
+}
+
+func TestSourceCollectionParamIsSource(ss []core.Source) {
+	core.Sink(ss) // want "a source has reached a sink"
+}
+
+func TestSourceCollectionReturnedFromFunctionCallIsSource() {
+	core.Sink(GetSourcesSlice()) // want "a source has reached a sink"
+	core.Sink(GetSourcesMap())   // want "a source has reached a sink"
+}
+
+func TestSourceCollectionReturnedFromMethodCallIsSource() {
+	core.Sink(SourcesHolder{}.GetSourcesSlice()) // want "a source has reached a sink"
+	core.Sink(SourcesHolder{}.GetSourcesMap())   // want "a source has reached a sink"
+}
+
+func TestSourceCollectionInStructFieldIsSource() {
+	core.Sink(SourcesHolder{}.Sources)    // want "a source has reached a sink"
+	core.Sink(SourcesHolder{}.SourcesMap) // want "a source has reached a sink"
+}
+
+type SourceSlice []core.Source
+
+func TestAliasedSourceSollectionIsSource() {
+	core.Sink(SourceSlice{}) // want "a source has reached a sink"
+}
+
+type DeeplyNested map[string][]map[string]map[string][][]map[string]core.Source
+
+func TestDeeplyNestedSourceCollectionIsSource() {
+	core.Sink(DeeplyNested{}) // want "a source has reached a sink"
+}
+
+func GetSourcesSlice() []core.Source {
+	return nil
+}
+
+func GetSourcesMap() map[string]core.Source {
+	return nil
+}
+
+type SourcesHolder struct {
+	Sources    []core.Source
+	SourcesMap map[string]core.Source
+}
+
+func (sh SourcesHolder) GetSourcesSlice() []core.Source {
+	return sh.Sources
+}
+
+func (sh SourcesHolder) GetSourcesMap() map[string]core.Source {
+	return sh.SourcesMap
+}
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/pointers/tests.go b/internal/pkg/levee/testdata/src/example.com/tests/pointers/tests.go
index f15a8a3..13ddbed 100644
--- a/internal/pkg/levee/testdata/src/example.com/tests/pointers/tests.go
+++ b/internal/pkg/levee/testdata/src/example.com/tests/pointers/tests.go
@@ -24,9 +24,9 @@ func TestNew() {
 }
 
 func TestDoublePointer(s **core.Source) {
-	core.Sink(s)   // TODO want "a source has reached a sink"
-	core.Sink(*s)  // TODO want "a source has reached a sink"
-	core.Sink(**s) // TODO want "a source has reached a sink"
+	core.Sink(s)   // want "a source has reached a sink"
+	core.Sink(*s)  // want "a source has reached a sink"
+	core.Sink(**s) // want "a source has reached a sink"
 }
 
 func TestDoubleReference(s core.Source) {
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/position/tests.go b/internal/pkg/levee/testdata/src/example.com/tests/position/tests.go
index 9dde540..cfe9dc9 100644
--- a/internal/pkg/levee/testdata/src/example.com/tests/position/tests.go
+++ b/internal/pkg/levee/testdata/src/example.com/tests/position/tests.go
@@ -23,6 +23,29 @@ func TestSourcePointerExtract() {
 	core.Sink(s) // want "a source has reached a sink, source: .*tests.go:22:19"
 }
 
+// In order for the SSA to contain a FieldAddr, the EmbedsSource instance's fields have to be addressable.
+// We use a local variable here so that the fields will be addressable.
+// The field is extracted on its own line so we can differentiate between the struct's position
+// and the field's position.
+// The Source in this function is created by a FieldAddr that is not represented explicitly in the code.
+// Indeed, es.Data is actually es.Source.Data.
+// Because of this, we expect the report to be produced at the struct's position.
+func TestEmbeddedSourceFieldAddr() {
+	es := EmbedsSource{}
+	d := es.Data
+	core.Sink(d) // want "a source has reached a sink, source: .*tests.go:34:2"
+}
+
+// In order for the SSA to contain a Field, the EmbedsSource instance's fields must not be addressable.
+// One way to do this is to create a literal and to access the field directly, as part of the same expression.
+func TestEmbeddedSourceField() {
+	core.Sink(EmbedsSource{}.Data) // want "a source has reached a sink, source: .*tests.go:42:24"
+}
+
+type EmbedsSource struct {
+	core.Source
+}
+
 func NewSource() (*core.Source, error) {
 	return &core.Source{}, nil
 }
diff --git a/internal/pkg/source/testdata/src/analyzertest/sourcetest/identification.go b/internal/pkg/source/testdata/src/analyzertest/sourcetest/identification.go
index 8c90ce8..2a975f4 100644
--- a/internal/pkg/source/testdata/src/analyzertest/sourcetest/identification.go
+++ b/internal/pkg/source/testdata/src/analyzertest/sourcetest/identification.go
@@ -46,17 +46,37 @@ func TestSourceDeclarations() {
 	noop(varZeroVal, declZeroVal, populatedVal, constPtr, ptr, newPtr, ptrToDeclZero, ptrToDeclPopulataed, alias, def)
 }
 
-// A report should be emitted for each parameter.
-func TestSourceParameters(val Source, ptr *Source) { // want "source identified" "source identified"
+// A report should be emitted for each parameter, as well as the (implicit) Alloc for val.
+func TestSourceParameters(val Source, ptr *Source) { // want "source identified" "source identified" "source identified"
 
 }
 
 func TestSourceExtracts() {
-	s, err := CreateSource()                     // want "source identified"
-	sptr, err := NewSource()                     // want "source identified"
-	mapSource, ok := map[string]Source{}[""]     // want "source identified"
-	mapSourcePtr, ok := map[string]*Source{}[""] // want "source identified"
-	chanSource, ok := <-(make(chan Source))      // want "source identified"
-	chanSourcePtr, ok := <-(make(chan *Source))  // want "source identified"
+	s, err := CreateSource() // want "source identified"
+	sptr, err := NewSource() // want "source identified"
+
+	// we expect two reports for the following cases, since the map is a Source
+	mapSource, ok := map[string]Source{}[""]     // want "source identified" "source identified"
+	mapSourcePtr, ok := map[string]*Source{}[""] // want "source identified" "source identified"
+
+	// we expect two reports for the following cases, since the chan is a Source
+	chanSource, ok := <-(make(chan Source))     // want "source identified" "source identified"
+	chanSourcePtr, ok := <-(make(chan *Source)) // want "source identified" "source identified"
+
 	_, _, _, _, _, _, _, _ = s, sptr, mapSource, chanSource, mapSourcePtr, chanSourcePtr, err, ok
 }
+
+func TestCollections(ss []Source) { // want "source identified"
+	_ = map[Source]string{} // want "source identified"
+	_ = map[string]Source{} // want "source identified"
+	_ = map[Source]Source{} // want "source identified"
+	_ = [1]Source{}         // want "source identified"
+	_ = []Source{}          // want "source identified"
+	_ = make(chan Source)   // want "source identified"
+	_ = []*Source{}         // want "source identified"
+	_ = SourceSlice{}       // want "source identified"
+	_ = DeeplyNested{}      // want "source identified"
+}
+
+type SourceSlice []Source
+type DeeplyNested map[string][]map[string]map[string][][]map[string]Source