Repository: google/go-flow-levee
Pull request #31 (2020-07-16 13:47:58)

Field tags analyzer
A minimal field tags analyzer to check whether I'm on the right track.
At this point this is mostly to establish feasibility and prompt further discussion.


Patch:
diff --git a/internal/pkg/fieldtags/analyzer.go b/internal/pkg/fieldtags/analyzer.go
new file mode 100644
index 0000000..39b940c
--- /dev/null
+++ b/internal/pkg/fieldtags/analyzer.go
@@ -0,0 +1,121 @@
+// 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 fieldtags defines an analyzer that identifies struct fields identified
+// as sources via a field tag.
+package fieldtags
+
+import (
+	"go/ast"
+	"reflect"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+type keyValue struct {
+	key   string
+	value string
+}
+
+type sourcePatterns []keyValue
+
+var patterns sourcePatterns = []keyValue{
+	{"levee", "source"},
+}
+
+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.",
+	Run:  run,
+	Requires: []*analysis.Analyzer{
+		inspect.Analyzer,
+	},
+	ResultType: reflect.TypeOf(new([]*ast.Field)).Elem(),
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	inspectResult := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	var results []*ast.Field
+
+	nodeFilter := []ast.Node{
+		(*ast.Field)(nil),
+	}
+
+	inspectResult.Preorder(nodeFilter, func(n ast.Node) {
+		field, ok := n.(*ast.Field)
+		if !ok {
+			return
+		}
+		if patterns.isSource(field) {
+			results = append(results, field)
+			pass.Reportf(field.Pos(), "tagged field")
+		}
+	})
+	return results, nil
+}
+
+func (sp *sourcePatterns) isSource(field *ast.Field) bool {
+	if field.Tag == nil {
+		return false
+	}
+
+	tag := field.Tag.Value
+
+	// TODO: consider refactoring this logic into a regex matcher
+	i := 1 // skip initial quote
+	j := 1
+	for j < len(tag) {
+		for j < len(tag) && tag[j] != ':' {
+			j++
+		}
+		key := tag[i:j]
+		if key == "" {
+			return false
+		}
+
+		i = j + 1 // skip colon
+		if tag[i] == '\\' {
+			i++
+		}
+		i++ // skip quote
+
+		j = i
+		for j < len(tag) && tag[j] != '"' {
+			// skip escape character
+			if tag[j] == '\\' {
+				j++
+			}
+			j++
+		}
+		value := tag[i:j]
+		if value == "" {
+			return false
+		}
+
+		for _, p := range *sp {
+			// value may be a list of comma separated values
+			if p.key == key && strings.Contains(value, p.value) {
+				return true
+			}
+		}
+
+		i = j + 2 // skip closing quote and space
+		j = i
+	}
+
+	return false
+}
diff --git a/internal/pkg/source/source.go b/internal/pkg/source/source.go
index 35ea45d..6f448f0 100644
--- a/internal/pkg/source/source.go
+++ b/internal/pkg/source/source.go
@@ -198,7 +198,7 @@ func sourcesFromBlocks(fn *ssa.Function, conf classifier) []*Source {
 			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())) {
+				if conf.IsSource(utils.Dereference(v.Type())) && !isProducedBySanitizer(v, conf) {
 					sources = append(sources, New(v, conf))
 				}
 
@@ -213,3 +213,20 @@ func sourcesFromBlocks(fn *ssa.Function, conf classifier) []*Source {
 	}
 	return sources
 }
+
+func isProducedBySanitizer(v *ssa.Alloc, conf classifier) bool {
+	for _, instr := range *v.Referrers() {
+		store, ok := instr.(*ssa.Store)
+		if !ok {
+			continue
+		}
+		call, ok := store.Val.(*ssa.Call)
+		if !ok {
+			continue
+		}
+		if conf.IsSanitizer(call) {
+			return true
+		}
+	}
+	return false
+}


Test patch:
diff --git a/internal/pkg/fieldtags/analyzer_test.go b/internal/pkg/fieldtags/analyzer_test.go
new file mode 100644
index 0000000..40abfec
--- /dev/null
+++ b/internal/pkg/fieldtags/analyzer_test.go
@@ -0,0 +1,27 @@
+// 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 fieldtags
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestFieldTagsAnalysis(t *testing.T) {
+	testdata := analysistest.TestData()
+
+	analysistest.Run(t, testdata, Analyzer, "tests")
+}
diff --git a/internal/pkg/fieldtags/testdata/src/tests/test.go b/internal/pkg/fieldtags/testdata/src/tests/test.go
new file mode 100644
index 0000000..7a44e12
--- /dev/null
+++ b/internal/pkg/fieldtags/testdata/src/tests/test.go
@@ -0,0 +1,23 @@
+// 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 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"
+	name               string      `some_key:"non_secret"`
+	someNotTaggedField int
+}
diff --git a/internal/pkg/source/testdata/src/analyzertest/sourcetest/test.go b/internal/pkg/source/testdata/src/analyzertest/sourcetest/test.go
index 86f70f4..53bd09e 100644
--- a/internal/pkg/source/testdata/src/analyzertest/sourcetest/test.go
+++ b/internal/pkg/source/testdata/src/analyzertest/sourcetest/test.go
@@ -28,12 +28,19 @@ func TestSourceDeclarations() {
 	declZeroVal := Source{}                       // want "source identified"
 	populatedVal := Source{Data: "secret", ID: 0} // want "source identified"
 
-	var ptr *Source                                       // TODO want "source identified"
+	// We do not want a "source identified" here, since this is nil
+	// and gets optimized out when the SSA is built.
+	var constPtr *Source
+
+	var ptr *Source
+	// We do want a "source identified" here.
+	// ptr does not get optimized out because it gets assigned.
+	ptr = &Source{}                                       // 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)
+	noop(varZeroVal, declZeroVal, populatedVal, constPtr, ptr, newPtr, ptrToDeclZero, ptrToDeclPopulataed)
 }
 
 // A report should be emitted for each parameter.
diff --git a/internal/testdata/src/example.com/tests/dominance/tests.go b/internal/testdata/src/example.com/tests/dominance/tests.go
index 6e81b90..cf06d62 100644
--- a/internal/testdata/src/example.com/tests/dominance/tests.go
+++ b/internal/testdata/src/example.com/tests/dominance/tests.go
@@ -33,8 +33,7 @@ func TestSanitizedSourceDoesNotTriggerFindingWhenTypeAsserted(c *core.Source) {
 
 func TestSanitizedSourceDoesNotTriggerFindingWithTypedSanitizer(c core.Source) {
 	sanitized := core.SanitizeSource(c)
-	// TODO This should not trigger
-	core.Sinkf("Sanitized %v", sanitized) // want "a source has reached a sink,"
+	core.Sinkf("Sanitized %v", sanitized)
 }
 
 func TestNotGuaranteedSanitization(c *core.Source) {