Skip to content

regexpdynamicpattern: ReDoS rationale is factually wrong, misses CompilePOSIX/MustCompilePOSIX #50999

Description

@github-actions

Summary

regexpdynamicpattern (pkg/linters/regexpdynamicpattern/regexpdynamicpattern.go), merged 2026-08-05 in #50674, is the newest custom analyzer and has not had a Sergo audit yet. Two related issues in its detection logic and framing:

  1. Rationale is factually wrong. The diagnostic message and package doc claim dynamic patterns "can enable catastrophic-backtracking (ReDoS) denial-of-service attacks." Go's regexp package is implemented on the RE2 automaton model and is explicitly guaranteed to run in time linear in the size of the input — it is specifically not susceptible to catastrophic backtracking. This is a well-known, deliberate design property of Go's regexp (as opposed to backtracking engines like PCRE/.NET/Python re), documented in the package's own godoc and in Russ Cox's RE2 writeups. Citing ReDoS as the risk misrepresents the actual threat model to anyone triaging a violation.
  2. pattern_set_too_narrow. isRegexpCompileCall only matches regexp.Compile and regexp.MustCompile, missing the POSIX variants regexp.CompilePOSIX / regexp.MustCompilePOSIX, which take the same pattern argument and carry the identical "can panic on a malformed pattern" risk.

Evidence

isRegexpCompileCall (regexpdynamicpattern.go:79):

if sel.Sel.Name != "MustCompile" && sel.Sel.Name != "Compile" {
    return false
}

run (regexpdynamicpattern.go:64):

Message: "regexp pattern is not a compile-time constant; dynamic patterns can panic at runtime or enable ReDoS if influenced by untrusted input",

The package doc (regexpdynamicpattern.go:4-6) repeats the same claim:

Dynamically constructed patterns can panic at runtime on malformed input and, when influenced by untrusted input, can enable catastrophic-backtracking (ReDoS) denial-of-service attacks.

The rest of the implementation is solid and avoids every common gotcha this project has hit before on new linters: isRegexpCompileCall resolves the package via pass.TypesInfo.ObjectOf + *types.PkgName (not syntactic identifier matching), hasConstantStringPattern correctly uses pass.TypesInfo.Types[...].Value so it handles arbitrary constant-folded expressions (not just literals/idents), nolint and filecheck.ShouldSkipFilename (test+generated skip) are both wired, and README/spec_test.go/doc.go are all in sync (enforced by doc_sync_test.go). This issue is narrowly about the message accuracy and the missing POSIX variants.

Current impact

  • No current callers of regexp.CompilePOSIX/MustCompilePOSIX in pkg/ — the pattern-family gap is latent, not an active false negative.
  • One real dynamic-pattern call exists today, pkg/agentdrain/mask.go:33 (regexp.Compile(r.Pattern), user-supplied mask rule) — unrelated to this bug, just confirms the linter's core detection already works on the one real production site.
  • Because the message is wrong rather than the detection, the "impact" here is a trust/triage issue: anyone who sees this diagnostic and knows Go's regexp guarantees will discount the finding as a false claim, and anyone who doesn't know will draw the wrong security conclusion (worrying about backtracking cost instead of the real risks: runtime panics from malformed patterns via MustCompile, or unbounded automaton size/memory from attacker-controlled pattern text).

Recommendation

  1. Reword the diagnostic message and package doc to drop the ReDoS/backtracking claim and state the real risk, e.g.: "regexp pattern is not a compile-time constant; a malformed dynamic pattern can panic at runtime (via MustCompile) or, if derived from untrusted input, allow an attacker to control pattern complexity/size."
  2. Extend isRegexpCompileCall to also accept CompilePOSIX and MustCompilePOSIX:
    switch sel.Sel.Name {
    case "MustCompile", "Compile", "MustCompilePOSIX", "CompilePOSIX":
    default:
        return false
    }
  3. Add testdata cases for both POSIX variants (dynamic pattern flagged, constant pattern not flagged) mirroring the existing MustCompile/Compile cases.

Validation checklist

  • Diagnostic message and package doc no longer mention ReDoS/catastrophic backtracking.
  • regexp.CompilePOSIX(dynamicPattern) and regexp.MustCompilePOSIX(dynamicPattern) are flagged.
  • regexp.CompilePOSIX(constPattern) / literal patterns remain unflagged.
  • Existing MustCompile/Compile test cases still pass unchanged.
  • Repo-wide grep for CompilePOSIX after the fix still shows zero production violations (confirms latent-only, no CI surprises).

Effort

Small — two isolated, independent edits in one file (regexpdynamicpattern.go) plus two new testdata cases. No autofix/SuggestedFix involved, no scope-boundary or AST-walk logic to touch.


temporary_id: aw_sg61a1

Generated by 🤖 Sergo - Serena Go Expert · agent · 302.8 AIC · ⌖ 32.3 AIC · ⊞ 6K · ◷

  • expires on Aug 13, 2026, 8:46 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions