fix: taint analysis false positives with G703,G705 (#1522)

* fix: taint analysis false positives with G703,G705

* additional tests

* cross package coverage increase

* Add additonal G118 tests for codecov

* improve code coverage

* field-level taint tracking and test coverage

* add more tests

* improve test coverage

* fix unnecessary nosec comments for tool

* improve code coverage

* address codecov issues

* improve code coverage

Author: ravisastryk

Makefile

@@ -57,7 +57,7 @@ govulncheck: install-govulncheck
 	fi
 
 test-coverage:
-	go test -race -v -count=1 -coverprofile=coverage.out ./...
+	go test -race -v -count=1 -coverpkg=./... -coverprofile=coverage.out ./...
 
 build:
 	go build $(LDFLAGS) -o $(BIN) ./cmd/gosec/

analyzers/context_propagation.go

@@ -234,7 +234,7 @@ func (s *contextPropagationState) detectLostCancel(fn *ssa.Function) {
 				continue
 			}
 
-			if !isCancelCalled(cancelValue) {
+			if !isCancelCalled(cancelValue, s.ssaFuncs) {
 				s.addIssue(instr.Pos(), msgLostCancel, issue.Medium, issue.High)
 			}
 		}
@@ -673,7 +673,7 @@ func isCancelFuncType(t types.Type) bool {
 	return true
 }
 
-func isCancelCalled(cancelValue ssa.Value) bool {
+func isCancelCalled(cancelValue ssa.Value, allFuncs []*ssa.Function) bool {
 	if cancelValue == nil {
 		return false
 	}
@@ -699,6 +699,13 @@ func isCancelCalled(cancelValue ssa.Value) bool {
 				if r.Val != current {
 					continue
 				}
+				// Check if storing to a struct field — if so, search other
+				// methods of the same type for loads of that field + call.
+				if fa, ok := r.Addr.(*ssa.FieldAddr); ok {
+					if isCancelCalledViaStructField(fa, allFuncs) {
+						return true
+					}
+				}
 				queue = append(queue, r.Addr)
 			case *ssa.UnOp:
 				if r.Op == token.MUL && r.X == current {
@@ -725,6 +732,136 @@ func isCancelCalled(cancelValue ssa.Value) bool {
 	return false
 }
 
+// isCancelCalledViaStructField checks whether a cancel function stored into a
+// struct field (e.g., job.cancelFn = cancel) is subsequently called in any other
+// method of the same receiver type (e.g., job.Close() calls job.cancelFn()).
+func isCancelCalledViaStructField(storeFA *ssa.FieldAddr, allFuncs []*ssa.Function) bool {
+	// Get the field index and the receiver pointer type
+	fieldIdx := storeFA.Field
+	structPtrType := storeFA.X.Type()
+
+	for _, fn := range allFuncs {
+		if fn == nil || fn.Blocks == nil {
+			continue
+		}
+		// Only check methods on the same receiver type
+		if fn.Signature == nil || fn.Signature.Recv() == nil {
+			continue
+		}
+		if !types.Identical(fn.Signature.Recv().Type(), structPtrType) {
+			continue
+		}
+
+		// Look for a load of the same field followed by a call
+		for _, block := range fn.Blocks {
+			for _, instr := range block.Instrs {
+				fa, ok := instr.(*ssa.FieldAddr)
+				if !ok || fa.Field != fieldIdx {
+					continue
+				}
+				// Check that this FieldAddr is on the receiver (Params[0])
+				if len(fn.Params) == 0 {
+					continue
+				}
+				if !reachesParam(fa.X, fn.Params[0]) {
+					continue
+				}
+				// Check if the value loaded from this field is eventually called
+				if isFieldValueCalled(fa) {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}
+
+// reachesParam checks if a value traces back to the given parameter,
+// following through pointer dereferences and phi nodes.
+func reachesParam(v ssa.Value, param *ssa.Parameter) bool {
+	seen := make(map[ssa.Value]bool)
+	return reachesParamImpl(v, param, seen)
+}
+
+func reachesParamImpl(v ssa.Value, param *ssa.Parameter, seen map[ssa.Value]bool) bool {
+	if v == nil || seen[v] {
+		return false
+	}
+	seen[v] = true
+
+	if v == param {
+		return true
+	}
+	switch val := v.(type) {
+	case *ssa.UnOp:
+		return reachesParamImpl(val.X, param, seen)
+	case *ssa.Phi:
+		for _, e := range val.Edges {
+			if reachesParamImpl(e, param, seen) {
+				return true
+			}
+		}
+	case *ssa.FieldAddr:
+		return reachesParamImpl(val.X, param, seen)
+	}
+	return false
+}
+
+// isFieldValueCalled checks if the value loaded from a FieldAddr is eventually
+// used as a callee (i.e., the loaded function pointer is called).
+func isFieldValueCalled(fa *ssa.FieldAddr) bool {
+	refs := fa.Referrers()
+	if refs == nil {
+		return false
+	}
+	for _, ref := range *refs {
+		// Look for a load (UnOp MUL = pointer dereference)
+		unop, ok := ref.(*ssa.UnOp)
+		if !ok || unop.Op != token.MUL {
+			continue
+		}
+		// Check if the loaded value is called
+		loadRefs := unop.Referrers()
+		if loadRefs == nil {
+			continue
+		}
+		queue := []ssa.Value{unop}
+		visited := make(map[ssa.Value]bool)
+		for len(queue) > 0 {
+			cur := queue[0]
+			queue = queue[1:]
+			if cur == nil || visited[cur] {
+				continue
+			}
+			visited[cur] = true
+			curRefs := cur.Referrers()
+			if curRefs == nil {
+				continue
+			}
+			for _, r := range *curRefs {
+				switch rr := r.(type) {
+				case ssa.CallInstruction:
+					if isUsedInCall(rr.Common(), cur) {
+						return true
+					}
+				case *ssa.Phi:
+					queue = append(queue, rr)
+				case *ssa.Store:
+					// stored then loaded elsewhere — follow addr
+					if rr.Val == cur {
+						queue = append(queue, rr.Addr)
+					}
+				case *ssa.UnOp:
+					if rr.X == cur {
+						queue = append(queue, rr)
+					}
+				}
+			}
+		}
+	}
+	return false
+}
+
 func isUsedInCall(common *ssa.CallCommon, target ssa.Value) bool {
 	if common == nil || target == nil {
 		return false

analyzers/loginjection.go

@@ -58,6 +58,21 @@ func LogInjection() taint.Config {
 			{Package: "strconv", Method: "Quote"},
 			// url.QueryEscape encodes special characters
 			{Package: "net/url", Method: "QueryEscape"},
+
+			// JSON encoding escapes all special characters including newlines,
+			// producing structurally safe output for log entries.
+			{Package: "encoding/json", Method: "Marshal"},
+			{Package: "encoding/json", Method: "MarshalIndent"},
+
+			// Numeric conversions produce strings that cannot contain
+			// log injection characters (newlines, carriage returns).
+			{Package: "strconv", Method: "Atoi"},
+			{Package: "strconv", Method: "Itoa"},
+			{Package: "strconv", Method: "ParseInt"},
+			{Package: "strconv", Method: "ParseUint"},
+			{Package: "strconv", Method: "ParseFloat"},
+			{Package: "strconv", Method: "FormatInt"},
+			{Package: "strconv", Method: "FormatFloat"},
 		},
 	}
 }

analyzers/pathtraversal.go

@@ -71,6 +71,20 @@ func PathTraversal() taint.Config {
 			{Package: "path/filepath", Method: "Rel"},
 			// url.PathEscape escapes path components
 			{Package: "net/url", Method: "PathEscape"},
+
+			// path.Base and path.Clean provide identical traversal-stripping
+			// semantics as their filepath counterparts (the only difference is
+			// separator handling, which is irrelevant for security).
+			{Package: "path", Method: "Base"},
+			{Package: "path", Method: "Clean"},
+
+			// Integer conversions eliminate path traversal vectors entirely —
+			// the result can never contain "/" or ".." characters.
+			{Package: "strconv", Method: "Atoi"},
+			{Package: "strconv", Method: "ParseInt"},
+			{Package: "strconv", Method: "ParseUint"},
+			{Package: "strconv", Method: "ParseFloat"},
+			{Package: "strconv", Method: "ParseBool"},
 		},
 	}
 }

analyzers/xss.go

@@ -58,6 +58,23 @@ func XSS() taint.Config {
 			// url.QueryEscape for URL parameter escaping
 			{Package: "net/url", Method: "QueryEscape"},
 			{Package: "net/url", Method: "PathEscape"},
+
+			// JSON encoding produces structurally safe output that cannot
+			// contain unescaped HTML tags or script injections. The output
+			// is served as application/json, not text/html.
+			{Package: "encoding/json", Method: "Marshal"},
+			{Package: "encoding/json", Method: "MarshalIndent"},
+
+			// Integer/float conversions produce numeric strings that cannot
+			// contain XSS payloads.
+			{Package: "strconv", Method: "Atoi"},
+			{Package: "strconv", Method: "Itoa"},
+			{Package: "strconv", Method: "ParseInt"},
+			{Package: "strconv", Method: "ParseUint"},
+			{Package: "strconv", Method: "ParseFloat"},
+			{Package: "strconv", Method: "FormatInt"},
+			{Package: "strconv", Method: "FormatUint"},
+			{Package: "strconv", Method: "FormatFloat"},
 		},
 	}
 }

cmd/gosecutil/tools.go

@@ -76,13 +76,11 @@ func (u *utilities) run(args ...string) {
 func shouldSkip(path string) bool {
 	st, e := os.Stat(path) // #nosec G703 -- gosecutil intentionally inspects user-supplied local file paths
 	if e != nil {
-		//#nosec
-		fmt.Fprintf(os.Stderr, "Skipping: %s - %s\n", path, e)
+		fmt.Fprintf(os.Stderr, "Skipping: %s - %s\n", path, e) //#nosec G705
 		return true
 	}
 	if st.IsDir() {
-		//#nosec
-		fmt.Fprintf(os.Stderr, "Skipping: %s - directory\n", path)
+		fmt.Fprintf(os.Stderr, "Skipping: %s - directory\n", path) //#nosec G705
 		return true
 	}
 	return false
@@ -135,7 +133,9 @@ func createContext(filename string) *context {
 		Scopes:     make(map[ast.Node]*types.Scope),
 		Implicits:  make(map[ast.Node]types.Object),
 	}
-	config := types.Config{Importer: importer.Default()}
+	// Use ForCompiler with "source" for more reliable import resolution
+	// This reads from source files instead of relying on compiled packages
+	config := types.Config{Importer: importer.ForCompiler(fileset, "source", nil)}
 	pkg, e := config.Check("main.go", fileset, []*ast.File{root}, info)
 	if e != nil {
 		//#nosec