[sergo] Sergo Report: Registry-Delta Audit (regexpdynamicpattern) - 2026-08-07 #51000
Closed
Replies: 1 comment
|
This discussion has been marked as outdated by Sergo - Serena Go Expert. A newer discussion is available at Discussion #51301. |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Executive summary
Registry grew 43 → 63 analyzers (+20) since the last recorded run (2026-07-08) — all non-sergo-driven growth, and (per reconciliation) every one of those 20 was already individually audited, precision-checked, and fixed through normal PRs, not left as backlog. Zero sergo-labeled issues are currently open. This run's focus was the single analyzer added without a Sergo pass yet:
regexpdynamicpattern(merged 2026-08-05, #50674). Filed one issue covering two related defects: a factually-incorrect security rationale in its diagnostic message, and a missing pair of pattern variants (CompilePOSIX/MustCompilePOSIX). Also reconciled and verified two recent landings as correct.Tool & registry updates
pkg/lintersregistry: 63 analyzers registered inregistry.go(was 43 at R60).cmd/linters/main.gono longer enumerates analyzers directly — it now just callslinters.All()— so the delta-detector moved togrep -c '\.Analyzer,$' pkg/linters/registry.go.doc.go,README.md,spec_test.go) is fully in sync at 63 and now appears to be auto-enforced bydoc_sync_test.go. This was the single most repeated historical gotcha (6+ prior issues) — it looks resolved by tooling, not just coincidence, so future runs can deprioritize manually re-checking it every time.regexpdynamicpattern(flagsregexp.Compile/MustCompilecalls with non-constant patterns).Strategy: 50/50 split
Cached-reuse half (proven patterns re-applied):
syntactic_stdlib_matchcheck against the new linter's package-identity resolution — clean, it already usespass.TypesInfo.ObjectOf+*types.PkgName, not identifier-name matching.goroutinemissingrecovernamed-recover-helper false positive) — confirmed the landed fix (1cac677) correctly resolves deferred named functions viatypes.Func, matching the original recommendation.var Analyzer = ...declarations) — specifically the historical URL/path hazard wherepanic-in-library-code's hyphenated directory could mismatch a name-derived doc URL. Verified it correctly usesNewAtPathwith the explicit path, no regression.New-exploration half (fresh target):
regexpdynamicpattern.goand its testdata, since it had never been through a Sergo pass — surfaced the two findings below.Targets this run: findings ≥ 3 (met: 5), ≥1 filed issue of clear/distinct value (met), ≥1 reconciliation of prior fixes (met: 2).
Findings
rationale_inaccuracy).regexpdynamicpattern's diagnostic and package doc claim dynamic patterns can enable "catastrophic-backtracking (ReDoS)" attacks. Go'sregexppackage is RE2-based and explicitly guaranteed to run in linear time — it is specifically not vulnerable to ReDoS. The claim is technically wrong and could mislead anyone triaging the finding about the actual threat (real risks: runtime panic viaMustCompileon malformed input, or attacker-controlled pattern size/complexity — not backtracking cost). This has a direct precedent: #44484 foundbytescomparestring's "allocates" rationale was also technically wrong undergceven though the stylistic suggestion had merit.pattern_set_too_narrow.isRegexpCompileCallmatches onlyregexp.Compile/MustCompile, missingregexp.CompilePOSIX/MustCompilePOSIX, which take the identical pattern argument and carry the identical malformed-pattern-panic risk. Latent — zero current production callers of the POSIX variants.pass.TypesInfo.Types[...].Value(correctly handles concatenated/const-derived patterns, not just literals),nolintwired, test/generated-file skip wired, doc-sync in sync.goroutinemissingrecoverfix for goroutinemissingrecover: defer of a named recover-helper function is misreported as "missing recover" #50763 verified correct on re-read (types.Func resolution matches the recommendation exactly).panic-in-library-code).Evidence detail
regexpdynamicpattern.go:79:regexpdynamicpattern.go:64:One real dynamic-pattern production call exists today (unrelated to the bug, just confirms core detection works):
pkg/agentdrain/mask.go:33—regexp.Compile(r.Pattern)on a user-supplied mask rule.Task generated
regexpdynamicpattern: fix ReDoS rationale + add CompilePOSIX/MustCompilePOSIX coverage — small effort, two isolated edits in one file plus two testdata cases. Issue filed with full evidence, recommendation, and validation checklist.Metrics
Historical context
Prior-run pattern library (for future runs)
phantom_reconcile,registry_delta_detect,syntactic_stdlib_match,double_traversal_dup,pattern_set_too_narrow,node_filter_too_narrow,parent_shape_too_narrow,map_dedup_hides_dups,suppression_enforce_gap,scope_boundary_funclit— all previously catalogued, all re-checked against the newest linter this run where applicable.rationale_inaccuracy— a linter's stated security/performance rationale can be technically wrong even when its AST/type-resolution logic is correct. Worth a dedicated check whenever a new linter's doc comment makes a specific technical claim (allocation behavior, complexity class, panic conditions, etc.) — verify against actual language/runtime semantics, not just detection correctness.doc_sync_test.go— deprioritized for future runs unless a fresh drift is observed.Recommendations & next-run focus
grep -c '\.Analyzer,$' pkg/linters/registry.go).rationale_inaccuracycandidates (e.g. any claim about allocation, complexity, or exploitability) now that this pattern class is catalogued — bytescomparestring and regexpdynamicpattern are two hits already from a class no one was screening for before.References:
All reactions