fatal: concurrent map read/write in readfile.Match during parallel analysis

[deliri]

gosec v2.24.7: concurrent map crash in rules.(*readfile).Match

Summary

gosec crashes with fatal error: concurrent map read and map write when
analyzing packages concurrently. The crash occurs in the readfile rule
(G304) because two plain maps on the readfile struct are accessed from
multiple goroutines without synchronization.

Version

• gosec: v2.24.7 (latest release, March 1 2026)
• Go: go1.26.1
• OS: darwin/arm64

Reproduction

The crash is non-deterministic. It triggers under concurrent package analysis
when multiple goroutines invoke readfile.Match / readfile.trackCleanAssign /
readfile.trackJoinAssignStmt simultaneously.

gosec -fmt=golint -exclude=G204,G301,G302,G306 -tests ./...

Running on a large Go project (~157 files, 48K lines) with -tests flag causes
gosec to analyze enough packages concurrently to trigger the race. The crash
reproduces intermittently — roughly 1 in 3 runs on this codebase.

Root Cause

In
rules/readfile.go,
the readfile struct contains two unsynchronized maps:

type readfile struct {
    callListRule
    pathJoin gosec.CallList
    clean    gosec.CallList

    cleanedVar map[*types.Var]ast.Node  // <-- unsynchronized
    joinedVar  map[*types.Var]ast.Node  // <-- unsynchronized
}

These maps are written by:

• trackCleanAssign() — writes to cleanedVar
• trackJoinAssignStmt() — writes to joinedVar

And read by:

• isFilepathClean() — reads from cleanedVar
• isSafeJoin() — likely reads from joinedVar
• Match() — calls all of the above

The Analyzer.Process method dispatches rule checking via errgroup.Go (see
analyzer.go), which calls checkRules in separate goroutines. Since all
goroutines share the same readfile rule instance, concurrent map reads and
writes race against each other.

Stack Trace

fatal error: concurrent map read and map write

goroutine 31 [running]:
internal/runtime/maps.fatal({0x103437fa3?, 0x19?})
github.com/securego/gosec/v2/rules.(*readfile).Match(0x1ef517238de0, {0x10456e330, 0x1ef51b64c2c0}, 0x1ef51cf54000)
github.com/securego/gosec/v2.(*astVisitor).Visit(0x1ef5187708c0, {0x10456e330, 0x1ef51b64c2c0})
go/ast.Walk({0x104564820?, 0x1ef5187708c0?}, {0x10456e330, 0x1ef51b64c2c0})
go/ast.walkList[...](...)
go/ast.Walk({0x104564820?, 0x1ef5187708c0?}, {0x10456e308, 0x1ef51b64c300})
go/ast.walkList[...](...)
go/ast.Walk({0x104564820?, 0x1ef5187708c0?}, {0x10456f060, 0x1ef51dcb55f0})
go/ast.Walk({0x104564820?, 0x1ef5187708c0?}, {0x10456e6a0, 0x1ef51dcb5650})
go/ast.walkList[...](...)
go/ast.Walk({0x104564820?, 0x1ef5187708c0?}, {0x10456e3f8, 0x1ef5202c5540})
github.com/securego/gosec/v2.(*Analyzer).checkRules(0x1ef51762e5b0, 0x1ef517b27ba0)
github.com/securego/gosec/v2.(*Analyzer).Process.func1()
golang.org/x/sync/errgroup.(*Group).Go.func1()
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 1

Suggested Fix

Option A — use sync.Map for cleanedVar and joinedVar:

type readfile struct {
    callListRule
    pathJoin gosec.CallList
    clean    gosec.CallList
    cleanedVar sync.Map // *types.Var -> ast.Node
    joinedVar  sync.Map // *types.Var -> ast.Node
}

Option B — protect both maps with a sync.Mutex:

type readfile struct {
    callListRule
    pathJoin gosec.CallList
    clean    gosec.CallList
    mu         sync.Mutex
    cleanedVar map[*types.Var]ast.Node
    joinedVar  map[*types.Var]ast.Node
}

Option C — create a fresh readfile instance per goroutine in checkRules
instead of sharing the same rule instance across all concurrent walkers.

Impact

• gosec exits with a fatal runtime error, reporting 0 actual findings
• The exit code is non-zero, which causes CI pipelines to fail
• The crash is non-deterministic — it passes on smaller runs and fails on larger
  concurrent analyses, making it difficult to diagnose

[ravisastryk]

Hey @deliri . Thank you for reporting this issue. The stack trace, reproduction steps, and option breakdown made this straightforward to confirm and act on.

Your Option C is the right instinct and is exactly the approach taken here. The only refinement from what you described is the granularity and mechanism:

• You suggested creating a fresh readfile instance per goroutine inside checkRules. The issue with goroutine granularity is that a single worker processes multiple packages sequentially, so you'd still accumulate cleanedVar/joinedVar state across packages within one worker. The correct boundary is per package walk, not per goroutine.

• The mechanism: rather than reconstructing readfile in isolation (which would duplicate constructor logic and leak implementation details into analyzer.go), we store the RuleBuilder functions that are already passed to LoadRules and re-invoke them once per checkRules call. This produces a fully fresh RuleSet — with fresh rule instances, fresh maps, fresh everything — for each package, using infrastructure that already exists.

The result is that readfile.go needs zero changes. The fix is entirely in analyzer.go, in the orchestration layer, which also means every future rule that carries per-package state gets the same isolation automatically — no rule author ever needs to think about concurrency.

Options A (sync.Map) and B (sync.Mutex) both fix the crash but leave the underlying structural issue open — the next stateful rule hits the same problem. Option C, taken to its logical conclusion, prevents that class of bug entirely.

Patch attached here. Only analyzer.go changes.

[ccojocar]

Fixed by #1589