diff --git a/docs/adr/50674-add-regexpdynamicpattern-linter.md b/docs/adr/50674-add-regexpdynamicpattern-linter.md index 5d0f299dd31..f67d7cb0b30 100644 --- a/docs/adr/50674-add-regexpdynamicpattern-linter.md +++ b/docs/adr/50674-add-regexpdynamicpattern-linter.md @@ -12,16 +12,18 @@ The `pkg/linters` package provides custom Go static analysis passes that enforce and style invariants. `regexp.Compile` and `regexp.MustCompile` are safe when called with compile-time constant patterns, but when the pattern is built from dynamic input (e.g. `fmt.Sprintf`, string concatenation with variables, or function parameters) two distinct risks arise: a malformed pattern -causes a runtime panic (or a returned error that callers often ignore), and if the dynamic portion is -influenced by untrusted input the pattern can trigger catastrophic backtracking (ReDoS). The existing +causes a runtime panic in `MustCompile` variants (or a returned error that callers often ignore), +and if the dynamic portion is influenced by untrusted input the attacker can control pattern +complexity or size. The existing `regexpcompileinfunction` linter only enforces *where* compilation occurs (package-level vs. function-level), not *what* is being compiled, leaving the dynamic-pattern risk unaddressed. ### Decision We will introduce a new `regexpdynamicpattern` analyzer in `pkg/linters/regexpdynamicpattern/` that -reports any `regexp.Compile` or `regexp.MustCompile` call whose first argument is not a compile-time -constant string (literal, `const` identifier, or constant-only expression). Package identity is +reports any `regexp.Compile`, `regexp.MustCompile`, `regexp.CompilePOSIX`, or +`regexp.MustCompilePOSIX` call whose first argument is not a compile-time constant string (literal, +`const` identifier, or constant-only expression). Package identity is resolved via the type checker to handle aliased imports without false positives. The analyzer is registered in `pkg/linters/registry.go` alongside the existing linters and respects `//nolint:regexpdynamicpattern` suppressions for intentional dynamic patterns. @@ -37,17 +39,17 @@ identified: the *content* of the pattern, not its *location*, determines the saf #### Alternative 2: Runtime validation wrapper -Wrap `regexp.Compile`/`MustCompile` with a project-internal helper that validates or sanitizes the -pattern at runtime. This would catch panics but cannot prevent ReDoS (the unsafe pattern still -executes) and introduces runtime overhead on every compilation call. It also requires migrating all -call sites, whereas a static linter operates without any code changes to call sites that already use -constant patterns. +Wrap regexp compile calls with a project-internal helper that validates or sanitizes the pattern at +runtime. This would catch panics or errors but cannot prevent untrusted input from controlling +pattern complexity or size, and introduces runtime overhead on every compilation call. It also +requires migrating all call sites, whereas a static linter operates without any code changes to call +sites that already use constant patterns. ### Consequences #### Positive - Eliminates an entire class of runtime panics caused by malformed dynamically-constructed regexp patterns. -- Reduces the ReDoS attack surface by flagging call sites where untrusted input could flow into regexp compilation. +- Flags call sites where untrusted input could control regexp pattern complexity or size. - Consistent with the project's existing philosophy of enforcing safety invariants at analysis time rather than at runtime. #### Negative diff --git a/pkg/linters/README.md b/pkg/linters/README.md index 30f7ea61b4d..6ccc1f13b14 100644 --- a/pkg/linters/README.md +++ b/pkg/linters/README.md @@ -43,7 +43,7 @@ This package currently provides custom Go analyzers in the following subpackages - `panic-in-library-code` — reports `panic()` calls in library packages (`pkg/*`) where errors should be returned instead. - `rawloginlib` — reports direct usage of the standard `log` package in library packages, where `pkg/logger` should be used. - `regexpcompileinfunction` — reports `regexp.MustCompile` / `regexp.Compile` calls inside functions that should be package-level. -- `regexpdynamicpattern` — reports `regexp.MustCompile` / `regexp.Compile` calls whose pattern is not a compile-time constant string. +- `regexpdynamicpattern` — reports regexp compile calls whose pattern is not a compile-time constant string. - `seenmapbool` — reports `map[string]bool` used as a set (values always `true`) that should use `map[string]struct{}` instead. - `sortslice` — reports `sort.Slice` / `sort.SliceStable` calls that should use `slices.SortFunc` / `slices.SortStableFunc`. - `sprintferrdot` — reports redundant `.Error()` calls on error values passed to `fmt` format functions where the fmt package calls `.Error()` automatically. @@ -113,7 +113,7 @@ This package currently provides custom Go analyzers in the following subpackages | `panic-in-library-code` | Custom `go/analysis` analyzer that flags `panic()` usage in library packages | | `rawloginlib` | Custom `go/analysis` analyzer that flags standard-library `log` package calls in library packages | | `regexpcompileinfunction` | Custom `go/analysis` analyzer that flags regexp compilation inside function bodies | -| `regexpdynamicpattern` | Custom `go/analysis` analyzer that flags regexp.MustCompile/Compile calls with non-constant patterns | +| `regexpdynamicpattern` | Custom `go/analysis` analyzer that flags regexp compile calls with non-constant patterns | | `seenmapbool` | Custom `go/analysis` analyzer that flags `map[string]bool` used as a set that should use `map[string]struct{}` | | `sortslice` | Custom `go/analysis` analyzer that flags `sort.Slice` / `sort.SliceStable` calls that should use `slices.SortFunc` / `slices.SortStableFunc` | | `sprintferrdot` | Custom `go/analysis` analyzer that flags redundant `.Error()` calls on error values passed to `fmt` format functions | diff --git a/pkg/linters/doc.go b/pkg/linters/doc.go index 4bfc26d79bb..58284add348 100644 --- a/pkg/linters/doc.go +++ b/pkg/linters/doc.go @@ -39,7 +39,7 @@ // - panic-in-library-code — flags panic() calls in library packages // - rawloginlib — flags direct usage of the standard log package in library packages // - regexpcompileinfunction — flags regexp.MustCompile/Compile calls inside functions -// - regexpdynamicpattern — flags regexp.MustCompile/Compile calls whose pattern is not a compile-time constant +// - regexpdynamicpattern — flags regexp compile calls whose pattern is not a compile-time constant // - seenmapbool — flags map[string]bool used as a set that should use map[string]struct{} // - sortslice — flags sort.Slice / sort.SliceStable calls that should use slices.SortFunc / slices.SortStableFunc // - sprintferrdot — flags redundant .Error() calls on error values passed to fmt format functions diff --git a/pkg/linters/regexpdynamicpattern/regexpdynamicpattern.go b/pkg/linters/regexpdynamicpattern/regexpdynamicpattern.go index dad70682687..7c4af369dad 100644 --- a/pkg/linters/regexpdynamicpattern/regexpdynamicpattern.go +++ b/pkg/linters/regexpdynamicpattern/regexpdynamicpattern.go @@ -1,9 +1,9 @@ // Package regexpdynamicpattern implements a Go analysis linter that flags -// calls to regexp.MustCompile() and regexp.Compile() whose pattern argument -// is not a compile-time constant string. 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. +// calls to regexp compile functions whose pattern argument is not a +// compile-time constant string. Malformed dynamic patterns can panic in +// MustCompile variants, return errors in Compile variants, and, when +// influenced by untrusted input, allow an attacker to control pattern +// complexity or size. package regexpdynamicpattern import ( @@ -23,7 +23,9 @@ import ( var pkgLog = logger.New("linters:regexpdynamicpattern") // Analyzer is the regexp-dynamic-pattern analysis pass. -var Analyzer = analyzerutil.New("regexpdynamicpattern", "reports regexp.MustCompile and regexp.Compile calls whose pattern is not a compile-time constant string", run) +var Analyzer = analyzerutil.New("regexpdynamicpattern", "reports regexp compile calls whose pattern is not a compile-time constant string", run) + +const diagnosticMessage = "regexp pattern is not a compile-time constant; malformed dynamic patterns can panic in MustCompile variants, return errors in Compile variants, or let untrusted input control pattern complexity/size" func run(pass *analysis.Pass) (any, error) { insp, err := astutil.Inspector(pass) @@ -61,14 +63,14 @@ func run(pass *analysis.Pass) (any, error) { pass.Report(analysis.Diagnostic{ Pos: call.Pos(), End: call.End(), - Message: "regexp pattern is not a compile-time constant; dynamic patterns can panic at runtime or enable ReDoS if influenced by untrusted input", + Message: diagnosticMessage, }) } return nil, nil } -// isRegexpCompileCall checks if the call is to regexp.MustCompile or regexp.Compile, +// isRegexpCompileCall checks if the call is to a regexp compile function, // resolving the package identity via the type checker to handle aliased imports // and avoid false positives from local identifiers named "regexp". func isRegexpCompileCall(pass *analysis.Pass, call *ast.CallExpr) bool { @@ -76,7 +78,10 @@ func isRegexpCompileCall(pass *analysis.Pass, call *ast.CallExpr) bool { if !ok { return false } - if sel.Sel.Name != "MustCompile" && sel.Sel.Name != "Compile" { + switch sel.Sel.Name { + case "MustCompile", "Compile", "MustCompilePOSIX", "CompilePOSIX": + // Recognized regexp compile function; continue with package identity checks. + default: return false } ident, ok := sel.X.(*ast.Ident) diff --git a/pkg/linters/regexpdynamicpattern/testdata/src/regexpdynamicpattern/regexpdynamicpattern.go b/pkg/linters/regexpdynamicpattern/testdata/src/regexpdynamicpattern/regexpdynamicpattern.go index 5924ce22370..4450d8255bb 100644 --- a/pkg/linters/regexpdynamicpattern/testdata/src/regexpdynamicpattern/regexpdynamicpattern.go +++ b/pkg/linters/regexpdynamicpattern/testdata/src/regexpdynamicpattern/regexpdynamicpattern.go @@ -6,7 +6,7 @@ import ( ) // not flagged: literal pattern at package level. -var packageLevelRegexp = regexp.MustCompile(`^[a-z]+$`) +var PackageLevelRegexp = regexp.MustCompile(`^[a-z]+$`) const constPattern = `^const$` const constSuffix = `$` @@ -29,9 +29,24 @@ func ValidateConstConcat(input string) bool { return re.MatchString(input) } +// not flagged: POSIX literal pattern. +func ValidatePOSIXLiteral(input string) (bool, error) { + re, err := regexp.CompilePOSIX(`^[a-z]+$`) + if err != nil { + return false, err + } + return re.MatchString(input), nil +} + +// not flagged: POSIX const identifier pattern. +func ValidatePOSIXConst(input string) bool { + re := regexp.MustCompilePOSIX(constPattern) + return re.MatchString(input) +} + // flagged: pattern built with fmt.Sprintf. func ValidateSprintf(prefix, input string) (bool, error) { - re, err := regexp.Compile(fmt.Sprintf("^%s$", prefix)) // want `regexp pattern is not a compile-time constant; dynamic patterns can panic at runtime or enable ReDoS if influenced by untrusted input` + re, err := regexp.Compile(fmt.Sprintf("^%s$", prefix)) // want `regexp pattern is not a compile-time constant; malformed dynamic patterns can panic in MustCompile variants, return errors in Compile variants, or let untrusted input control pattern complexity/size` if err != nil { return false, err } @@ -40,20 +55,35 @@ func ValidateSprintf(prefix, input string) (bool, error) { // flagged: string concatenation with a variable. func ValidateConcatVariable(suffix, input string) bool { - re := regexp.MustCompile(`^prefix` + suffix) // want `regexp pattern is not a compile-time constant; dynamic patterns can panic at runtime or enable ReDoS if influenced by untrusted input` + re := regexp.MustCompile(`^prefix` + suffix) // want `regexp pattern is not a compile-time constant; malformed dynamic patterns can panic in MustCompile variants, return errors in Compile variants, or let untrusted input control pattern complexity/size` return re.MatchString(input) } // flagged: pattern passed through from a function parameter. func ValidateDynamic(pattern, input string) (bool, error) { - re, err := regexp.Compile(pattern) // want `regexp pattern is not a compile-time constant; dynamic patterns can panic at runtime or enable ReDoS if influenced by untrusted input` + re, err := regexp.Compile(pattern) // want `regexp pattern is not a compile-time constant; malformed dynamic patterns can panic in MustCompile variants, return errors in Compile variants, or let untrusted input control pattern complexity/size` if err != nil { return false, err } return re.MatchString(input), nil } -func suppressedPreviousLine(pattern, input string) (bool, error) { +// flagged: POSIX pattern passed through from a function parameter. +func ValidatePOSIXDynamic(pattern, input string) (bool, error) { + re, err := regexp.CompilePOSIX(pattern) // want `regexp pattern is not a compile-time constant; malformed dynamic patterns can panic in MustCompile variants, return errors in Compile variants, or let untrusted input control pattern complexity/size` + if err != nil { + return false, err + } + return re.MatchString(input), nil +} + +// flagged: POSIX MustCompile pattern passed through from a function parameter. +func ValidateMustPOSIXDynamic(pattern, input string) bool { + re := regexp.MustCompilePOSIX(pattern) // want `regexp pattern is not a compile-time constant; malformed dynamic patterns can panic in MustCompile variants, return errors in Compile variants, or let untrusted input control pattern complexity/size` + return re.MatchString(input) +} + +func SuppressedPreviousLine(pattern, input string) (bool, error) { //nolint:regexpdynamicpattern re, err := regexp.Compile(pattern) if err != nil { @@ -62,7 +92,7 @@ func suppressedPreviousLine(pattern, input string) (bool, error) { return re.MatchString(input), nil } -func suppressedSameLine(pattern, input string) (bool, error) { +func SuppressedSameLine(pattern, input string) (bool, error) { re, err := regexp.Compile(pattern) //nolint:regexpdynamicpattern if err != nil { return false, err