Skip to content

G706 taint analysis hangs on isFieldTaintedOnValue Phi-node recursion #1624

@cplieger

Description

@cplieger

Summary

isFieldTaintedOnValue in taint/taint.go (line 1039) lacks a visited check, causing unbounded recursion through SSA Phi-node cycles. The maxTaintDepth (50) limit is insufficient because the recursion fans out exponentially across Phi edges.

Version

gosec v2.25.0 (latest release, also tested via golangci-lint master at commit 83284fe)

Reproduction

Any Go module with:

  • log/slog calls (G706 sinks)
  • Moderate complexity (many struct types, interface-driven architecture)
  • Loop constructs that produce SSA Phi nodes with back-edges

Minimal trigger: a Go module with ~9K lines, 194 struct types, and 360 slog.* calls. gosec runs indefinitely at 100% CPU. Other modules in the same project (without the numeric/DSP code) complete in under 2 seconds.

go install github.com/securego/gosec/v2/cmd/gosec@v2.25.0
gosec -include=G706 ./...   # hangs indefinitely
gosec -exclude=G706 ./...   # completes in seconds

Root Cause

isFieldTaintedOnValue (taint/taint.go:1039) does not check or update the visited map before recursing. Compare with isTainted (taint/taint.go:524) which correctly checks visited[v] and sets visited[v] = true before proceeding.

The *ssa.Phi case at line 1061 iterates edges and recurses:

case *ssa.Phi:
    for _, edge := range val.Edges {
        if a.isFieldTaintedOnValue(edge, fieldIdx, fn, visited, depth+1) {
            return true
        }
    }

When a Phi edge leads back to a value already in the recursion chain (common in loops), the function recurses up to maxTaintDepth (50) on each path. With multiple Phi nodes forming a cycle, this creates exponential fan-out: each Phi with N edges multiplies the work by N at every level.

Stack Trace Evidence

Obtained via GOTRACEBACK=crash + SIGQUIT after 35 seconds of 100% CPU:

goroutine 3045 [runnable]:
runtime.typehash(...)
  /usr/local/go/src/runtime/alg.go:215
runtime.interhash(...)
  /usr/local/go/src/runtime/alg.go:161
runtime.mapaccess1(...)
  /usr/local/go/src/internal/runtime/maps/runtime.go:81

github.com/securego/gosec/v2/taint.(*Analyzer).isTainted(...)
  taint/taint.go:535
github.com/securego/gosec/v2/taint.(*Analyzer).isFieldTaintedOnValue(...)
  taint/taint.go:1061   ← depth=0x32 (50)
github.com/securego/gosec/v2/taint.(*Analyzer).isFieldTaintedOnValue(...)
  taint/taint.go:1061   ← depth=0x31 (49)
  ... (45 identical frames, decrementing depth from 0x32 to 0x5) ...
github.com/securego/gosec/v2/taint.(*Analyzer).isFieldTaintedOnValue(...)
  taint/taint.go:1061   ← depth=0x5

github.com/securego/gosec/v2/taint.(*Analyzer).isFieldAccessTainted(...)
  taint/taint.go:1022
github.com/securego/gosec/v2/taint.(*Analyzer).isTainted(...)
  taint/taint.go:648
github.com/securego/gosec/v2/taint.(*Analyzer).isTainted(...)
  taint/taint.go:656
github.com/securego/gosec/v2/taint.(*Analyzer).isTainted(...)
  taint/taint.go:680
github.com/securego/gosec/v2/taint.(*Analyzer).isTainted(...)
  taint/taint.go:708
github.com/securego/gosec/v2/taint.(*Analyzer).isTainted(...)
  taint/taint.go:684
github.com/securego/gosec/v2/taint.(*Analyzer).analyzeFunctionSinks(...)
  taint/taint.go:383
github.com/securego/gosec/v2/taint.(*Analyzer).Analyze(...)
  taint/taint.go:331
github.com/securego/gosec/v2/analyzers.newLogInjectionAnalyzer.NewGosecAnalyzer.makeAnalyzerRunner.func1(...)
  taint/analyzer.go:62

Key observations:

  • All 45 recursive frames are isFieldTaintedOnValue at line 1061 (the Phi edge loop)
  • The same SSA value address (0x27d48233c690) appears in every frame
  • Depth counts down from 0x32 (50) to 0x5, confirming it hits maxTaintDepth per path but the fan-out across edges creates exponential total work

Suggested Fix

Add a visited check at the top of isFieldTaintedOnValue, matching the pattern used in isTainted:

func (a *Analyzer) isFieldTaintedOnValue(v ssa.Value, fieldIdx int, fn *ssa.Function, visited map[ssa.Value]bool, depth int) bool {
    if v == nil || depth > maxTaintDepth {
        return false
    }
+   if visited[v] {
+       return false
+   }
+   visited[v] = true

    switch val := v.(type) {
    // ...

Note: this may need a composite key (v, fieldIdx) instead of just v if different field indices on the same value should be analyzed independently. But even visited[v] alone would prevent the infinite recursion.

Workaround

Exclude G706 at the gosec level in .golangci.yaml:

gosec:
  excludes:
    - G706

Text-based exclusion rules (- text: "G706:") do NOT prevent the hang because they only suppress reporting after analysis completes.

Environment

  • Go 1.26.1, linux/amd64 (WSL2 on Windows)
  • gosec v2.25.0
  • golangci-lint 2.11.3 (also reproduced with golangci-lint built from master)
  • Triggering module: ~9K lines Go, 194 struct types, 360 slog calls, interface-driven architecture with numeric DSP code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions