Adding warning for usage of unsound features when a function is being…

… summarized.

analysis/dataflow/builtins.go

@@ -121,8 +121,8 @@ func doBuiltinCall(t *IntraAnalysisState, callValue ssa.Value, callCommon *ssa.C
 
 		// for recover, we will need some form of panic analysis
 		case "recover":
-			t.parentAnalyzerState.Logger.Warnf("Encountered recover at %s, the analysis may be unsound.\n",
-				instruction.Parent().Prog.Fset.Position(instruction.Pos()))
+			// TODO: handler recovers. Analysis is unsound with recovers.
+			// NOTE: Unsoundness is reported by reportUnsoundFeatures
 			return true
 		default:
 			// Special case: the call to Error() of the builtin error interface

analysis/dataflow/inter_procedural.go

@@ -150,7 +150,7 @@ func (g *InterProceduralFlowGraph) BuildGraph() {
 					externalContractSummary := g.AnalyzerState.LoadExternalContractSummary(node)
 					if externalContractSummary != nil {
 						logger.Debugf("Loaded %s from external contracts.\n",
-							formatutil.SanitizeRepr(node.CallSite().Common()))
+							formatutil.SanitizeRepr(node.Callee()))
 						g.Summaries[node.Callee()] = externalContractSummary
 						node.CalleeSummary = externalContractSummary
 						if x := externalContractSummary.Callsites[node.CallSite()]; x == nil {

analysis/dataflow/intra_procedural.go

@@ -106,6 +106,8 @@ func RunIntraProcedural(a *AnalyzerState, sm *SummaryGraph) (time.Duration, erro
 		postBlockCallback:   sm.postBlockCallBack,
 	}
 
+	reportUnsoundFeatures(a, sm.Parent)
+
 	// Output warning if defer stack is unbounded
 	if !state.deferStacks.DeferStackBounded {
 		a.Logger.Warnf("Defer stack unbounded in %s: %s",

analysis/dataflow/report.go

@@ -16,9 +16,12 @@ package dataflow
 
 import (
 	"fmt"
+	"go/token"
+	"strings"
 
 	"github.com/awslabs/ar-go-tools/analysis/lang"
 	"github.com/awslabs/ar-go-tools/internal/formatutil"
+	"golang.org/x/tools/go/ssa"
 )
 
 // ReportMissingOrNotConstructedSummary prints a missing summary message to the cache's logger.
@@ -106,3 +109,102 @@ func (s *AnalyzerState) ReportSummaryNotConstructed(callSite *CallNode) {
 		}
 	}
 }
+
+type unsoundFeaturesMap struct {
+	Recovers      map[token.Position]bool
+	UnsafeUsages  map[token.Position]string
+	ReflectUsages map[token.Position]string
+}
+
+// reportUnsoundFeatures logs warning messages when unsound features are used in a function. Those include:
+//
+// - call to recover builtin
+//
+// - unsafe
+//
+// - reflect
+//
+// Usages of those features are logged at WARN level with the position of where the feature is used.
+func reportUnsoundFeatures(state *AnalyzerState, f *ssa.Function) {
+	unsoundFeatures := FindUnsoundFeatures(f)
+	if len(unsoundFeatures.Recovers) > 0 ||
+		len(unsoundFeatures.UnsafeUsages) > 0 ||
+		len(unsoundFeatures.ReflectUsages) > 0 {
+		msg := fmt.Sprintf("Function %s is using features that may make the analysis unsound.\n", f.String())
+
+		if len(unsoundFeatures.Recovers) > 0 {
+			msg += "    Using recover at position:\n"
+		}
+		for pos := range unsoundFeatures.Recovers {
+			msg += "\t  " + pos.String() + "\n"
+		}
+
+		if len(unsoundFeatures.UnsafeUsages) > 0 {
+			msg += "    Usages of unsafe:\n"
+		}
+
+		for pos, usageMsg := range unsoundFeatures.UnsafeUsages {
+			msg += "      At " + pos.String() + " : " + usageMsg + "\n"
+		}
+
+		if len(unsoundFeatures.ReflectUsages) > 0 {
+			msg += "    Usages of reflection:\n"
+		}
+
+		for pos, usageMsg := range unsoundFeatures.ReflectUsages {
+			msg += "      At " + pos.String() + " : " + usageMsg + "\n"
+		}
+		msg += "    Adding a predefined summary might help avoid soundness issues.\n"
+
+		state.Logger.Warnf(msg)
+	}
+}
+func FindUnsoundFeatures(f *ssa.Function) unsoundFeaturesMap {
+	unsafeUsages := map[token.Position]string{}
+	recovers := map[token.Position]bool{}
+	reflectUsages := map[token.Position]string{}
+	lang.IterateInstructions(f, func(index int, instrI ssa.Instruction) {
+		iPos := instrI.Parent().Prog.Fset.Position(instrI.Pos())
+		switch instr := instrI.(type) {
+		case ssa.CallInstruction:
+			callCommon := instr.Common()
+			if callCommon.Value == nil {
+				return
+			}
+			if callCommon.Value.Name() == "recover" {
+				recovers[iPos] = true
+				return
+			}
+			if callCommon.IsInvoke() {
+				// Only warn for implementations.
+				return
+			}
+			typStr := callCommon.Value.Type().String()
+			if strings.Contains(typStr, "unsafe") {
+				unsafeUsages[iPos] = fmt.Sprintf("Calling %s from unsafe package.", callCommon.Value.Name())
+				return
+			}
+			if strings.Contains(typStr, "reflect") {
+				reflectUsages[iPos] = fmt.Sprintf("Calling %s from reflect package.", callCommon.Value.Name())
+				return
+			}
+
+		case *ssa.Alloc:
+			typ := instr.Type().Underlying()
+			if strings.HasPrefix(typ.String(), "unsafe") {
+				unsafeUsages[iPos] = fmt.Sprintf("Allocating object of type %s", typ.String())
+				return
+			}
+			if strings.HasPrefix(typ.String(), "reflect") {
+				reflectUsages[iPos] = fmt.Sprintf("Allocating object of type %s", typ.String())
+				return
+			}
+		case *ssa.Convert:
+			typStr := instr.Type().String()
+			if strings.Contains(typStr, "unsafe") {
+				unsafeUsages[iPos] = "Converting data to an unsafe pointer."
+			}
+		}
+	})
+	return unsoundFeaturesMap{recovers, unsafeUsages, reflectUsages}
+}

analysis/dataflow/testdata/unsound-features/config.yaml

@@ -0,0 +1,15 @@
+taint-tracking-problems:
+  - sources:
+      - package: main
+        method: Source
+      - package: command-line-arguments
+        method: Source
+      - package: (main|command-line-arguments)
+        field: Source
+    sinks:
+      - package: command-line-arguments
+        method: Sink
+      - package: main
+        method: Sink
+options:
+  summarize-on-demand: true

analysis/dataflow/testdata/unsound-features/main.go

@@ -0,0 +1,98 @@
+/*
+ * Copyright (c)
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *   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
+ *
+ *       http://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"
+	"math/rand"
+	"reflect"
+	"strconv"
+	"unsafe"
+)
+
+func mayPanic() {
+	panic("a problem")
+}
+
+func Source() string {
+	return "x" + strconv.Itoa(rand.Int())
+}
+
+func Sink(x string) {
+	fmt.Println(x)
+}
+
+type Example struct {
+	x int
+	f string
+}
+
+func (e Example) String() string {
+	return e.f + strconv.Itoa(e.x)
+}
+
+func usingUnsafe() {
+	s := Example{
+		x: 1243,
+		f: Source(),
+	}
+	x := []string{"a", "b", "c"}
+	var i uintptr
+	i = 3
+	// equivalent to f := unsafe.Pointer(&s.f)
+	f := unsafe.Pointer(uintptr(unsafe.Pointer(&s)) + unsafe.Offsetof(s.f))
+	// equivalent to e := unsafe.Pointer(&x[i])
+	e := unsafe.Pointer(uintptr(unsafe.Pointer(&x[0])) + i*unsafe.Sizeof(x[0]))
+
+	fmt.Println(e, f)
+}
+
+func usingReflect() {
+	x := 10
+	name := "Go Lang"
+	example := Example{1, Source()} // @Source(reflect)
+	fmt.Println(reflect.TypeOf(x))
+	fmt.Println(reflect.TypeOf(name))
+	fmt.Println(reflect.TypeOf(example))
+
+	method := reflect.ValueOf(example).MethodByName("String")
+	if !method.IsValid() {
+		fmt.Println("ERROR: String is not implemented")
+		return
+	}
+	e := method.Call(nil) // this call is elided by the pointer analysis and results in false negatives!
+	Sink(e[0].String())   // @Sink(reflect)
+}
+
+func usingRecover() {
+	defer func() {
+		if r := recover(); r != nil {
+
+			fmt.Println("Recovered. Error:\n", r)
+		}
+	}()
+	mayPanic()
+	fmt.Println("After mayPanic()")
+
+}
+
+func main() {
+	usingUnsafe()
+	usingReflect()
+	usingRecover()
+}

analysis/dataflow/unsound_features_test.go

@@ -0,0 +1,66 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+//
+// 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
+//
+//      http://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 dataflow_test
+
+import (
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/awslabs/ar-go-tools/analysis/config"
+	"github.com/awslabs/ar-go-tools/analysis/dataflow"
+	"github.com/awslabs/ar-go-tools/internal/analysistest"
+)
+
+func TestUnsoundFeatures(t *testing.T) {
+	dir := filepath.Join("testdata", "unsound-features")
+	lp, err := analysistest.LoadTest(testfsys, dir, []string{},
+		analysistest.LoadTestOptions{ApplyRewrite: true})
+	if err != nil {
+		t.Fatalf("failed to load test: %v", err)
+	}
+	c, err := dataflow.NewInitializedAnalyzerState(lp.Prog, lp.Pkgs, config.NewLogGroup(lp.Config), lp.Config)
+	if err != nil {
+		t.Errorf("error building state: %q", err)
+	}
+	var unsafeChecked, reflectChecked, recoverChecked bool
+	for f := range c.ReachableFunctions() {
+		if strings.Contains(f.Name(), "usingUnsafe") {
+
+			uf := dataflow.FindUnsoundFeatures(f)
+			if len(uf.UnsafeUsages) <= 2 {
+				t.Errorf("Did not detect enough usages of unsage in usingUnsafe")
+			}
+			unsafeChecked = true
+		}
+		if strings.Contains(f.Name(), "usingReflect") {
+			uf := dataflow.FindUnsoundFeatures(f)
+			if len(uf.ReflectUsages) <= 2 {
+				t.Errorf("Did not detect enough usages of unsage in usingReflect")
+			}
+			reflectChecked = true
+		}
+		if strings.Contains(f.Name(), "usingRecover$1") {
+			uf := dataflow.FindUnsoundFeatures(f)
+			if len(uf.Recovers) <= 0 {
+				t.Errorf("Did not detect enough usages of recover in usingRecover")
+			}
+			recoverChecked = true
+		}
+	}
+	if !(recoverChecked && reflectChecked && unsafeChecked) {
+		t.Errorf("Failed to check for recover, reflect and unsafe functions")
+	}
+}

analysis/summaries/standard_library.go

@@ -791,7 +791,12 @@ var summaryReflect = map[string]Summary{
 		[][]int{{0}, {1}},
 		[][]int{{0}, {0}},
 	},
-	"(reflect.Value).Bool":  SingleVarArgPropagation,
+	"(reflect.Value).Bool": SingleVarArgPropagation,
+	// Over-approximation for Call: it is assumed the function being called fully propagates data
+	"(reflect.Value).Call": {
+		[][]int{{0}, {1}},
+		[][]int{{0}, {0}},
+	},
 	"(reflect.Value).Float": SingleVarArgPropagation,
 	"(reflect.Value).Int":   SingleVarArgPropagation,
 	// func (v Value) Elem() Value
@@ -829,6 +834,11 @@ var summaryReflect = map[string]Summary{
 	"(reflect.Value).Kind": SingleVarArgPropagation,
 	// func (v Value) Len() int
 	"(reflect.Value).Len": SingleVarArgPropagation,
+	// func (v Value) MethodByName(name string) Value
+	"(reflect.Value).MethodByName": {
+		[][]int{{0}, {1}},
+		[][]int{{0}, {0}},
+	},
 	// func (v Value) NumField() int
 	"(reflect.Value).NumField": SingleVarArgPropagation,
 	// func (v Value) MapKeys() []Value

analysis/taint/testdata/basic/config.yaml

@@ -9,6 +9,9 @@ taint-tracking-problems:
     - package: "(basic)|(main)|(command-line-arguments)"
       type: "(chan \\*_S)|(chan _S)"
       kind: "channel receive"
+    - package: "(basic)|(main)|(command-line-arguments)"
+      type: "\\*Sample"
+      field: "Secret"
   sinks:
     - package: "(basic)|(main)|(command-line-arguments)"
       # Similarly, sinks are sink1 sink2 sink2 ...