Skip to content

Commit 7f73c24

Browse files
committed
relies on coraza.rule.no_regex_multiline
1 parent 55520b9 commit 7f73c24

File tree

7 files changed

+165
-29
lines changed

7 files changed

+165
-29
lines changed

README.md

+1
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ dictionaries to reduce memory consumption in deployments that launch several cor
104104
instances. For more context check [this issue](https://github.com/corazawaf/coraza-caddy/issues/76)
105105
* `no_fs_access` - indicates that the target environment has no access to FS in order to not leverage OS' filesystem related functionality e.g. file body buffers.
106106
* `coraza.rule.case_sensitive_args_keys` - enables case-sensitive matching for ARGS keys, aligning Coraza behavior with RFC 3986 specification. It will be enabled by default in the next major version.
107+
* `coraza.rule.no_regex_multiline` - disables enabling by default regexes multiline modifiers in `@rx` operator. It aligns with CRS expected behavior, reduces false positives and might improve performances. No multiline regexes by default will be enabled in the next major version. For more context check [this PR](https://github.com/corazawaf/coraza/pull/876)
107108

108109
## E2E Testing
109110

internal/operators/multilineregex.go

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Copyright 2024 Juan Pablo Tosso and the OWASP Coraza contributors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
//go:build coraza.rule.no_regex_multiline
5+
6+
package operators
7+
8+
var shouldNotUseMultilineRegexesOperatorByDefault = true
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Copyright 2024 Juan Pablo Tosso and the OWASP Coraza contributors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
//go:build !coraza.rule.no_regex_multiline
5+
6+
package operators
7+
8+
var shouldNotUseMultilineRegexesOperatorByDefault = false

internal/operators/rx.go

+13-4
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,19 @@ type rx struct {
2424
var _ plugintypes.Operator = (*rx)(nil)
2525

2626
func newRX(options plugintypes.OperatorOptions) (plugintypes.Operator, error) {
27-
// (?s) enables dotall mode, required by some CRS rules and matching ModSec behavior, see
28-
// - https://github.com/google/re2/wiki/Syntax
29-
// - Flag usage: https://groups.google.com/g/golang-nuts/c/jiVdamGFU9E
30-
data := fmt.Sprintf("(?s)%s", options.Arguments)
27+
var data string
28+
if shouldNotUseMultilineRegexesOperatorByDefault {
29+
// (?s) enables dotall mode, required by some CRS rules and matching ModSec behavior, see
30+
// - https://github.com/google/re2/wiki/Syntax
31+
// - Flag usage: https://groups.google.com/g/golang-nuts/c/jiVdamGFU9E
32+
data = fmt.Sprintf("(?s)%s", options.Arguments)
33+
} else {
34+
// TODO: deprecate multiline modifier set by default in Coraza v4
35+
// CRS rules will explicitly set the multiline modifier when needed
36+
// Having it enabled by default can lead to false positives and less performance
37+
// See https://github.com/corazawaf/coraza/pull/876
38+
data = fmt.Sprintf("(?sm)%s", options.Arguments)
39+
}
3140

3241
if matchesArbitraryBytes(data) {
3342
// Use binary regex matcher if expression matches non-utf8 bytes. The binary matcher does
+119
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// Copyright 2022 Juan Pablo Tosso and the OWASP Coraza contributors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
//go:build coraza.rule.no_regex_multiline
5+
6+
package operators
7+
8+
import (
9+
"fmt"
10+
"testing"
11+
12+
"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
13+
"github.com/corazawaf/coraza/v3/internal/corazawaf"
14+
)
15+
16+
func TestRx(t *testing.T) {
17+
tests := []struct {
18+
pattern string
19+
input string
20+
want bool
21+
}{
22+
{
23+
pattern: "som(.*)ta",
24+
input: "somedata",
25+
want: true,
26+
},
27+
{
28+
pattern: "som(.*)ta",
29+
input: "notdata",
30+
want: false,
31+
},
32+
{
33+
pattern: "ハロー",
34+
input: "ハローワールド",
35+
want: true,
36+
},
37+
{
38+
pattern: "ハロー",
39+
input: "グッバイワールド",
40+
want: false,
41+
},
42+
{
43+
pattern: `\xac\xed\x00\x05`,
44+
input: "\xac\xed\x00\x05t\x00\x04test",
45+
want: true,
46+
},
47+
{
48+
pattern: `\xac\xed\x00\x05`,
49+
input: "\xac\xed\x00t\x00\x04test",
50+
want: false,
51+
},
52+
{
53+
// Requires dotall
54+
pattern: `hello.*world`,
55+
input: "hello\nworld",
56+
want: true,
57+
},
58+
{
59+
// Requires multiline disabled by default
60+
pattern: `^hello.*world`,
61+
input: "test\nhello\nworld",
62+
want: false,
63+
},
64+
{
65+
// Makes sure multiline can be enabled by the user
66+
pattern: `(?m)^hello.*world`,
67+
input: "test\nhello\nworld",
68+
want: true,
69+
},
70+
{
71+
// Makes sure, (?s) passed by the user does not
72+
// break the regex.
73+
pattern: `(?s)hello.*world`,
74+
input: "hello\nworld",
75+
want: true,
76+
},
77+
{
78+
// Make sure user flags are also applied
79+
pattern: `(?i)hello.*world`,
80+
input: "testHELLO\nworld",
81+
want: true,
82+
},
83+
{
84+
// The so called DOLLAR_ENDONLY modifier in PCRE2 is meant to tweak the meaning of dollar '$'
85+
// so that it matches only at the very end of the string (see: https://www.pcre.org/current/doc/html/pcre2pattern.html#SEC6)
86+
// It seems that re2 already behaves like that by default.
87+
pattern: `123$`,
88+
input: "123\n",
89+
want: false,
90+
},
91+
{
92+
// Dollar endonly match
93+
pattern: `123$`,
94+
input: "test123",
95+
want: true,
96+
},
97+
}
98+
99+
for _, tc := range tests {
100+
tt := tc
101+
t.Run(fmt.Sprintf("%s/%s", tt.pattern, tt.input), func(t *testing.T) {
102+
103+
opts := plugintypes.OperatorOptions{
104+
Arguments: tt.pattern,
105+
}
106+
rx, err := newRX(opts)
107+
if err != nil {
108+
t.Error(err)
109+
}
110+
waf := corazawaf.NewWAF()
111+
tx := waf.NewTransaction()
112+
tx.Capture = true
113+
res := rx.Evaluate(tx, tt.input)
114+
if res != tt.want {
115+
t.Errorf("want %v, got %v", tt.want, res)
116+
}
117+
})
118+
}
119+
}

internal/operators/rx_test.go

+7-25
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright 2022 Juan Pablo Tosso and the OWASP Coraza contributors
22
// SPDX-License-Identifier: Apache-2.0
33

4+
//go:build !coraza.rule.no_regex_multiline
5+
46
package operators
57

68
import (
@@ -55,42 +57,22 @@ func TestRx(t *testing.T) {
5557
want: true,
5658
},
5759
{
58-
// Requires multiline disabled by default
60+
// Requires multiline
5961
pattern: `^hello.*world`,
6062
input: "test\nhello\nworld",
61-
want: false,
62-
},
63-
{
64-
// Makes sure multiline can be enabled by the user
65-
pattern: `(?m)^hello.*world`,
66-
input: "test\nhello\nworld",
6763
want: true,
6864
},
6965
{
70-
// Makes sure, (?s) passed by the user does not
66+
// Makes sure, (?sm) passed by the user does not
7167
// break the regex.
72-
pattern: `(?s)hello.*world`,
68+
pattern: `(?sm)hello.*world`,
7369
input: "hello\nworld",
7470
want: true,
7571
},
7672
{
7773
// Make sure user flags are also applied
78-
pattern: `(?i)hello.*world`,
79-
input: "testHELLO\nworld",
80-
want: true,
81-
},
82-
{
83-
// The so called DOLLAR_ENDONLY modifier in PCRE2 is meant to tweak the meaning of dollar '$'
84-
// so that it matches only at the very end of the string (see: https://www.pcre.org/current/doc/html/pcre2pattern.html#SEC6)
85-
// It seems that re2 already behaves like that by default.
86-
pattern: `123$`,
87-
input: "123\n",
88-
want: false,
89-
},
90-
{
91-
// Dollar endonly match
92-
pattern: `123$`,
93-
input: "test123",
74+
pattern: `(?i)^hello.*world`,
75+
input: "test\nHELLO\nworld",
9476
want: true,
9577
},
9678
}

magefile.go

+9
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,15 @@ func Test() error {
127127
return err
128128
}
129129

130+
// Execute FTW tests with coraza.rule.no_regex_multiline as well
131+
if err := sh.RunV("go", "test", "-tags=coraza.rule.no_regex_multiline", "./testing/coreruleset"); err != nil {
132+
return err
133+
}
134+
135+
if err := sh.RunV("go", "test", "-tags=coraza.rule.no_regex_multiline", "-run=^TestRx", "./..."); err != nil {
136+
return err
137+
}
138+
130139
if err := sh.RunV("go", "test", "-tags=coraza.rule.case_sensitive_args_keys", "-run=^TestCaseSensitive", "./..."); err != nil {
131140
return err
132141
}

0 commit comments

Comments
 (0)