diff --git a/.changeset/polite-cobras-grin.md b/.changeset/polite-cobras-grin.md new file mode 100644 index 0000000..a6e7d00 --- /dev/null +++ b/.changeset/polite-cobras-grin.md @@ -0,0 +1,5 @@ +--- +'grafana-zabbix': patch +--- + +Fix CVE-2025-10630 by implementing regex scanner for exploits and adding timeout to compilation diff --git a/pkg/zabbix/utils.go b/pkg/zabbix/utils.go index 8bea620..6afa87e 100644 --- a/pkg/zabbix/utils.go +++ b/pkg/zabbix/utils.go @@ -4,6 +4,7 @@ import ( "fmt" "regexp" "strings" + "time" "github.com/dlclark/regexp2" "github.com/grafana/grafana-plugin-sdk-go/backend" @@ -65,6 +66,78 @@ func splitKeyParams(paramStr string) []string { return params } +// isPathologicalRegex detects potentially dangerous regex patterns that could cause ReDoS +func isPathologicalRegex(pattern string) bool { + // Check for consecutive quantifiers + consecutiveQuantifiers := []string{`\*\*`, `\+\+`, `\*\+`, `\+\*`} + for _, q := range consecutiveQuantifiers { + if matched, _ := regexp.MatchString(q, pattern); matched { + return true + } + } + + // Check for nested quantifiers + nestedQuantifiers := []string{ + `\([^)]*\+[^)]*\)\+`, // (a+)+ + `\([^)]*\*[^)]*\)\*`, // (a*)* + `\([^)]*\+[^)]*\)\*`, // (a+)* + `\([^)]*\*[^)]*\)\+`, // (a*)+ + } + for _, nested := range nestedQuantifiers { + if matched, _ := regexp.MatchString(nested, pattern); matched { + return true + } + } + + // Check for specific catastrophic patterns + catastrophicPatterns := []string{ + `\(\.\*\)\*`, // (.*)* + `\(\.\+\)\+`, // (.+)+ + `\(\.\*\)\+`, // (.*)+ + `\(\.\+\)\*`, // (.+)* + } + for _, catastrophic := range catastrophicPatterns { + if matched, _ := regexp.MatchString(catastrophic, pattern); matched { + return true + } + } + + // Check for obvious overlapping alternation (manual check for exact duplicates) + if strings.Contains(pattern, "(a|a)") || + strings.Contains(pattern, "(1|1)") || + strings.Contains(pattern, "(.*|.*)") { + return true + } + + return false +} + +// safeRegexpCompile compiles a regex with timeout protection +func safeRegexpCompile(pattern string) (*regexp2.Regexp, error) { + // Channel to receive compilation result + resultCh := make(chan struct { + regex *regexp2.Regexp + err error + }, 1) + + // Compile regex in goroutine with timeout + go func() { + regex, err := regexp2.Compile(pattern, regexp2.RE2) + resultCh <- struct { + regex *regexp2.Regexp + err error + }{regex, err} + }() + + // Wait for compilation or timeout + select { + case result := <-resultCh: + return result.regex, result.err + case <-time.After(5 * time.Second): + return nil, fmt.Errorf("regex compilation timeout (5s) - pattern may be too complex") + } +} + func parseFilter(filter string) (*regexp2.Regexp, error) { vaildREModifiers := "imncsxrde" regex := regexp.MustCompile(`^/(.+)/([imncsxrde]*)$`) @@ -75,17 +148,35 @@ func parseFilter(filter string) (*regexp2.Regexp, error) { return nil, nil } + regexPattern := matches[1] + + // Security: Check for pathological regex patterns + if isPathologicalRegex(regexPattern) { + return nil, backend.DownstreamErrorf("error parsing regexp: potentially dangerous regex pattern detected") + } + + // Security: Limit regex pattern length + if len(regexPattern) > 1000 { + return nil, backend.DownstreamErrorf("error parsing regexp: pattern too long (max 1000 characters)") + } + pattern := "" if matches[2] != "" { if flagRE.MatchString(matches[2]) { pattern += "(?" + matches[2] + ")" } else { - return nil, backend.DownstreamError(fmt.Errorf("error parsing regexp: unsupported flags `%s` (expected [%s])", matches[2], vaildREModifiers)) + return nil, backend.DownstreamErrorf("error parsing regexp: unsupported flags `%s` (expected [%s])", matches[2], vaildREModifiers) } } - pattern += matches[1] + pattern += regexPattern - return regexp2.Compile(pattern, regexp2.RE2) + // Security: Test compilation with timeout + compiled, err := safeRegexpCompile(pattern) + if err != nil { + return nil, backend.DownstreamErrorf("error parsing regexp: %v", err) + } + + return compiled, nil } func isRegex(filter string) bool { diff --git a/pkg/zabbix/utils_test.go b/pkg/zabbix/utils_test.go index d750a13..02e6f37 100644 --- a/pkg/zabbix/utils_test.go +++ b/pkg/zabbix/utils_test.go @@ -2,6 +2,7 @@ package zabbix import ( "reflect" + "strings" "testing" "github.com/dlclark/regexp2" @@ -95,6 +96,48 @@ func TestParseFilter(t *testing.T) { expectNoError: false, expectedError: "", }, + { + name: "Pathological regex - nested quantifiers (a+)+", + filter: "/^(a+)+$/", + want: nil, + expectNoError: false, + expectedError: "error parsing regexp: potentially dangerous regex pattern detected", + }, + { + name: "Pathological regex - (.*)* pattern", + filter: "/(.*)*/", + want: nil, + expectNoError: false, + expectedError: "error parsing regexp: potentially dangerous regex pattern detected", + }, + { + name: "Pathological regex - overlapping alternation", + filter: "/(a|a)*/", + want: nil, + expectNoError: false, + expectedError: "error parsing regexp: potentially dangerous regex pattern detected", + }, + { + name: "Pathological regex - consecutive quantifiers", + filter: "/a**/", + want: nil, + expectNoError: false, + expectedError: "error parsing regexp: potentially dangerous regex pattern detected", + }, + { + name: "Pattern too long", + filter: "/" + strings.Repeat("a", 1001) + "/", + want: nil, + expectNoError: false, + expectedError: "error parsing regexp: pattern too long (max 1000 characters)", + }, + { + name: "Safe complex regex", + filter: "/^[a-zA-Z0-9_-]+\\.[a-zA-Z]{2,}$/", + want: regexp2.MustCompile("^[a-zA-Z0-9_-]+\\.[a-zA-Z]{2,}$", regexp2.RE2), + expectNoError: true, + expectedError: "", + }, } for _, tt := range tests { @@ -113,3 +156,69 @@ func TestParseFilter(t *testing.T) { }) } } + +func TestIsPathologicalRegex(t *testing.T) { + tests := []struct { + name string + pattern string + expected bool + }{ + { + name: "Safe pattern", + pattern: "^[a-zA-Z0-9]+$", + expected: false, + }, + { + name: "Nested quantifiers (a+)+", + pattern: "^(a+)+$", + expected: true, + }, + { + name: "Nested quantifiers (a*)*", + pattern: "(a*)*", + expected: true, + }, + { + name: "Overlapping alternation (a|a)*", + pattern: "(a|a)*", + expected: true, + }, + { + name: "Consecutive quantifiers **", + pattern: "a**", + expected: true, + }, + { + name: "Consecutive quantifiers ++", + pattern: "a++", + expected: true, + }, + { + name: "Catastrophic (.*)* pattern", + pattern: "(.*)*", + expected: true, + }, + { + name: "Catastrophic (.+)+ pattern", + pattern: "(.+)+", + expected: true, + }, + { + name: "Safe alternation with different patterns", + pattern: "(cat|dog)*", + expected: false, + }, + { + name: "Safe quantifier usage", + pattern: "[0-9]+\\.[0-9]*", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isPathologicalRegex(tt.pattern) + assert.Equal(t, tt.expected, result, "Pattern: %s", tt.pattern) + }) + } +}