* Port G120 from SSA-based to taint analysis
Fix#1600: G120 now detects ParseMultipartForm across function boundaries
using the taint engine's interprocedural call graph analysis.
Fix#1603: Remove ParseForm, FormValue, and PostFormValue from G120 sinks.
These methods already enforce a built-in 10 MiB body limit in Go's
standard library, so flagging them was a false positive. Only
ParseMultipartForm (genuinely unbounded without MaxBytesReader) is now
flagged.
Changes:
- Replace the 521-line custom SSA analyzer in form_parsing_limits.go with
a ~55-line taint analysis configuration.
- Extract the shared dependencyChecker (used by G119, G121, G122) into
its own file dependency_checker.go.
- Add FormParsingLimitRule (CWE-400) to the taint rule registry.
- Rewrite test samples to cover the new behavior including interprocedural
detection and the built-in limit exclusions.
* Update the RULES.md to be consistent with the implementation
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
---------
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* fix(G118): eliminate false positive for package-level cancel variables
G118 was incorrectly reporting context cancellation function not called
when the cancel function was assigned to a package-level variable (e.g.,
in init()) and called in a separate function (e.g., signal handler).
Root cause: isCancelCalled() lacked special handling for *ssa.Global
(package-level variables), causing cross-function tracking to fail.
Solution: Add dedicated tracking for package-level globals, similar to
the struct field handling added in PR #1596. The fix includes:
- Check in *ssa.Store case to detect global variable assignments
- isGlobalCalledInAnyFunc() helper to search all functions for calls
- isValueCalled() generalized helper for BFS value tracking
* additional test
Add taint analysis rule G709 to detect unsafe deserialization when
untrusted input flows into encoding/gob, encoding/xml, or
gopkg.in/yaml.v2 deserialization functions.
CWE-502. Includes 5 test samples (3 positive, 2 negative).
Add taint analysis rule G708 to detect SSTI vulnerabilities when using
Go text/template package. Covers two attack vectors:
- User input flowing into Template.Parse() (SSTI/RCE)
- Tainted data passed to Execute/ExecuteTemplate with http.ResponseWriter (XSS)
CWE-94. Includes 6 test samples (3 positive, 3 negative).
* fix(G118): eliminate false positive when cancel stored in struct field post-construction
When a cancel function is assigned to a struct field after construction
(e.g. s.cancel = cancel), the SSA FieldAddr for the store is a distinct
value from any FieldAddr created later for defer s.cancel() or inside a
closure. The existing isCancelCalledViaStructField only matched receiver
methods and missed these patterns.
Add isFieldCalledInAnyFunc which scans all SSA functions (including
closures) for a FieldAddr with matching struct pointer type and field
index, then checks whether the loaded value is called. As a side effect,
this also resolves the known false positive for nested struct field access.
fixes: 1595
* update rules documentation
When a cancel function is stored in a struct field and the struct is
returned to the caller, the cancel responsibility is transferred.
isCancelCalled did not detect this pattern because the FieldAddr trace
reached a dead end — it never connected the field store to the struct
being returned.
Add isStructFieldReturnedFromFunc that checks whether the struct base
pointer of a FieldAddr is loaded and returned, and call it from the
Store+FieldAddr branch in isCancelCalled.
Fixes#1591
The isCancelCalled function did not handle *ssa.MakeClosure, so when
a cancel function was captured as a free variable in a closure
(e.g. go func() { cancel() }()), gosec could not see the call inside
the closure body and reported a false positive.
Add a MakeClosure case that follows the cancel value into the closure's
FreeVars, allowing the existing call-detection logic to find the
invocation.
Fixes#1590
The isCancelCalled BFS did not handle *ssa.Return, so a cancel
function returned from a helper was flagged as lost even though
responsibility was transferred to the caller.
Add a Return case that recognises the cancel value among the return
operands and treats it as called.
Update the existing 'cancel returned to caller' test to expect 0
issues, and add a new test case matching the exact pattern from
issue #1584 (cancel returned as func() and invoked by the caller
through a shutdown chain).
Route redirect dependency checks through the cycle-safe
dependencyChecker instead of raw recursive valueDependsOn traversal.
This ensures Phi-cycle graphs terminate quickly and avoids recursive
work amplification that can look like hangs on large/generated
codebases.
Also remove the nil fallback in dependencyChecker.dependsOn so all
analyzer paths consistently use cycle-aware logic.
Add regression tests covering raw valueDependsOn behavior on:
Phi cycle without target (must return false)
Phi cycle with target path (must return true)
Self-referential Phi node (must return false)
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
Summary:
This change fixes the hang/perceived hang reported in issue #1555 when
scanning large codebases with complex SSA graphs. The fix is
intentionally scoped to G120 form parsing analysis only.
Root cause:
G120 wrapper/middleware protection logic repeatedly called
value-dependency checks across many function/call combinations. The
dependency traversal was depth-limited but not memoized, so cyclic and
branch-heavy SSA structures (especially Phi-related paths) caused
repeated re-traversal of the same graph regions and severe runtime
blowup.
What changed:
A cycle-safe, memoized dependency checker was added and used only inside
the G120 form parsing analysis flow. Existing G120 logic was kept
semantically equivalent while replacing repeated raw dependency
traversals with cached checks. Focused regression tests were added for
cyclic Phi graphs to verify both cases: no target reachable (false) and
target reachable (true), including stable repeated evaluation.
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* fix(G705): eliminate false positive for non-HTTP io.Writer
Adds ArgTypeGuards map[int]string to taint.Sink. The XSS analyzer now
requires arg[0] of fmt.Fprint* to implement net/http.ResponseWriter.
Writing exec pipe output to os.Stdout no longer triggers G705.
Fixes: #1548
* improve code coverage
This change introduces a new taint-analysis rule, G707, to detect
potential SMTP command/header injection when untrusted input reaches
net/smtp sink.
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
Add a new SSA-based analyzer, G123, to detect risky TLS configurations
where VerifyPeerCertificate is set, VerifyConnection is not set, and
session resumption may still be enabled.
The analyzer inspects tls.Config field assignments and also follows
configurations returned from GetConfigForClient callbacks so
callback-based setup paths are covered as well.
This change wires G123 into analyzer registration, maps it to CWE-295,
updates the README rule list, and adds dedicated vulnerable/safe sample
coverage in analyzer tests.
It also includes a targeted #nosec G101 suppression on the analyzer
message string to prevent a known false positive from the linter
(message text only, no credential handling impact).
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
This change introduces a new SSA-based analyzer, G122, to detect unsafe
filesystem operations inside filepath.Walk, filepath.WalkDir, and
io/fs.WalkDir callbacks when callback path values flow into race-prone
sinks such as os.Remove, os.OpenFile, os.Rename, and os.Chmod.
It adds CWE mapping for the new rule as G122 -> CWE-367 (TOCTOU race
condition), and adds the CWE-367 definition to the CWE data.
It wires G122 into analyzer registration and updates the README
available rules list.
It adds dedicated G122 sample coverage with vulnerable and safe cases,
including safe root-scoped usage through os.Root APIs (for example
root.Open and root.Remove).
Validation was completed: full test suite passes, golangci-lint reports
zero issues, and gosec CLI validation confirms expected trigger and
non-trigger behavior for G122.
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
This change fixes a false positive in G602 when iterating over
fixed-size arrays with range and indexing using the loop variable.
Corrects loop bound normalization in SSA-based index analysis so offsets
are not applied twice.
Preserves true-positive detection for real out-of-bounds patterns (for
example index + 1 at upper edge).
Adds regression samples covering both:
valid range-over-array indexing (no issue expected)
invalid shifted indexing inside the same loop (issue expected)
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
Introduces new G120 to detect potential memory-exhaustion paths caused
by unbounded form parsing in HTTP handlers.
Uses a pure SSA implementation (no AST fallback), checking ParseForm,
ParseMultipartForm, FormValue, and PostFormValue on *http.Request.
Suppresses findings when request bodies are explicitly bounded with
http.MaxBytesReader.
Wires G120 into analyzer registration, README rule catalog, and CWE
mapping (CWE-400).
Adds focused vulnerable/safe samples and analyzer test coverage;
analyzer tests and lint pass.
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
Fixes false positives for guarded conversions in loops by excluding back-edge dominators from reachability check. Fixes false negatives for array/slice element conversions by preventing recursive range resolution through IndexAddr. Also fixes isNonNegative check for range loops.
* Add G118 SSA analyzer for context propagation failures that can cause goroutine/resource leaks
This PR introduces G118, a new SSA-based gosec rule that detects
high-risk context misuse patterns: goroutines using
context.Background/TODO when request context exists, missing cancel()
calls from WithCancel/WithTimeout/WithDeadline, and unbounded blocking
loop regions without ctx.Done() guards.
These patterns can leak goroutines and I/O resources, leading to
resource exhaustion/DoS in production services.
The rule is mapped to CWE-400, integrated into analyzer registration and
docs, and includes positive/negative samples (including complex loop CFG
cases) to reduce false positives while preserving detection quality.
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* Fix false pasitive
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
---------
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
Implements a new SSA-based analyzer G113 to detect HTTP request
smuggling vulnerabilities caused by setting conflicting
Transfer-Encoding and Content-Length headers on the same HTTP response.
Addresses CVE-2025-22871 where ambiguous HTTP message parsing can lead
to request smuggling attacks. When both Transfer-Encoding: chunked and
Content-Length headers are set, intermediary proxies and backend servers
may disagree on message boundaries, allowing attackers to inject
malicious requests.
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* Add G408: SSH PublicKeyCallback Authentication Bypass Analyzer
Implements a new SSA-based security analyzer (G408) that detects
stateful misuse of ssh.PublicKeyCallback in SSH server configurations.
This vulnerability can lead to authentication bypass where a server
authenticates one SSH key but performs authorization checks on a
different key.
This addresses a critical security vulnerability (CVE-2024-45337, CVSS
9.1) that has affected production systems including Kubernetes and other
SSH-based services. The vulnerability occurs when developers incorrectly
capture and modify state within PublicKeyCallback closures, enabling
attackers to authenticate with one key while the server operates on
another key's credentials.
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* Fix tests
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
---------
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* feat: add taint analysis engine for data flow security
Implements SSA-based taint analysis to detect security vulnerabilities:
- G701: SQL injection via string concatenation
- G702: Command injection via user input
- G703: Path traversal via user input
- G704: SSRF via user-controlled URLs
- G705: XSS via unescaped user input
- G706: Log injection via user input
Uses golang.org/x/tools for SSA/call graph analysis with CHA.
Zero external dependencies beyond existing gosec imports.
- Replace dynamic fmt.Errorf with static errors in hot paths
- Replace regex-based directive parsing with manual string parsing (removed regexp import)
- Use const for directive prefix
* G115: Enhance RangeAnalyzer with constant propagation and chained arithmetic support
* Fix G115 overflow detection for negated values and robustify RangeAnalyzer propagation
* refactor
* optimizations
* Refactor analyzers: unify range logic and optimize allocations- Centralize numeric range analysis in util.go (shared by G115/G602).- Implement object pooling for slice_bounds and hardcoded_nonce.- Update conversion_overflow tests to use real analyzer logic.
* Refactor RangeAnalyzer
* Refine G407 to improve detection and coverage of hardcoded nonces
* chore: consolidate common analyzer patterns into util.go and improve G602 coverage
* Optimize G602 and G115 with state caching and regex pre-compilation
* Improve G115 overflow detection and fix false positives and false negatives
* golangci-lint workaround
* feat(slice): enhance slice bounds analysis with dynamic bounds handling
* feat(slice): enhance extractLenBound to support additional offset patterns and improve slice bounds analysis
* golangci-lint run
* Improve G602 slice bounds detection: support 3-index slices and correct capacity tracking
* Support out-of-bounds detection for range loops with offsets
Improve slice bound check to habdle bounded values and properly parse
the address index only from references
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
Cosmin Cojocar
Ravi Sastry Kadali
oittaa
kondratevandKondratev Pavel