Repository: google/go-flow-levee (go)

Issue:
Source graph should continue beyond sanitizers.
Currently, when building a graph of a source we [stop](https://github.com/google/go-flow-levee/blob/master/internal/pkg/source/source.go#L86) following the graph when a sanitizer is encountered.
However, there is not guarantee that such sanitizer will dominate the yet to be discovered sinks.
Therefore, we should build the graph beyond sanitizers.
Once we know all the sinks, we could trip the graph by removing sinks that are dominated by a sanitizers.


Pull request:
Follow the graph beyond the sanitizer.
Fixes #24

Currently, when building a graph of a source we stop following the graph when a sanitizer is encountered.
However, there is not guarantee that such sanitizer will dominate the yet to be discovered sinks.
Therefore, we should build the graph beyond sanitizers.
Once we know all the sinks, we could trip the graph by removing sinks that are dominated by a sanitizers.



Patch:
diff --git a/internal/pkg/source/source.go b/internal/pkg/source/source.go
index dc3af6d..82248ca 100644
--- a/internal/pkg/source/source.go
+++ b/internal/pkg/source/source.go
@@ -81,19 +81,25 @@ func (a *Source) bfs() {
 				continue
 			}
 
-			if c, ok := r.(*ssa.Call); ok && a.config.IsSanitizer(c) {
-				a.sanitizers = append(a.sanitizers, &sanitizer.Sanitizer{Call: c})
-				continue
-			}
-
-			// Need to stay within the scope of the function under analysis.
-			if call, ok := r.(*ssa.Call); ok && !a.config.IsPropagator(call) {
-				continue
-			}
+			// TODO(immutableT) We are putting too much logic into this function - it should just
+			// be constructing a graph. Add specialized methods that know how to trim or interpret
+			// connections.
+
+			switch v := r.(type) {
+			case *ssa.Call:
+				// This is to avoid attaching calls where the source is the receiver, ex:
+				// core.Sinkf("Source id: %v", wrapper.Source.GetID())
+				if v.Call.Signature().Recv() != nil {
+					continue
+				}
 
-			// Do not follow innocuous field access.
-			if addr, ok := r.(*ssa.FieldAddr); ok && !a.config.IsSourceFieldAddr(addr) {
-				continue
+				if a.config.IsSanitizer(v) {
+					a.sanitizers = append(a.sanitizers, &sanitizer.Sanitizer{Call: v})
+				}
+			case *ssa.FieldAddr:
+				if !a.config.IsSourceFieldAddr(v) {
+					continue
+				}
 			}
 
 			a.marked[r.(ssa.Node)] = true


Test patch:
diff --git a/internal/pkg/source/source_test.go b/internal/pkg/source/source_test.go
index 43bc05e..ca43551 100644
--- a/internal/pkg/source/source_test.go
+++ b/internal/pkg/source/source_test.go
@@ -103,6 +103,7 @@ func TestSource(t *testing.T) {
 		pattern                      string
 		config                       *testConfig
 		wantCallConnectionIdx        []int
+		wantSanitizedCallIdx         []int
 		wantFieldAccessConnectionIdx []int
 		wantStoreConnectionIdx       []int
 	}{
@@ -122,6 +123,12 @@ func TestSource(t *testing.T) {
 			pattern:                      "sanitization",
 			config:                       config,
 			wantFieldAccessConnectionIdx: []int{0},
+			wantSanitizedCallIdx:         []int{1},
+		},
+		{
+			pattern:               "domination",
+			config:                config,
+			wantCallConnectionIdx: []int{2},
 		},
 	}
 
@@ -145,11 +152,17 @@ func TestSource(t *testing.T) {
 			t.Logf("Testing source:\n%v", src)
 
 			for i := 0; i < len(tt.wantCallConnectionIdx); i++ {
-				if !src.HasPathTo(a.calls[i]) {
+				if !src.HasPathTo(a.calls[tt.wantCallConnectionIdx[i]]) {
 					t.Errorf("Expected\n%v to have a path to %v", src, a.calls[i])
 				}
 			}
 
+			for i := 0; i < len(tt.wantSanitizedCallIdx); i++ {
+				if !src.IsSanitizedAt(a.calls[tt.wantSanitizedCallIdx[i]]) {
+					t.Errorf("Expected\n%v to be sanitized befere a call to %v", src, a.calls[i])
+				}
+			}
+
 			for i := 0; i < len(tt.wantStoreConnectionIdx); i++ {
 				if !src.HasPathTo(a.store[i]) {
 					t.Errorf("Expected\n%v to have a path to %v", src, a.store[i])
diff --git a/internal/pkg/source/testdata/src/domination/test.go b/internal/pkg/source/testdata/src/domination/test.go
new file mode 100644
index 0000000..2aee36e
--- /dev/null
+++ b/internal/pkg/source/testdata/src/domination/test.go
@@ -0,0 +1,41 @@
+// 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 domination
+
+import (
+	"fmt"
+	"time"
+)
+
+type foo struct {
+	name     string
+	password string
+}
+
+func f1() {
+	f := &foo{name: "n", password: "p"}
+	if time.Now().Year() == 2020 {
+		sanitizer(f)
+	}
+	sink(f)
+}
+
+func sanitizer(in *foo) {
+	in.password = "redacted"
+}
+
+func sink(in *foo) {
+	fmt.Print(in)
+}