Add log injection analysis (#5)

* Add log injection analysis
* Improve callgraph construction
* Add `(callgraph.Path).String` method
* Use `→` for `(callgraph.Edge).String`
* Add `(*callgraph.Graph).String` method
* Do NOT assume so much about anonymous functions

callgraph/callgraph.go

@@ -1,6 +1,7 @@
 package callgraph
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"go/token"
@@ -85,30 +86,48 @@ func New(root *ssa.Function, srcFns ...*ssa.Function) (*Graph, error) {
 					switch instrt := instr.(type) {
 					case *ssa.Call:
 						// debugf("found call instr")
-						fn, ok := instrt.Call.Value.(*ssa.Function)
-						if ok {
-							err := g.AddFunction(fn, allFns)
-							if err != nil {
-								return fmt.Errorf("failed to add src function %v from block instr: %w", fn, err)
+						var instrCall *ssa.Function
+
+						// Handle the case where the function calls a
+						// named function (*ssa.Function), and the case
+						// where the function calls an anonymous
+						// function (*ssa.MakeClosure).
+						switch callt := instrt.Call.Value.(type) {
+						case *ssa.Function:
+							// debugf("found call instr to function")
+							instrCall = callt
+						case *ssa.MakeClosure:
+							// debugf("found call instr to closure")
+							switch calltFn := callt.Fn.(type) {
+							case *ssa.Function:
+								instrCall = calltFn
 							}
 						}
 
+						// If we could not determine the function being
+						// called, skip this instruction.
+						if instrCall == nil {
+							continue
+						}
+
+						AddEdge(g.CreateNode(fn), instrt, g.CreateNode(instrCall))
+
+						err := g.AddFunction(instrCall, allFns)
+						if err != nil {
+							return fmt.Errorf("failed to add function %v from block instr: %w", instrCall, err)
+						}
+
 						// attempt to link function arguments that are functions
 						for a := 0; a < len(instrt.Call.Args); a++ {
 							arg := instrt.Call.Args[a]
 							switch argt := arg.(type) {
 							case *ssa.Function:
 								// TODO: check if edge already exists?
-								AddEdge(g.CreateNode(fn), instrt, g.CreateNode(argt))
+								AddEdge(g.CreateNode(instrCall), instrt, g.CreateNode(argt))
 							case *ssa.MakeClosure:
 								switch argtFn := argt.Fn.(type) {
 								case *ssa.Function:
-									AddEdge(g.CreateNode(fn), instrt, g.CreateNode(argtFn))
-
-									// Assumes the anonymous functions are called.
-									for _, anFn := range argtFn.AnonFuncs {
-										AddEdge(g.CreateNode(fn), instrt, g.CreateNode(anFn))
-									}
+									AddEdge(g.CreateNode(instrCall), instrt, g.CreateNode(argtFn))
 								}
 							}
 						}
@@ -208,6 +227,19 @@ func (g *Graph) CreateNode(fn *ssa.Function) *Node {
 	return n
 }
 
+func (g *Graph) String() string {
+	var buf bytes.Buffer
+
+	for _, n := range g.Nodes {
+		fmt.Fprintf(&buf, "%s\n", n)
+		for _, e := range n.Out {
+			fmt.Fprintf(&buf, "\t→ %s\n", e.Callee)
+		}
+		fmt.Fprintf(&buf, "\n")
+	}
+	return buf.String()
+}
+
 // A Node represents a node in a call graph.
 type Node struct {
 	sync.RWMutex
@@ -232,7 +264,7 @@ type Edge struct {
 }
 
 func (e Edge) String() string {
-	return fmt.Sprintf("%s --> %s", e.Caller, e.Callee)
+	return fmt.Sprintf("%s → %s", e.Caller, e.Callee)
 }
 
 func (e Edge) Description() string {
@@ -343,6 +375,24 @@ func (p Path) Last() *Edge {
 	return p[len(p)-1]
 }
 
+// String returns a string representation of the path which
+// is a sequence of edges separated by " → ".
+//
+// Intended to be used while debugging.
+func (p Path) String() string {
+	var buf bytes.Buffer
+	for i, e := range p {
+		if i == 0 {
+			buf.WriteString(e.Caller.String())
+		}
+
+		buf.WriteString(" → ")
+
+		buf.WriteString(e.Callee.String())
+	}
+	return buf.String()
+}
+
 type Paths []Path
 
 func PathSearch(start *Node, isEnd func(*Node) bool) Path {

cmd/logi/main.go

@@ -0,0 +1,10 @@
+package main
+
+import (
+	"github.com/picatz/taint/log/injection"
+	"golang.org/x/tools/go/analysis/singlechecker"
+)
+
+func main() {
+	singlechecker.Main(injection.Analyzer)
+}

log/injection/injection.go

@@ -0,0 +1,122 @@
+package injection
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/picatz/taint"
+	"github.com/picatz/taint/callgraph"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/buildssa"
+)
+
+var userControlledValues = taint.NewSources(
+	"*net/http.Request",
+)
+
+var injectableLogFunctions = taint.NewSinks(
+	// Note: at this time, they *must* be a function or method.
+	"log.Fatal",
+	"log.Fatalf",
+	"log.Fatalln",
+	"log.Panic",
+	"log.Panicf",
+	"log.Panicln",
+	"log.Print",
+	"log.Printf",
+	"log.Println",
+	"log.Output",
+	"log.SetOutput",
+	"log.SetPrefix",
+	"log.Writer",
+	"(*log.Logger).Fatal",
+	"(*log.Logger).Fatalf",
+	"(*log.Logger).Fatalln",
+	"(*log.Logger).Panic",
+	"(*log.Logger).Panicf",
+	"(*log.Logger).Panicln",
+	"(*log.Logger).Print",
+	"(*log.Logger).Printf",
+	"(*log.Logger).Println",
+	"(*log.Logger).Output",
+	"(*log.Logger).SetOutput",
+	"(*log.Logger).SetPrefix",
+	"(*log.Logger).Writer",
+	// TODO: consider adding the following logger packages,
+	//       and the ability to configure this list generically.
+	//
+	// https://pkg.go.dev/golang.org/x/exp/slog
+	// https://pkg.go.dev/github.com/golang/glog
+	// https://pkg.go.dev/github.com/hashicorp/go-hclog
+	// https://pkg.go.dev/github.com/sirupsen/logrus
+	// https://pkg.go.dev/go.uber.org/zap
+	// ...
+)
+
+// Analyzer finds potential log injection issues to demonstrate
+// the github.com/picatz/taint package.
+var Analyzer = &analysis.Analyzer{
+	Name:     "logi",
+	Doc:      "finds potential log injection issues",
+	Run:      run,
+	Requires: []*analysis.Analyzer{buildssa.Analyzer},
+}
+
+// imports returns true if the package imports any of the given packages.
+func imports(pass *analysis.Pass, pkgs ...string) bool {
+	var imported bool
+	for _, imp := range pass.Pkg.Imports() {
+		for _, pkg := range pkgs {
+			if strings.HasSuffix(imp.Path(), pkg) {
+				imported = true
+				break
+			}
+		}
+		if imported {
+			break
+		}
+	}
+	return imported
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	// Require the log package is imported in the
+	// program being analyzed before running the analysis.
+	//
+	// This prevents wasting time analyzing programs that don't log.
+	if !imports(pass, "log") {
+		return nil, nil
+	}
+
+	// Get the built SSA IR.
+	buildSSA := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
+
+	// Identify the main function from the package's SSA IR.
+	mainFn := buildSSA.Pkg.Func("main")
+	if mainFn == nil {
+		return nil, nil
+	}
+
+	// Construct a callgraph, using the main function as the root,
+	// constructed of all other functions. This returns a callgraph
+	// we can use to identify directed paths to logging functions.
+	cg, err := callgraph.New(mainFn, buildSSA.SrcFuncs...)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create new callgraph: %w", err)
+	}
+
+	// Run taint check for user controlled values (sources) ending
+	// up in injectable log functions (sinks).
+	results := taint.Check(cg, userControlledValues, injectableLogFunctions)
+
+	// For each result, check if a prepared statement is providing
+	// a mitigation for the user controlled value.
+	//
+	// TODO: ensure this makes sense for all the GORM usage?
+	for _, result := range results {
+		pass.Reportf(result.SinkValue.Pos(), "potential log injection")
+	}
+
+	return nil, nil
+}

log/injection/injection_test.go

@@ -0,0 +1,21 @@
+package injection
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+var testdata = analysistest.TestData()
+
+func TestA(t *testing.T) {
+	analysistest.Run(t, testdata, Analyzer, "a")
+}
+
+func TestB(t *testing.T) {
+	analysistest.Run(t, testdata, Analyzer, "b")
+}
+
+func TestC(t *testing.T) {
+	analysistest.Run(t, testdata, Analyzer, "c")
+}

log/injection/testdata/src/a/main.go

@@ -0,0 +1,14 @@
+package main
+
+import (
+	"log"
+	"net/http"
+)
+
+func main() {
+	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		log.Println(r.URL.Query().Get("input")) // want "potential log injection"
+	})
+
+	http.ListenAndServe(":8080", nil)
+}

log/injection/testdata/src/b/main.go

@@ -0,0 +1,25 @@
+package main
+
+import (
+	"log"
+	"net/http"
+)
+
+func l(input string) {
+	l := log.New(nil, "", 0)
+	l.Println(input) // want "potential log injection"
+}
+
+func buisness(input string) {
+	l(input)
+}
+
+func main() {
+	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		input := r.URL.Query().Get("input")
+
+		buisness(input)
+	})
+
+	http.ListenAndServe(":8080", nil)
+}

log/injection/testdata/src/c/main.go

@@ -0,0 +1,28 @@
+package main
+
+import (
+	"log"
+	"net/http"
+)
+
+func l(input string) {
+	log.Println(input) // want "potential log injection"
+}
+
+func buisness(input string) {
+	l(input)
+}
+
+func main() {
+	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		input := r.URL.Query().Get("input")
+
+		f := func() {
+			buisness(input)
+		}
+
+		f()
+	})
+
+	http.ListenAndServe(":8080", nil)
+}