Skip to content

Commit

Permalink
fix: MULTIPART_STRICT_ERROR, updates CRS tests to v4.6.0 (#1166)
Browse files Browse the repository at this point in the history
* fix MultipartStrictError, updates tests to CRS v4.6 adding proper overrides

* go mod tidy

---------

Co-authored-by: José Carlos Chávez <jcchavezs@gmail.com>
  • Loading branch information
M4tteoP and jcchavezs authored Oct 5, 2024
1 parent b9cc95d commit e1c5b1b
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 22 deletions.
2 changes: 1 addition & 1 deletion examples/http-server/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/corazawaf/libinjection-go v0.2.1 // indirect
github.com/magefile/mage v1.15.0 // indirect
github.com/petar-dambovaliev/aho-corasick v0.0.0-20240411101913-e07a1f0e8eb4 // indirect
github.com/tidwall/gjson v1.17.3 // indirect
github.com/tidwall/gjson v1.18.0 // indirect
github.com/tidwall/match v1.1.1 // indirect
github.com/tidwall/pretty v1.2.1 // indirect
golang.org/x/net v0.29.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions examples/http-server/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ github.com/miekg/dns v1.1.57 h1:Jzi7ApEIzwEPLHWRcafCN9LZSBbqQpxjt/wpgvg7wcM=
github.com/miekg/dns v1.1.57/go.mod h1:uqRjCRUuEAA6qsOiJvDd+CFo/vW+y5WR6SNmHE55hZk=
github.com/petar-dambovaliev/aho-corasick v0.0.0-20240411101913-e07a1f0e8eb4 h1:1Kw2vDBXmjop+LclnzCb/fFy+sgb3gYARwfmoUcQe6o=
github.com/petar-dambovaliev/aho-corasick v0.0.0-20240411101913-e07a1f0e8eb4/go.mod h1:EHPiTAKtiFmrMldLUNswFwfZ2eJIYBHktdaUTZxYWRw=
github.com/tidwall/gjson v1.17.3 h1:bwWLZU7icoKRG+C+0PNwIKC6FCJO/Q3p2pZvuP0jN94=
github.com/tidwall/gjson v1.17.3/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY=
github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
Expand Down
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,12 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/magefile/mage v1.15.0 h1:BvGheCMAsG3bWUDbZ8AyXXpCNwU9u5CB6sM+HNb9HYg=
github.com/magefile/mage v1.15.0/go.mod h1:z5UZb/iS3GoOSn0JgWuiw7dxlurVYTu+/jHXqQg881A=
github.com/mccutchen/go-httpbin/v2 v2.14.0 h1:9N7GUf8+JunYMFd+yHPIVYApC6KYgqtF0pHIcTGYcVQ=
github.com/mccutchen/go-httpbin/v2 v2.14.0/go.mod h1:f4DUXYlU6yH0V81O4lJIwqpmYdTXXmYwzxMnYEimFPk=
github.com/mccutchen/go-httpbin/v2 v2.15.0 h1:3b2s8LMRR2aFd+8U+1Bx2kdgHNQ5ZQkQOiW8e52Jj9A=
github.com/mccutchen/go-httpbin/v2 v2.15.0/go.mod h1:GBy5I7XwZ4ZLhT3hcq39I4ikwN9x4QUt6EAxNiR8Jus=
github.com/miekg/dns v1.1.57 h1:Jzi7ApEIzwEPLHWRcafCN9LZSBbqQpxjt/wpgvg7wcM=
github.com/miekg/dns v1.1.57/go.mod h1:uqRjCRUuEAA6qsOiJvDd+CFo/vW+y5WR6SNmHE55hZk=
github.com/petar-dambovaliev/aho-corasick v0.0.0-20240411101913-e07a1f0e8eb4 h1:1Kw2vDBXmjop+LclnzCb/fFy+sgb3gYARwfmoUcQe6o=
github.com/petar-dambovaliev/aho-corasick v0.0.0-20240411101913-e07a1f0e8eb4/go.mod h1:EHPiTAKtiFmrMldLUNswFwfZ2eJIYBHktdaUTZxYWRw=
github.com/tidwall/gjson v1.17.3 h1:bwWLZU7icoKRG+C+0PNwIKC6FCJO/Q3p2pZvuP0jN94=
github.com/tidwall/gjson v1.17.3/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY=
github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
Expand Down
8 changes: 5 additions & 3 deletions internal/corazawaf/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ func (tx *Transaction) Collection(idx variables.RuleVariable) collection.Collect
return tx.variables.xml
case variables.MultipartPartHeaders:
return tx.variables.multipartPartHeaders
case variables.MultipartStrictError:
return tx.variables.multipartStrictError
}

return collections.Noop
Expand Down Expand Up @@ -1049,12 +1051,12 @@ func (tx *Transaction) ProcessRequestBody() (*types.Interruption, error) {
return tx.interruption, nil
}

