fix(taint): prioritize call-graph over type-based source matching

taint/taint.go

@@ -858,67 +858,67 @@ func (a *Analyzer) isParameterTainted(param *ssa.Parameter, fn *ssa.Function, vi
 		}
 	}
 
-	// Check if parameter type is a source type.
-	// This is the ONLY place where type-based source matching should trigger
-	// automatic taint — because parameters represent data flowing IN from
-	// external callers we don't control.
-	if a.isSourceType(param.Type()) {
-		if paramIdx >= 0 && a.paramTaintCache != nil {
-			a.paramTaintCache[paramKey{fn: fn, paramIdx: paramIdx}] = true
-		}
-		return true
-	}
-
-	// Use call graph to find callers and check their arguments
-	if a.callGraph == nil {
-		return false
-	}
+	// ── Step 1: Call-graph check (most precise) ──────────────────────────
+	// If we can see ALL callers and none pass tainted data, the parameter
+	// is safe — even if its type is a source type (e.g. *http.Request in a
+	// wrapper that only receives requests built from hardcoded URLs).
+	callGraphDefinitive := false
+
+	if a.callGraph != nil && paramIdx >= 0 {
+		node := a.callGraph.Nodes[fn]
+		if node != nil && len(node.In) > 0 {
+			hasReceiver := fn.Signature.Recv() != nil
+
+			edgesChecked := 0
+			capped := false
+			for _, inEdge := range node.In {
+				if edgesChecked >= maxCallerEdges {
+					capped = true
+					break
+				}
 
-	node := a.callGraph.Nodes[fn]
-	if node == nil {
-		return false
-	}
+				site := inEdge.Site
+				if site == nil {
+					continue
+				}
 
-	if paramIdx < 0 {
-		return false
-	}
+				// For interface invocations (IsInvoke) the receiver is in
+				// Call.Value, not in Args, so argument indices are shifted
+				// by -1 compared to static method calls.
+				idx := paramIdx
+				if hasReceiver && site.Common().IsInvoke() {
+					idx = paramIdx - 1
+				}
 
-	// Compute the adjusted index ONCE outside the loop.
-	adjustedIdx := paramIdx
-	if fn.Signature.Recv() != nil {
-		// In SSA, method parameters include the receiver at index 0.
-		// fn.Params already includes the receiver, so paramIdx is correct
-		// relative to fn.Params. But call site Args also include the receiver
-		// at index 0 for bound methods. So we don't need to adjust—the
-		// indices are already aligned.
-		// However, for interface method invocations (IsInvoke), the receiver
-		// is in Call.Value, not Args. We handle that separately below.
-		adjustedIdx = paramIdx
-	}
-
-	// Check each caller, capping at maxCallerEdges to avoid combinatorial
-	// explosion from CHA over-approximation of interface method calls.
-	edgesChecked := 0
-	for _, inEdge := range node.In {
-		if edgesChecked >= maxCallerEdges {
-			break
-		}
+				callArgs := site.Common().Args
+				if idx >= 0 && idx < len(callArgs) {
+					edgesChecked++
+					if a.isTainted(callArgs[idx], inEdge.Caller.Func, visited, depth+1) {
+						if a.paramTaintCache != nil {
+							a.paramTaintCache[paramKey{fn: fn, paramIdx: paramIdx}] = true
+						}
+						return true
+					}
+				}
+			}
 
-		site := inEdge.Site
-		if site == nil {
-			continue
+			// If we inspected every valid edge without hitting the cap,
+			// the call graph gives a definitive answer — skip type fallback.
+			callGraphDefinitive = !capped && edgesChecked > 0
 		}
+	}
 
-		callArgs := site.Common().Args
-
-		if adjustedIdx < len(callArgs) {
-			edgesChecked++
-			if a.isTainted(callArgs[adjustedIdx], inEdge.Caller.Func, visited, depth+1) {
-				if a.paramTaintCache != nil {
-					a.paramTaintCache[paramKey{fn: fn, paramIdx: paramIdx}] = true
-				}
-				return true
+	// ── Step 2: Type-based fallback ──────────────────────────────────────
+	// Only apply when the call graph could NOT give a definitive answer:
+	//  - No call graph / no node / no incoming edges (framework-registered
+	//    handlers like http.HandleFunc have no visible callers)
+	//  - We hit the maxCallerEdges cap (incomplete picture)
+	if !callGraphDefinitive {
+		if a.isSourceType(param.Type()) {
+			if paramIdx >= 0 && a.paramTaintCache != nil {
+				a.paramTaintCache[paramKey{fn: fn, paramIdx: paramIdx}] = true
 			}
+			return true
 		}
 	}
 

testutils/g701_samples.go

@@ -1873,8 +1873,8 @@ execute:
 }
 `}, 1, gosec.NewConfig()},
 
-	// Interprocedural with interface implementation
-	// NOTE: Interface method taint tracking not yet fully supported - documented limitation
+	// Interprocedural with interface implementation: taint flows through
+	// interface method call to the concrete implementation's sink.
 	{[]string{`
 package main
 
@@ -1900,7 +1900,7 @@ func handler(db *sql.DB, r *http.Request) {
 	var executor QueryExecutor = &SimpleExecutor{}
 	executor.Execute(db, query)
 }
-`}, 0, gosec.NewConfig()},
+`}, 1, gosec.NewConfig()},
 
 	// Multiple Phi nodes with complex control flow
 	{[]string{`

testutils/g704_samples.go

@@ -101,4 +101,74 @@ func handler(r *http.Request) {
 	http.Get(target) //nolint:errcheck
 }
 `}, 1, gosec.NewConfig()},
+	// Wrapper method with hardcoded URL must NOT trigger G704.
+	// The *http.Request parameter in the wrapper is safe because
+	// all callers pass a request built from a constant URL.
+	{[]string{`
+package main
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+)
+
+type HTTPDoer interface {
+	Do(req *http.Request) (*http.Response, error)
+}
+
+type NamedClient struct {
+	HTTPClient *http.Client
+}
+
+func (c *NamedClient) Do(req *http.Request) (*http.Response, error) {
+	req.Header.Set("User-Agent", "test-agent")
+	return c.HTTPClient.Do(req)
+}
+
+func doImport(httpDoer HTTPDoer) error {
+	ctx := context.Background()
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, "/import", http.NoBody)
+	if err != nil {
+		return fmt.Errorf("creating import POST: %w", err)
+	}
+	resp, err := httpDoer.Do(req)
+	if err != nil {
+		return fmt.Errorf("performing import POST: %w", err)
+	}
+	defer resp.Body.Close()
+	return nil
+}
+
+func main() {
+	client := &NamedClient{HTTPClient: http.DefaultClient}
+	_ = doImport(client)
+}
+`}, 0, gosec.NewConfig()},
+	// Wrapper method with tainted URL MUST trigger G704.
+	// Two sinks fire: NewRequest (tainted URL arg) and the inner Client.Do
+	// (tainted request flows through the call graph to the wrapper's parameter).
+	{[]string{`
+package main
+
+import (
+	"net/http"
+	"os"
+)
+
+type APIClient struct {
+	HTTPClient *http.Client
+}
+
+func (c *APIClient) Do(req *http.Request) (*http.Response, error) {
+	return c.HTTPClient.Do(req)
+}
+
+func main() {
+	client := &APIClient{HTTPClient: http.DefaultClient}
+	url := os.Getenv("TARGET_URL")
+	req, _ := http.NewRequest(http.MethodGet, url, nil)
+	client.Do(req)
+}
+`}, 2, gosec.NewConfig()},
 }