From a10ab2c2e6a26d2f279aa478c59b4c88a78e813b Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Sat, 5 Oct 2024 15:41:43 +0200 Subject: [PATCH 1/2] fix MultipartStrictError, updates tests to CRS v4.6 adding proper overrides --- internal/corazawaf/transaction.go | 8 +-- testing/coraza_test.go | 3 +- testing/coreruleset/.ftw-overrides.yml | 21 ++++++++ testing/coreruleset/go.mod | 4 +- testing/coreruleset/go.sum | 8 +-- testing/engine/multipart.go | 72 ++++++++++++++++++++++++++ 6 files changed, 103 insertions(+), 13 deletions(-) diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index 36159c6e5..c05128271 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -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 @@ -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 diff --git a/testing/coraza_test.go b/testing/coraza_test.go index 70f83a2d6..8edaff198 100644 --- a/testing/coraza_test.go +++ b/testing/coraza_test.go @@ -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) } diff --git a/testing/coreruleset/.ftw-overrides.yml b/testing/coreruleset/.ftw-overrides.yml index 0d32f24d4..3773d7e2d 100644 --- a/testing/coreruleset/.ftw-overrides.yml +++ b/testing/coreruleset/.ftw-overrides.yml @@ -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] diff --git a/testing/coreruleset/go.mod b/testing/coreruleset/go.mod index ddae7cca9..8fcd0de38 100644 --- a/testing/coreruleset/go.mod +++ b/testing/coreruleset/go.mod @@ -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 @@ -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 diff --git a/testing/coreruleset/go.sum b/testing/coreruleset/go.sum index 5efec5a34..f04fded22 100644 --- a/testing/coreruleset/go.sum +++ b/testing/coreruleset/go.sum @@ -11,14 +11,10 @@ github.com/corazawaf/coraza-coreruleset/v4 v4.5.0/go.mod h1:1FQt1p+JSQ6tYrafMqZr 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= @@ -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= diff --git a/testing/engine/multipart.go b/testing/engine/multipart.go index b56b14bdd..9c0923d47 100644 --- a/testing/engine/multipart.go +++ b/testing/engine/multipart.go @@ -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." + `, +}) From 3703ce9bbf5d9759673d07faa78d4c1057904300 Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Sat, 5 Oct 2024 15:44:44 +0200 Subject: [PATCH 2/2] go mod tidy --- examples/http-server/go.mod | 2 +- examples/http-server/go.sum | 4 ++-- go.sum | 4 ---- testing/coreruleset/go.sum | 4 ++-- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/examples/http-server/go.mod b/examples/http-server/go.mod index 11ab0d944..5ffc77031 100644 --- a/examples/http-server/go.mod +++ b/examples/http-server/go.mod @@ -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 diff --git a/examples/http-server/go.sum b/examples/http-server/go.sum index bde8489e6..3912e9463 100644 --- a/examples/http-server/go.sum +++ b/examples/http-server/go.sum @@ -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= diff --git a/go.sum b/go.sum index fde1e3ef5..78b991429 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/testing/coreruleset/go.sum b/testing/coreruleset/go.sum index f04fded22..425673122 100644 --- a/testing/coreruleset/go.sum +++ b/testing/coreruleset/go.sum @@ -6,8 +6,8 @@ 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=