Merge branch 'master' into only-traverse-to-reference-args

internal/pkg/levee/testdata/src/example.com/tests/collections/maps.go

@@ -53,12 +53,9 @@ func TestMapRemainsTaintedWhenSourceIsDeleted(s core.Source) {
 	core.Sink(m) // want "a source has reached a sink"
 }
 
-func TestDeletingFromTaintedMapDoesNotTaintKey(key string, sources map[string]core.Source) {
-	delete(sources, key)
-	core.Sink(key)
-}
-
-func TestDeletingFromTaintedMapDoesNotTaintPointerKey(key *string, sources map[*string]core.Source) {
+func TestDeletingFromTaintedMapDoesNotTaintKey(key *string, sources map[*string]core.Source) {
+	// The key needs to be a pointer parameter, because we don't traverse to non-pointer
+	// arguments of a call, and we don't traverse to Allocs.
 	delete(sources, key)
 	// TODO: no report should be produced here
 	core.Sink(key) // want "a source has reached a sink"

internal/pkg/levee/testdata/src/example.com/tests/colocation/tests.go

@@ -16,45 +16,57 @@ package colocation
 
 import (
 	"encoding/json"
+	"reflect"
 	"unsafe"
 
 	"example.com/core"
 )
 
 func colocateString(core.Source, string) {}
+
 func TestBasicTypeIsNotTainted(s core.Source, str string) {
 	colocateString(s, str)
 	core.Sink(str)
 }
 
 func colocateStringPointer(core.Source, *string) {}
+
 func TestBasicPointerTypeIsTainted(s core.Source, strptr *string) {
 	colocateStringPointer(s, strptr)
 	core.Sink(strptr) // want "a source has reached a sink"
 }
+
 func TestPointerToBasicTypeIsTainted(s core.Source, str string) {
+	// This test is failing because &x introduces an Alloc,
+	// and we don't traverse through non-array Allocs
 	colocateStringPointer(s, &str)
 	core.Sink(str) // TODO want "a source has reached a sink"
 }
 
 func colocateInnoc(core.Source, core.Innocuous) {}
+
 func TestNamedStructTypeIsNotTainted(s core.Source, i core.Innocuous) {
 	colocateInnoc(s, i)
 	core.Sink(i)
 
 }
 
 func colocateInnocPtr(core.Source, *core.Innocuous) {}
+
 func TestNamedStructPointerIsTainted(s core.Source, i *core.Innocuous) {
 	colocateInnocPtr(s, i)
 	core.Sink(i) // want "a source has reached a sink"
 }
+
 func TestPointerToNamedStructIsTainted(s core.Source, i core.Innocuous) {
+	// This test is failing because &x introduces an Alloc,
+	// and we don't traverse through non-array Allocs
 	colocateInnocPtr(s, &i)
 	core.Sink(i) // TODO want "a source has reached a sink"
 }
 
 func colocateUnsafePointer(core.Source, unsafe.Pointer) {}
+
 func TestUnsafePointerIsTainted(s core.Source, up unsafe.Pointer) {
 	colocateUnsafePointer(s, up)
 	core.Sink(up) // want "a source has reached a sink"
@@ -63,39 +75,46 @@ func TestUnsafePointerIsTainted(s core.Source, up unsafe.Pointer) {
 type PointerHolder struct{ ptr *core.Source }
 
 func colocatePointerHolder(core.Source, PointerHolder) {}
+
 func TestNamedStructPointerHolderIsTainted(s core.Source, ph PointerHolder) {
+	// This test is failing because ph is created by an Alloc,
+	// and we don't traverse through non-array Allocs
 	colocatePointerHolder(s, ph)
 	core.Sink(ph) // TODO want "a source has reached a sink"
 }
 
 type InnocSlice []core.Innocuous
 
 func colocateInnocSlice(core.Source, InnocSlice) {}
+
 func TestNamedSliceTypeIsTainted(s core.Source, is InnocSlice) {
 	colocateInnocSlice(s, is)
 	core.Sink(is) // want "a source has reached a sink"
 }
 
 func colocateArrOfValues(core.Source, [1]string) {}
+
 func TestArrOfValuesIsNotTainted(s core.Source, arr [1]string) {
 	colocateArrOfValues(s, arr)
 	core.Sink(arr)
 }
 
 func colocateArrOfPointers(core.Source, [1]*string) {}
+
 func TestArrOfPointersIsTainted(s core.Source, arr [1]*string) {
 	colocateArrOfPointers(s, arr)
-	// XXX
 	core.Sink(arr) // want "a source has reached a sink"
 }
 
 func colocateFunc(core.Source, func()) {}
+
 func TestFuncIsNotTainted(s core.Source, f func()) {
 	colocateFunc(s, f)
 	core.Sink(f)
 }
 
 func colocateReferenceCollections(core.Source, map[string]string, chan string, []string) {}
+
 func TestReferenceCollectionsAreTainted(s core.Source) {
 	m := make(map[string]string)
 	c := make(chan string)
@@ -106,24 +125,39 @@ func TestReferenceCollectionsAreTainted(s core.Source) {
 	core.Sink(sl) // want "a source has reached a sink"
 }
 
+func colocateReflectValue(core.Source, reflect.Value) {}
+
+func TestReflectValuesAreTainted(s core.Source, r reflect.Value) {
+	// This test is failing because ph is created by an Alloc,
+	// and we don't traverse through non-array Allocs
+	colocateReflectValue(s, r)
+	core.Sink(r) // TODO want "a source has reached a sink"
+}
+
 func colocateEface(s core.Source, taintees ...interface{}) {}
+
 func TestTaintedEface(s core.Source, i interface{}) {
 	colocateEface(s, i)
-	// XXX: we have no idea what i is hiding, so we have to assume the worst
 	core.Sink(i) // want "a source has reached a sink"
 
 }
+
 func TestTaintedThroughEface(s core.Source, str string, i core.Innocuous) {
 	colocateEface(s, str, i)
-	// XXX: it is true that we don't want reports here, but I don't really believe it...
-	core.Sink(str)
-	core.Sink(i)
+	// Ideally, we wouldn't want reports for either of those values, because they
+	// are not pointers.
+	// However, because they are passed to the call via an interface type,
+	// we have no easy way to know that these values can't actually be tainted.
+	core.Sink(str) // want "a source has reached a sink"
+	core.Sink(i)   // TODO want "a source has reached a sink"
 }
+
 func TestPointerTaintedThroughEface(s core.Source, str string, i core.Innocuous) {
 	colocateEface(s, &str, &i)
-	// XXX: this is probably failing because of Allocs
-	core.Sink(str) // want "a source has reached a sink"
-	core.Sink(i)   // want "a source has reached a sink"
+	// These tests are failing because &x introduces an Alloc,
+	// and we don't traverse through non-array Allocs
+	core.Sink(str) // TODO want "a source has reached a sink"
+	core.Sink(i)   // TODO want "a source has reached a sink"
 }
 
 // CVE-2020-8564

internal/pkg/source/source.go

@@ -176,10 +176,11 @@ func (s *Source) visit(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]int, last
 		}
 
 		s.visitReferrers(n, maxInstrReached, lastBlockVisited)
-		// A call's operands should only be visited if they are pointers.
-		s.visitOperands(n, maxInstrReached, lastBlockVisited, func(v ssa.Value) bool {
-			return canPoint(v.Type())
-		})
+		for _, a := range t.Call.Args {
+			if canPoint(a.Type()) {
+				s.dfs(a.(ssa.Node), maxInstrReached, lastBlockVisited, false)
+			}
+		}
 
 	case *ssa.FieldAddr:
 		deref := utils.Dereference(t.X.Type())
@@ -189,7 +190,7 @@ func (s *Source) visit(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]int, last
 			return
 		}
 		s.visitReferrers(n, maxInstrReached, lastBlockVisited)
-		s.visitOperands(n, maxInstrReached, lastBlockVisited, nil)
+		s.visitOperands(n, maxInstrReached, lastBlockVisited)
 
 	// Everything but the actual integer Index should be visited.
 	case *ssa.Index:
@@ -216,7 +217,7 @@ func (s *Source) visit(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]int, last
 
 	// These nodes' operands should not be visited, because they can only receive
 	// taint from their operands, not propagate taint to them.
-	case *ssa.BinOp, *ssa.ChangeInterface, *ssa.ChangeType, *ssa.Convert, *ssa.Extract, *ssa.MakeChan, *ssa.MakeMap, *ssa.MakeSlice, *ssa.Phi, *ssa.Range, *ssa.Slice, *ssa.UnOp:
+	case *ssa.BinOp, *ssa.ChangeInterface, *ssa.ChangeType, *ssa.Convert, *ssa.Extract, *ssa.MakeChan, *ssa.MakeMap, *ssa.MakeSlice, *ssa.Phi, *ssa.Range:
 		s.visitReferrers(n, maxInstrReached, lastBlockVisited)
 
 	// These nodes don't have operands; they are Values, not Instructions.
@@ -225,12 +226,12 @@ func (s *Source) visit(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]int, last
 
 	// These nodes don't have referrers; they are Instructions, not Values.
 	case *ssa.Go, *ssa.Store:
-		s.visitOperands(n, maxInstrReached, lastBlockVisited, nil)
+		s.visitOperands(n, maxInstrReached, lastBlockVisited)
 
 	// These nodes are both Instructions and Values, and currently have no special restrictions.
-	case *ssa.Field, *ssa.MakeInterface, *ssa.Select, *ssa.TypeAssert:
+	case *ssa.Field, *ssa.MakeInterface, *ssa.Select, *ssa.Slice, *ssa.TypeAssert, *ssa.UnOp:
 		s.visitReferrers(n, maxInstrReached, lastBlockVisited)
-		s.visitOperands(n, maxInstrReached, lastBlockVisited, nil)
+		s.visitOperands(n, maxInstrReached, lastBlockVisited)
 
 	// These nodes cannot propagate taint.
 	case *ssa.Builtin, *ssa.DebugRef, *ssa.Defer, *ssa.Function, *ssa.If, *ssa.Jump, *ssa.MakeClosure, *ssa.Next, *ssa.Panic, *ssa.Return, *ssa.RunDefers:
@@ -249,9 +250,9 @@ func (s *Source) visitReferrers(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]
 	}
 }
 
-func (s *Source) visitOperands(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]int, lastBlockVisited *ssa.BasicBlock, shouldVisit func(ssa.Value) bool) {
+func (s *Source) visitOperands(n ssa.Node, maxInstrReached map[*ssa.BasicBlock]int, lastBlockVisited *ssa.BasicBlock) {
 	for _, o := range n.Operands(nil) {
-		if *o == nil || shouldVisit != nil && !shouldVisit(*o) {
+		if *o == nil {
 			continue
 		}
 		s.dfs((*o).(ssa.Node), maxInstrReached, lastBlockVisited, false)
@@ -297,10 +298,6 @@ func (s *Source) compress() []ssa.Node {
 	return compressed
 }
 
-func (s *Source) RefersTo(n ssa.Node) bool {
-	return s.HasPathTo(n)
-}
-
 // HasPathTo returns true when a Node is part of declaration-use graph.
 func (s *Source) HasPathTo(n ssa.Node) bool {
 	return s.marked[n]
@@ -468,13 +465,13 @@ func isSourceType(c classifier, t types.Type) bool {
 	}
 }
 
-// A type "can point if it is itself a pointer or pointer-like type, or it contains
+// A type "can point" if it is itself a pointer or pointer-like type, or it contains
 // a pointer or pointer-like type.
 func canPoint(t types.Type) bool {
 	switch tt := t.(type) {
 	// These types can point.
 	// Chan, Map and Slice are reference-like collections.
-	// Some interfaces can hold pointers.
+	// Interfaces can hold pointers.
 	case *types.Chan, *types.Interface, *types.Map, *types.Pointer, *types.Slice:
 		return true
 
@@ -488,6 +485,11 @@ func canPoint(t types.Type) bool {
 		return false
 
 	case *types.Named:
+		// This snippet is plundered from golang.org/x/tools/go/pointer.CanPoint
+		if obj := tt.Obj(); obj.Name() == "Value" && obj.Pkg().Path() == "reflect" {
+			return true // treat reflect.Value like interface{}
+		}
+
 		return canPoint(tt.Underlying())
 
 	// A struct can point if one of its fields can point.