From f11948b681fb3af8738f768b6175eac2464f2eb5 Mon Sep 17 00:00:00 2001 From: Juan Pablo Tosso Date: Sat, 1 Oct 2022 13:25:01 -0300 Subject: [PATCH 1/6] add base for MULTIPART_PART_HEADERS --- internal/corazawaf/transaction.go | 1 + internal/corazawaf/waf.go | 2 ++ types/variables.go | 2 +- types/variables/variables.go | 3 +++ 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index 8587503b2..d8436fb31 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -1048,6 +1048,7 @@ type TransactionVariables struct { XML *collection.Map RequestXML *collection.Map ResponseXML *collection.Map + MultipartPartHeaders *collection.Map // Persistent variables IP *collection.Map // Translation Proxy Variables diff --git a/internal/corazawaf/waf.go b/internal/corazawaf/waf.go index c419fa325..3d61367af 100644 --- a/internal/corazawaf/waf.go +++ b/internal/corazawaf/waf.go @@ -342,6 +342,8 @@ func (w *WAF) NewTransaction(ctx context.Context) *Transaction { tx.Collections[variables.ResponseXML] = tx.Variables.ResponseXML tx.Variables.RequestXML = collection.NewMap(variables.RequestXML) tx.Collections[variables.RequestXML] = tx.Variables.RequestXML + tx.Variables.MultipartPartHeaders = collection.NewMap(variables.MultipartPartHeaders) + tx.Collections[variables.MultipartPartHeaders] = tx.Variables.MultipartPartHeaders tx.Variables.ArgsCombinedSize = collection.NewCollectionSizeProxy(variables.ArgsCombinedSize, tx.Variables.ArgsGet, tx.Variables.ArgsPost) tx.Collections[variables.ArgsCombinedSize] = tx.Variables.ArgsCombinedSize diff --git a/types/variables.go b/types/variables.go index d99745cba..2a6ccfca7 100644 --- a/types/variables.go +++ b/types/variables.go @@ -5,4 +5,4 @@ package types // VariablesCount contains the number of variables handled by the variables package // It is used to create arrays of the correct size -const VariablesCount = 91 +const VariablesCount = 92 diff --git a/types/variables/variables.go b/types/variables/variables.go index c50fe077c..f2329e902 100644 --- a/types/variables/variables.go +++ b/types/variables/variables.go @@ -223,6 +223,8 @@ const ( RequestXML // XML is a pointer to ResponseXML XML + // MultipartPartHeaders contains the multipart headers + MultipartPartHeaders ) var rulemap = map[RuleVariable]string{ @@ -317,6 +319,7 @@ var rulemap = map[RuleVariable]string{ RequestXML: "REQUEST_XML", ResponseXML: "RESPONSE_XML", ResponseArgs: "RESPONSE_ARGS", + MultipartPartHeaders: "MULTIPART_PART_HEADERS", } var rulemapRev = map[string]RuleVariable{} From f264c92fd706786be8fe84693f2acf0411ed7938 Mon Sep 17 00:00:00 2001 From: Juan Pablo Tosso Date: Sat, 1 Oct 2022 13:56:16 -0300 Subject: [PATCH 2/6] add body processor for mp headers and tests --- bodyprocessors/multipart.go | 9 ++++- bodyprocessors/multipart_test.go | 65 ++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/bodyprocessors/multipart.go b/bodyprocessors/multipart.go index c2be7731f..889a715f9 100644 --- a/bodyprocessors/multipart.go +++ b/bodyprocessors/multipart.go @@ -39,6 +39,7 @@ func (mbp *multipartBodyProcessor) ProcessRequest(reader io.Reader, collections postCol := (collections[variables.ArgsPost]).(*collection.Map) filesCombinedSizeCol := (collections[variables.FilesCombinedSize]).(*collection.Simple) filesNamesCol := (collections[variables.FilesNames]).(*collection.Map) + headersNames := (collections[variables.MultipartPartHeaders]).(*collection.Map) for { p, err := mr.NextPart() if err == io.EOF { @@ -47,7 +48,13 @@ func (mbp *multipartBodyProcessor) ProcessRequest(reader io.Reader, collections if err != nil { return err } - + partName := p.FormName() + for key, values := range p.Header { + for _, value := range values { + // TODO preallocate? + headersNames.Add(partName, fmt.Sprintf("%s: %s", key, value)) + } + } // if is a file filename := originFileName(p) if filename != "" { diff --git a/bodyprocessors/multipart_test.go b/bodyprocessors/multipart_test.go index 9c525f114..7c1ebbdd5 100644 --- a/bodyprocessors/multipart_test.go +++ b/bodyprocessors/multipart_test.go @@ -2,3 +2,68 @@ // SPDX-License-Identifier: Apache-2.0 package bodyprocessors + +import ( + "strings" + "testing" + + "github.com/corazawaf/coraza/v3/collection" + "github.com/corazawaf/coraza/v3/types" + "github.com/corazawaf/coraza/v3/types/variables" +) + +func TestMultipartPayload(t *testing.T) { + payload := strings.TrimSpace(` +-----------------------------9051914041544843365972754266 +Content-Disposition: form-data; name="text" + +text default +-----------------------------9051914041544843365972754266 +Content-Disposition: form-data; name="file1"; filename="a.txt" +Content-Type: text/plain + +Content of a.txt. + +-----------------------------9051914041544843365972754266 +Content-Disposition: form-data; name="file2"; filename="a.html" +Content-Type: text/html + +Content of a.html. + +-----------------------------9051914041544843365972754266-- +`) + mp := &multipartBodyProcessor{} + collections := createCollections() + if err := mp.ProcessRequest(strings.NewReader(payload), collections, Options{ + Mime: "multipart/form-data; boundary=---------------------------9051914041544843365972754266", + }); err != nil { + t.Fatal(err) + } + // first we validate we got the headers + headers := collections[variables.MultipartPartHeaders].(*collection.Map) + if h := headers.Get("file2"); len(h) == 0 { + t.Fatal("expected headers for file2") + } else { + if len(h) != 2 { + t.Fatal("expected 2 headers for file2") + } + if h[0] != "Content-Disposition: form-data; name=\"file2\"; filename=\"a.html\"" { + t.Fatal("expected Content-Disposition header for file2") + } + if h[1] != "Content-Type: text/html" { + t.Fatal("expected Content-Type header for file2") + } + } +} + +func createCollections() [types.VariablesCount]collection.Collection { + collections := [types.VariablesCount]collection.Collection{} + collections[variables.Files] = collection.NewMap(variables.Files) + collections[variables.FilesTmpNames] = collection.NewMap(variables.FilesTmpNames) + collections[variables.FilesSizes] = collection.NewMap(variables.FilesSizes) + collections[variables.ArgsPost] = collection.NewMap(variables.ArgsPost) + collections[variables.FilesCombinedSize] = collection.NewSimple(variables.FilesCombinedSize) + collections[variables.FilesNames] = collection.NewMap(variables.FilesNames) + collections[variables.MultipartPartHeaders] = collection.NewMap(variables.MultipartPartHeaders) + return collections +} From c97d753c09c6a69ee616e81d954ddf53fb492b87 Mon Sep 17 00:00:00 2001 From: Juan Pablo Tosso Date: Sat, 1 Oct 2022 13:59:22 -0300 Subject: [PATCH 3/6] update tests --- bodyprocessors/multipart_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bodyprocessors/multipart_test.go b/bodyprocessors/multipart_test.go index 7c1ebbdd5..ff1111807 100644 --- a/bodyprocessors/multipart_test.go +++ b/bodyprocessors/multipart_test.go @@ -48,10 +48,10 @@ Content-Type: text/html t.Fatal("expected 2 headers for file2") } if h[0] != "Content-Disposition: form-data; name=\"file2\"; filename=\"a.html\"" { - t.Fatal("expected Content-Disposition header for file2") + t.Fatalf("expected Content-Disposition header for file2, got %s", h[0]) } if h[1] != "Content-Type: text/html" { - t.Fatal("expected Content-Type header for file2") + t.Fatalf("expected Content-Type header for file2, got %s", h[1]) } } } From 8a849ac5602c9aef43627d01f4eb7df5351e9931 Mon Sep 17 00:00:00 2001 From: Juan Pablo Tosso Date: Sat, 1 Oct 2022 14:03:29 -0300 Subject: [PATCH 4/6] fix test race condition --- bodyprocessors/multipart_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bodyprocessors/multipart_test.go b/bodyprocessors/multipart_test.go index ff1111807..3f496043e 100644 --- a/bodyprocessors/multipart_test.go +++ b/bodyprocessors/multipart_test.go @@ -41,17 +41,16 @@ Content-Type: text/html } // first we validate we got the headers headers := collections[variables.MultipartPartHeaders].(*collection.Map) + header1 := "Content-Disposition: form-data; name=\"file2\"; filename=\"a.html\"" + header2 := "Content-Type: text/html" if h := headers.Get("file2"); len(h) == 0 { t.Fatal("expected headers for file2") } else { if len(h) != 2 { t.Fatal("expected 2 headers for file2") } - if h[0] != "Content-Disposition: form-data; name=\"file2\"; filename=\"a.html\"" { - t.Fatalf("expected Content-Disposition header for file2, got %s", h[0]) - } - if h[1] != "Content-Type: text/html" { - t.Fatalf("expected Content-Type header for file2, got %s", h[1]) + if (h[0] != header1 && h[0] != header2) || (h[1] != header1 && h[1] != header2) { + t.Fatalf("Got invalid multipart headers") } } } From 1d503dff543a8d6dda55b448ff4bf65a960a9ad7 Mon Sep 17 00:00:00 2001 From: Juan Pablo Tosso Date: Sat, 1 Oct 2022 14:09:14 -0300 Subject: [PATCH 5/6] add more tests --- testing/engine/multipart.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testing/engine/multipart.go b/testing/engine/multipart.go index 1e0d72529..aaf5cd11f 100644 --- a/testing/engine/multipart.go +++ b/testing/engine/multipart.go @@ -42,7 +42,7 @@ airween `, }, Output: profile.ExpectedOutput{ - TriggeredRules: []int{100}, + TriggeredRules: []int{100, 999997, 999998, 999999}, NonTriggeredRules: []int{150, 200002}, }, }, @@ -54,6 +54,9 @@ airween SecRequestBodyAccess On SecRule ARGS_POST:_msg_body "Hi" "id:100, phase:2,log" SecRule ARGS_GET:_msg_body "Hi" "id:150, phase:2,log" +SecRule ARGS:_msg_body "@rx Hi Martin," "id:999997, phase:2,log" +SecRule MULTIPART_PART_HEADERS:_msg_body "Content-Disposition" "id:999998, phase:2, log" +SecRule MULTIPART_PART_HEADERS "Content-Disposition" "id:999999, phase:2, log" 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" `, From f00c968f9654352e5a62b2d3ffbcf2e9106308cc Mon Sep 17 00:00:00 2001 From: Juan Pablo Tosso Date: Sun, 2 Oct 2022 11:48:30 -0300 Subject: [PATCH 6/6] minor changes --- bodyprocessors/multipart.go | 1 - testing/engine/multipart.go | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/bodyprocessors/multipart.go b/bodyprocessors/multipart.go index 889a715f9..7d1a71b90 100644 --- a/bodyprocessors/multipart.go +++ b/bodyprocessors/multipart.go @@ -51,7 +51,6 @@ func (mbp *multipartBodyProcessor) ProcessRequest(reader io.Reader, collections partName := p.FormName() for key, values := range p.Header { for _, value := range values { - // TODO preallocate? headersNames.Add(partName, fmt.Sprintf("%s: %s", key, value)) } } diff --git a/testing/engine/multipart.go b/testing/engine/multipart.go index aaf5cd11f..b56b14bdd 100644 --- a/testing/engine/multipart.go +++ b/testing/engine/multipart.go @@ -42,7 +42,7 @@ airween `, }, Output: profile.ExpectedOutput{ - TriggeredRules: []int{100, 999997, 999998, 999999}, + TriggeredRules: []int{100, 200, 250, 300}, NonTriggeredRules: []int{150, 200002}, }, }, @@ -54,9 +54,9 @@ airween SecRequestBodyAccess On SecRule ARGS_POST:_msg_body "Hi" "id:100, phase:2,log" SecRule ARGS_GET:_msg_body "Hi" "id:150, phase:2,log" -SecRule ARGS:_msg_body "@rx Hi Martin," "id:999997, phase:2,log" -SecRule MULTIPART_PART_HEADERS:_msg_body "Content-Disposition" "id:999998, phase:2, log" -SecRule MULTIPART_PART_HEADERS "Content-Disposition" "id:999999, phase:2, log" +SecRule ARGS:_msg_body "@rx Hi Martin," "id:200, phase:2,log" +SecRule MULTIPART_PART_HEADERS:_msg_body "Content-Disposition" "id:250, phase:2, log" +SecRule MULTIPART_PART_HEADERS "Content-Disposition" "id:300, phase:2, log" 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" `,