// ProcessResponseHeaders Perform the analysis on the response readers.
// ProcessResponseHeaders performs the analysis on the response headers.
//
// This method perform the analysis on the response headers, notice however
// This method performs the analysis on the response headers. Note, however,
// that the headers should be added prior to the execution of this function.
//
// note: Remember to check for a possible intervention.
// Note: Remember to check for a possible intervention.
func (tx *Transaction) ProcessResponseHeaders(code int, proto string) *types.Interruption {
if tx.RuleEngine == types.RuleEngineOff {
return nil
Expand Down
3 changes: 1 addition & 2 deletions testing/coraza_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ func TestEngine(t *testing.T) {
t.Error(err)
}
for _, test := range tt {
testname := p.Tests[0].Title
t.Run(testname, func(t *testing.T) {
t.Run(test.Name, func(t *testing.T) {
if err := test.RunPhases(); err != nil {
t.Errorf("%s, ERROR: %s", test.Name, err)
}
Expand Down
21 changes: 21 additions & 0 deletions testing/coreruleset/.ftw-overrides.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,24 @@ test_overrides:
status: 505
log:
no_expect_ids: [920430]

- rule_id: 922130
test_ids: [1,2,7]
reason: "Multipart parsing tests. 922130 rule is not reached, Coraza triggers rule 200003 (MULTIPART_STRICT_ERROR) at parsing time"
output:
log:
expect_ids: [200003]
- rule_id: 922130
test_ids: [4]
reason: |
Multipart parsing tests. A space caracther is not in the valid range. 0x20 character raises MULTIPART_STRICT_ERROR (see multipart_error.yaml engine test),
But 922130-4 test does not. In this case, rule 922130 is matched.
output:
log:
expect_ids: [922130]
- rule_id: 922130
test_ids: [3,5,6]
reason: "Valid Multipart parsing payloads. Coraza should not trigger rules 200002 (REQBODY_ERROR), 200003 (MULTIPART_STRICT_ERROR), 922130"
output:
log:
no_expect_ids: [200002,200003,922130]
4 changes: 2 additions & 2 deletions testing/coreruleset/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.22.3

require (
github.com/bmatcuk/doublestar/v4 v4.6.1
github.com/corazawaf/coraza-coreruleset/v4 v4.5.0
github.com/corazawaf/coraza-coreruleset/v4 v4.6.0
github.com/corazawaf/coraza/v3 v3.0.0-00010101000000-000000000000
github.com/coreruleset/albedo v0.0.16
github.com/coreruleset/go-ftw v1.0.4-0.20240923043156-8474a93d514a
Expand Down Expand Up @@ -41,7 +41,7 @@ require (
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/petar-dambovaliev/aho-corasick v0.0.0-20240411101913-e07a1f0e8eb4 // indirect
github.com/rogpeppe/go-internal v1.13.1 // indirect
github.com/tidwall/gjson v1.17.3 // indirect
github.com/tidwall/gjson v1.18.0 // indirect
github.com/tidwall/match v1.1.1 // indirect
github.com/tidwall/pretty v1.2.1 // indirect
github.com/valllabh/ocsf-schema-golang v1.0.3 // indirect
Expand Down
12 changes: 4 additions & 8 deletions testing/coreruleset/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,15 @@ github.com/Masterminds/sprig v2.22.0+incompatible h1:z4yfnGrZ7netVz+0EDJ0Wi+5VZC
github.com/Masterminds/sprig v2.22.0+incompatible/go.mod h1:y6hNFY5UBTIWBxnzTeuNhlNS5hqE0NB0E6fgfo2Br3o=
github.com/bmatcuk/doublestar/v4 v4.6.1 h1:FH9SifrbvJhnlQpztAx++wlkk70QBf0iBWDwNy7PA4I=
github.com/bmatcuk/doublestar/v4 v4.6.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
github.com/corazawaf/coraza-coreruleset/v4 v4.5.0 h1:4BDr9/yWKSJ7Ch3h7SvSqJBASju73+EqIIF0WxjsFgQ=
github.com/corazawaf/coraza-coreruleset/v4 v4.5.0/go.mod h1:1FQt1p+JSQ6tYrafMqZrEEdDmhq6aVuIJdnk+bM9hMY=
github.com/corazawaf/coraza-coreruleset/v4 v4.6.0 h1:VGlMw3QMuKaV7XgifPgcqCm66K+HRSdM4d9PRh1nD50=
github.com/corazawaf/coraza-coreruleset/v4 v4.6.0/go.mod h1:1FQt1p+JSQ6tYrafMqZrEEdDmhq6aVuIJdnk+bM9hMY=
github.com/corazawaf/libinjection-go v0.2.1 h1:vNJ7L6c4xkhRgYU6sIO0Tl54TmeCQv/yfxBma30Dy/Y=
github.com/corazawaf/libinjection-go v0.2.1/go.mod h1:OP4TM7xdJ2skyXqNX1AN1wN5nNZEmJNuWbNPOItn7aw=
github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
github.com/coreruleset/albedo v0.0.16-0.20240924185852-4b95a321ebfd h1:4KjOGOv4I81ZOuW/TY29oe0yZCNeNGuPt8HXvmjukPI=
github.com/coreruleset/albedo v0.0.16-0.20240924185852-4b95a321ebfd/go.mod h1:6mYBASfvvRM2ckXgYO7N5nyKAj8OqLnT4+YLbM0/XWE=
github.com/coreruleset/albedo v0.0.16 h1:YYWEJBSfAwVmZ5tbBgQQhL8uU6IKeA2QwpAkii3UPKY=
github.com/coreruleset/albedo v0.0.16/go.mod h1:6mYBASfvvRM2ckXgYO7N5nyKAj8OqLnT4+YLbM0/XWE=
github.com/coreruleset/ftw-tests-schema/v2 v2.1.0 h1:2ilKzKRG5UzzxBcrJLXFtPalStdQ9jzzaYFuFk0OEk0=
github.com/coreruleset/ftw-tests-schema/v2 v2.1.0/go.mod h1:ZHVFX5ses4+5IxUP0ufCNg/VqRWxziH6ZuUca092Hxo=
github.com/coreruleset/go-ftw v1.0.4-0.20240809050408-f8169f0325ac h1:I++204ogJDnOyYQrMs6IdfTYRBIMr4A9Dtix+XdtZEc=
github.com/coreruleset/go-ftw v1.0.4-0.20240809050408-f8169f0325ac/go.mod h1:gI2N2EYdTIZnXQbsdzBRxbj/zSaYEyrhLJUCOJ3VK6I=
github.com/coreruleset/go-ftw v1.0.4-0.20240923043156-8474a93d514a h1:ucsngMh75QkedILO3N9JRiVWPbkcelhH055adS51hNs=
github.com/coreruleset/go-ftw v1.0.4-0.20240923043156-8474a93d514a/go.mod h1:/HKpRGHn0rOEb+VdkfjLqltb3Jr5DHDeKPAiC/04lR8=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
Expand Down Expand Up @@ -99,8 +95,8 @@ github.com/rs/zerolog v1.33.0 h1:1cU2KZkvPxNyfgEmhHAz/1A9Bz+llsdYzklWFzgp0r8=
github.com/rs/zerolog v1.33.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
github.com/tidwall/gjson v1.17.3 h1:bwWLZU7icoKRG+C+0PNwIKC6FCJO/Q3p2pZvuP0jN94=
github.com/tidwall/gjson v1.17.3/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY=
github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
Expand Down
72 changes: 72 additions & 0 deletions testing/engine/multipart.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,75 @@ SecRule REQBODY_ERROR "!@eq 0" \
"id:'200002', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}',severity:2"
`,
})

var _ = profile.RegisterProfile(profile.Profile{
Meta: profile.Meta{
Author: "M4tteoP",
Description: "MULTIPART_STRICT_ERROR rule triggered",
Enabled: true,
Name: "multipart_error.yaml",
},
Tests: []profile.Test{
{
Title: "multipart error invalid 0x0E",
Stages: []profile.Stage{
{
Stage: profile.SubStage{
Input: profile.StageInput{
URI: "/test.php?id=1",
Headers: map[string]string{
"Host": "www.example.com",
"Content-Type": "multipart/form-data; boundary=--0000",
},
Data: `
----0000
\x0EContent-Disposition: form-data; name="_msg_body"
The Content-Disposition header contains an invalid character (0x0E).
----0000--
`,
},
Output: profile.ExpectedOutput{
TriggeredRules: []int{200002, 200003},
},
},
},
},
},
{
Title: "multipart error invalid 0x20",
Stages: []profile.Stage{
{
Stage: profile.SubStage{
Input: profile.StageInput{
URI: "/test.php",
Headers: map[string]string{
"Host": "www.example.com",
"Content-Type": "multipart/form-data; boundary=--0000",
},
Data: `
----0000
Content-\x20Disposition: form-data; name="file"; filename="1.php"
0x20 character is expected to be the last invalid character before the valid range.
Therefore, the parser should fail and raise MULTIPART_STRICT_ERROR.
----0000--
`,
},
Output: profile.ExpectedOutput{
TriggeredRules: []int{200002, 200003},
},
},
},
},
},
},
Rules: `
SecRuleEngine DetectionOnly
SecRequestBodyAccess On
SecRule REQBODY_ERROR "!@eq 0" \
"id:'200002', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}'"
SecRule MULTIPART_STRICT_ERROR "!@eq 0" \
"id:'200003',phase:2,t:none,log,deny,status:400, msg:'Multipart request body failed strict validation."
`,
})

0 comments on commit e1c5b1b

Please sign in to comment.