Use pull_request_target event so the GOOGLE_API_KEY secret is available
when PRs come from forks. Checkout the PR head SHA explicitly since
pull_request_target defaults to the base branch. Guard other jobs to
skip on pull_request_target to avoid duplicate runs.
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
G117 now skips findings when:
- The marshal call is inside a custom marshaler method (MarshalJSON, MarshalYAML, etc.)
- The type being marshaled implements a custom marshaler interface
- A composite literal wraps the sensitive field value in a function call (e.g. mask())
Also fixes the issue message to show the correct format name (JSON/YAML/XML/TOML)
instead of always saying "JSON key".
Closes#1614
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* Add barry security scanner as a step in the CI
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* Enable SARIF upload to GitHub security center
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* Switch to gemini-3-flash for validation and autofix
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* Use the correct model name
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* Fix the SARIF upload
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* Update the output directory of the barry scan
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* Fix the output and the permissions to comment on pull request
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
---------
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
Cap incoming call graph edges to 32 per function and add cross-query
parameter taint memoization to prevent combinatorial explosion when CHA
over-approximates interface method calls across transitive dependencies.
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Packages with type errors (pkg.IllTyped == true) have partial type
information that can cause nil pointer dereferences inside the SSA
builder (golang.org/x/tools/go/ssa.emitConv).
Add a pkg.IllTyped guard in buildSSA that returns a clean error
instead of letting the SSA builder panic. AST-based rules continue
to run normally on the affected package.
Fixes#1604
* 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>
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).
The interprocedural taint analysis functions isCalleValueTainted,
isFieldOfAllocTaintedInCallee, and isFieldTaintedViaCall did not check
the visited map before recursing. Since callee-scope SSA values are
different objects from caller-scope ones, the visited map populated by
isTainted never cached them, causing exponential recursion on codebases
with multi-level constructor chains that fan out tainted config through
struct fields.
Add visited map checks at the entry of each interprocedural function to
prevent re-analyzing the same SSA values, call sites, and allocations.
Also add test cases exercising multi-level constructor chains, fan-out
constructors, and deep nested struct field access to verify termination.
Fixes#1587
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>
The relese process is extended to push also images to GHCR in addition
to DockerHub. This is in preparation to migrate to GHCR after the next
release.
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
This PR adds regression coverage for the G602 false-positive reported in
issue #1545 by introducing two sample cases: one valid range-over-array
indexing pattern that should not trigger, and one true out-of-bounds
variant that should still be detected.
It also fixes test instability in the rules suite by adding the missing
BurntSushi TOML module metadata required by G117 sample compilation in
the test harness.
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
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>
* Improve taint analyzer performance with shared SSA cache, parallel analyzer execution, and CI regression guard
* Added a shared per-package SSA analysis cache with lazy,
concurrency-safe call graph reuse across analyzers.
* Updated taint analyzers to consume the shared cache instead of
recomputing expensive artifacts per rule run.
* Parallelized analyzer execution at package level while preserving
deterministic issue aggregation.
* Added a package-level taint benchmark to measure real end-to-end taint
analyzer pass performance.
* Introduced a CI benchmark regression guard with configurable
thresholds for ns/op, B/op, and allocs/op.
* Documented the performance guard workflow, local run command, and
baseline update process in the README.
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
* Fix script
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
---------
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>
* 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>
Update Github action to use the release of gosec v2.23.0
Change-Id: I72672694bea0a1e25229283e15459f7762965fba
Signed-off-by: Cosmin Cojocar <ccojocar@google.com>
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>
* Update go version to 1.25.5 and 1.24.11 in CI
Signed-off-by: Cosmin Cojocar <ccojocar@google.com>
* Update the buildSSA to use the new tools package
Signed-off-by: Cosmin Cojocar <ccojocar@google.com>
* Remove the type allignment check
Signed-off-by: Cosmin Cojocar <ccojocar@google.com>
---------
Signed-off-by: Cosmin Cojocar <ccojocar@google.com>
Cosmin Cojocar
