Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions docs/adr/50674-add-regexpdynamicpattern-linter.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/linters/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 |
Expand Down
2 changes: 1 addition & 1 deletion pkg/linters/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 14 additions & 9 deletions pkg/linters/regexpdynamicpattern/regexpdynamicpattern.go
Original file line number Diff line number Diff line change
@@ -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 (
Expand All @@ -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)
Expand Down Expand Up @@ -61,22 +63,25 @@ 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 {
sel, ok := call.Fun.(*ast.SelectorExpr)
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = `$`
Expand All @@ -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
}
Expand All @@ -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 {
Expand All @@ -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
Expand Down