Add regex safety checks and tests for pathological patterns (#2083)
This commit is contained in:
5
.changeset/polite-cobras-grin.md
Normal file
5
.changeset/polite-cobras-grin.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
'grafana-zabbix': patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fix CVE-2025-10630 by implementing regex scanner for exploits and adding timeout to compilation
|
||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"regexp"
|
"regexp"
|
||||||
"strings"
|
"strings"
|
||||||
|
"time"
|
||||||
|
|
||||||
"github.com/dlclark/regexp2"
|
"github.com/dlclark/regexp2"
|
||||||
"github.com/grafana/grafana-plugin-sdk-go/backend"
|
"github.com/grafana/grafana-plugin-sdk-go/backend"
|
||||||
@@ -65,6 +66,78 @@ func splitKeyParams(paramStr string) []string {
|
|||||||
return params
|
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) {
|
func parseFilter(filter string) (*regexp2.Regexp, error) {
|
||||||
vaildREModifiers := "imncsxrde"
|
vaildREModifiers := "imncsxrde"
|
||||||
regex := regexp.MustCompile(`^/(.+)/([imncsxrde]*)$`)
|
regex := regexp.MustCompile(`^/(.+)/([imncsxrde]*)$`)
|
||||||
@@ -75,17 +148,35 @@ func parseFilter(filter string) (*regexp2.Regexp, error) {
|
|||||||
return nil, nil
|
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 := ""
|
pattern := ""
|
||||||
if matches[2] != "" {
|
if matches[2] != "" {
|
||||||
if flagRE.MatchString(matches[2]) {
|
if flagRE.MatchString(matches[2]) {
|
||||||
pattern += "(?" + matches[2] + ")"
|
pattern += "(?" + matches[2] + ")"
|
||||||
} else {
|
} 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 {
|
func isRegex(filter string) bool {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package zabbix
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"reflect"
|
"reflect"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/dlclark/regexp2"
|
"github.com/dlclark/regexp2"
|
||||||
@@ -95,6 +96,48 @@ func TestParseFilter(t *testing.T) {
|
|||||||
expectNoError: false,
|
expectNoError: false,
|
||||||
expectedError: "",
|
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 {
|
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)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user