fix(go): stop SQLi false positives on constant/parameterized queries - #248
Merged
Merged
Conversation
An adversarial precision pass (running our Go pack over AWS deception-bench's
code-level samples) exposed 6 false positives: our SQLi taint rule fired on
constant, no-user-input queries like `db.Query("SELECT id FROM users LIMIT 50")`.
Root cause: the Gin source pattern `$C.Query(...)` (for `c.Query("param")`) also
matched database/sql's own `db.Query(...)` — so every constant DB query was read
as BOTH a taint source and the sink, self-flowed, and false-positived. (It also
means govwa's SQLi was being "caught" by this collision, not real dataflow.)
Fix:
- Drop `$C.Query(...)` from the shared request sources (it collides with
db.Query/stmt.Query); Gin still covered by DefaultQuery/Param/PostForm/etc.
- Redesign the SQLi taint rule to key on the real dynamic-build signal —
source = fmt.Sprintf/concatenation, sink = single-arg query (gosec G201). A
constant query has no source and is never flagged; a parameterized call is
multi-arg and excluded by the sink; a Sprintf/concat-built query (inline or
assign-then-execute, e.g. govwa) is still caught.
- Inline the shared source list (semgrep's YAML-anchor handling was fragile).
Verified:
- deception-bench Go SQLi FPs: 6 -> 0. Constant/parameterized queries no longer
flagged; Sprintf/concat-built queries still are.
- govwa SQLi still caught; go-sast still 4/4; SAST-injection fixture gate still
53/53, 0 FP. Full suite: 2621 passed.
- Mined the exact FP class (a bare constant query) into the safe/ fixtures as a
permanent regression guard.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZ4QoTqRoYWE25CNoV2Wfy
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
An adversarial precision pass — running our Go pack over AWS deception-bench's code-level samples — exposed 6 false positives: our SQLi taint rule fired on constant, no-user-input queries like
db.Query("SELECT id FROM users LIMIT 50").Root cause
The Gin source pattern
$C.Query(...)(forc.Query("param")) also matched database/sql'sdb.Query(...)— so every constant DB query was read as both a taint source and the sink, self-flowed, and false-positived. (It also means govwa's SQLi was being "caught" by this collision, not real dataflow.)Fix
$C.Query(...)from the shared request sources; Gin still covered byDefaultQuery/Param/PostForm/etc.fmt.Sprintf/concatenation, sink = single-arg query (gosec G201). Constant → no source → not flagged; parameterized → multi-arg → excluded;Sprintf/concat-built (inline or assign-then-execute, e.g. govwa) → still caught.Verified
safe/as a permanent regression guard.🤖 Generated with Claude Code
https://claude.ai/code/session_01EZ4QoTqRoYWE25CNoV2Wfy