From 93593ff36c46aaf539d73abf8cc2d1754f4a8b37 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 14 Dec 2023 09:42:20 -0300 Subject: [PATCH 1/5] chore(deps): update github/codeql-action action to v3 (#941) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/codeql-analysis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index b29985f6b..0648fd0ab 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -13,12 +13,12 @@ jobs: uses: actions/checkout@v4 - name: Initialize CodeQL - uses: github/codeql-action/init@v2 + uses: github/codeql-action/init@v3 with: languages: go - name: Autobuild - uses: github/codeql-action/autobuild@v2 + uses: github/codeql-action/autobuild@v3 - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v2 + uses: github/codeql-action/analyze@v3 From fc1596e54f7dab4e9bebeafda57911e7d58bac57 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 14 Dec 2023 14:14:14 -0300 Subject: [PATCH 2/5] fix(deps): update module github.com/mccutchen/go-httpbin/v2 to v2.13.1 (#939) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index c9ffa5c58..ef518d7a4 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/corazawaf/libinjection-go v0.1.2 github.com/foxcpp/go-mockdns v1.0.0 github.com/magefile/mage v1.15.0 - github.com/mccutchen/go-httpbin/v2 v2.12.0 + github.com/mccutchen/go-httpbin/v2 v2.13.1 github.com/petar-dambovaliev/aho-corasick v0.0.0-20230725210150-fb29fc3c913e github.com/tidwall/gjson v1.17.0 golang.org/x/net v0.19.0 diff --git a/go.sum b/go.sum index 06215a1c6..ec6c6109a 100644 --- a/go.sum +++ b/go.sum @@ -8,6 +8,8 @@ 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.12.0 h1:MPrFw/Avug0E83SN/j5SYDuD9By0GDAJ9hNTR4RwjyU= github.com/mccutchen/go-httpbin/v2 v2.12.0/go.mod h1:f4DUXYlU6yH0V81O4lJIwqpmYdTXXmYwzxMnYEimFPk= +github.com/mccutchen/go-httpbin/v2 v2.13.1 h1:mDTz2RTD3tugs1BKZM7o6YJsXODYWNvjKZko30B/aWk= +github.com/mccutchen/go-httpbin/v2 v2.13.1/go.mod h1:f4DUXYlU6yH0V81O4lJIwqpmYdTXXmYwzxMnYEimFPk= github.com/miekg/dns v1.1.25/go.mod h1:bPDLeHnStXmXAq1m/Ch/hvfNHr14JKNPMBo3VZKjuso= github.com/miekg/dns v1.1.50 h1:DQUfb9uc6smULcREF09Uc+/Gd46YWqJd5DbpPE9xkcA= github.com/miekg/dns v1.1.50/go.mod h1:e3IlAVfNqAllflbibAZEWOXOQ+Ynzk/dDozDxY7XnME= From 3b532a6df9ee403a0a7468826fba022907a33daa Mon Sep 17 00:00:00 2001 From: blotus Date: Mon, 18 Dec 2023 16:48:01 +0100 Subject: [PATCH 3/5] feat: add uppercase transformation (#935) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add uppercase transformation * add uppercase.json for tests --------- Co-authored-by: Felipe Zipitría <3012076+fzipi@users.noreply.github.com> --- .../transformations/testdata/uppercase.json | 38 ++++++++++ internal/transformations/transformations.go | 1 + internal/transformations/uppercase.go | 15 ++++ internal/transformations/uppercase_test.go | 69 +++++++++++++++++++ 4 files changed, 123 insertions(+) create mode 100644 internal/transformations/testdata/uppercase.json create mode 100644 internal/transformations/uppercase.go create mode 100644 internal/transformations/uppercase_test.go diff --git a/internal/transformations/testdata/uppercase.json b/internal/transformations/testdata/uppercase.json new file mode 100644 index 000000000..c8b8f15e9 --- /dev/null +++ b/internal/transformations/testdata/uppercase.json @@ -0,0 +1,38 @@ +[ + { + "input" : "", + "name" : "uppercase", + "type" : "tfn", + "ret" : 0, + "output" : "" + }, + { + "output" : "TESTCASE", + "ret" : 0, + "name" : "uppercase", + "input" : "testcase", + "type" : "tfn" + }, + { + "ret" : 0, + "input" : "test\u0000case", + "type" : "tfn", + "name" : "uppercase", + "output" : "TEST\u0000CASE" + }, + { + "output" : "TESTCASE", + "input" : "testcase", + "name" : "uppercase", + "type" : "tfn", + "ret" : 1 + }, + { + "ret" : 1, + "input" : "Test\u0000Case", + "name" : "uppercase", + "type" : "tfn", + "output" : "TEST\u0000CASE" + } + ] + \ No newline at end of file diff --git a/internal/transformations/transformations.go b/internal/transformations/transformations.go index bcfa533b6..75a9f58fa 100644 --- a/internal/transformations/transformations.go +++ b/internal/transformations/transformations.go @@ -51,6 +51,7 @@ func init() { Register("replaceComments", replaceComments) Register("replaceNulls", replaceNulls) Register("sha1", sha1T) + Register("uppercase", upperCase) Register("urlDecode", urlDecode) Register("urlDecodeUni", urlDecodeUni) Register("urlEncode", urlEncode) diff --git a/internal/transformations/uppercase.go b/internal/transformations/uppercase.go new file mode 100644 index 000000000..c3b6da8e7 --- /dev/null +++ b/internal/transformations/uppercase.go @@ -0,0 +1,15 @@ +// Copyright 2022 Juan Pablo Tosso and the OWASP Coraza contributors +// SPDX-License-Identifier: Apache-2.0 + +package transformations + +import ( + "strings" +) + +func upperCase(data string) (string, bool, error) { + // TODO: Explicit implementation of ToUpper would allow optimizing away the byte by byte comparison for returning the changed boolean + // See https://github.com/corazawaf/coraza/pull/778#discussion_r1186963422 + transformedData := strings.ToUpper(data) + return transformedData, data != transformedData, nil +} diff --git a/internal/transformations/uppercase_test.go b/internal/transformations/uppercase_test.go new file mode 100644 index 000000000..e2172e09d --- /dev/null +++ b/internal/transformations/uppercase_test.go @@ -0,0 +1,69 @@ +// Copyright 2022 Juan Pablo Tosso and the OWASP Coraza contributors +// SPDX-License-Identifier: Apache-2.0 + +package transformations + +import "testing" + +func TestUpperCase(t *testing.T) { + tests := []struct { + input string + want string + }{ + { + input: "TestCase", + want: "TESTCASE", + }, + { + input: "test\u0000case", + want: "TEST\u0000CASE", + }, + { + input: "TESTCASE", + want: "TESTCASE", + }, + { + input: "", + want: "", + }, + { + input: "ThIs Is A tExT fOr TeStInG uPPerCAse FuNcTiOnAlItY.", + want: "THIS IS A TEXT FOR TESTING UPPERCASE FUNCTIONALITY.", + }, + } + + for _, tc := range tests { + tt := tc + t.Run(tt.input, func(t *testing.T) { + have, changed, err := upperCase(tt.input) + if err != nil { + t.Error(err) + } + if tt.input == tt.want && changed || tt.input != tt.want && !changed { + t.Errorf("input %q, have %q with changed %t", tt.input, have, changed) + } + if have != tt.want { + t.Errorf("have %q, want %q", have, tt.want) + } + }) + } +} + +func BenchmarkUppercase(b *testing.B) { + tests := []string{ + "tesTcase", + "ThIs Is A tExT fOr TeStInG lOwErCaSe FuNcTiOnAlItY.ThIs Is A tExT fOr TeStInG lOwErCaSe FuNcTiOnAlItY. ThIs Is A tExT fOr TeStInG lOwErCaSe FuNcTiOnAlItY.ThIs Is A tExT fOr TeStInG lOwErCaSe FuNcTiOnAlItY.", + } + for i := 0; i < b.N; i++ { + for _, tt := range tests { + b.Run(tt, func(b *testing.B) { + for j := 0; j < b.N; j++ { + _, _, err := upperCase(tt) + if err != nil { + b.Error(err) + } + } + }) + } + } +} From f1cfd138d37a320463e02c831520aec9d73bc75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Zipitr=C3=ADa?= <3012076+fzipi@users.noreply.github.com> Date: Mon, 18 Dec 2023 12:48:50 -0300 Subject: [PATCH 4/5] fix: parse multiple cookies with spaces (#943) * fix: parse multiple cookies with spaces --------- Signed-off-by: Felipe Zipitria --- go.sum | 2 - internal/cookies/cookies.go | 36 ++++++++++ internal/cookies/cookies_test.go | 97 ++++++++++++++++++++++++++ internal/corazawaf/transaction.go | 19 ++++- internal/corazawaf/transaction_test.go | 22 ++++++ internal/url/url.go | 5 -- 6 files changed, 171 insertions(+), 10 deletions(-) create mode 100644 internal/cookies/cookies.go create mode 100644 internal/cookies/cookies_test.go diff --git a/go.sum b/go.sum index ec6c6109a..a66a41595 100644 --- a/go.sum +++ b/go.sum @@ -6,8 +6,6 @@ github.com/foxcpp/go-mockdns v1.0.0 h1:7jBqxd3WDWwi/6WhDvacvH1XsN3rOLXyHM1uhvIx6 github.com/foxcpp/go-mockdns v1.0.0/go.mod h1:lgRN6+KxQBawyIghpnl5CezHFGS9VLzvtVlwxvzXTQ4= 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.12.0 h1:MPrFw/Avug0E83SN/j5SYDuD9By0GDAJ9hNTR4RwjyU= -github.com/mccutchen/go-httpbin/v2 v2.12.0/go.mod h1:f4DUXYlU6yH0V81O4lJIwqpmYdTXXmYwzxMnYEimFPk= github.com/mccutchen/go-httpbin/v2 v2.13.1 h1:mDTz2RTD3tugs1BKZM7o6YJsXODYWNvjKZko30B/aWk= github.com/mccutchen/go-httpbin/v2 v2.13.1/go.mod h1:f4DUXYlU6yH0V81O4lJIwqpmYdTXXmYwzxMnYEimFPk= github.com/miekg/dns v1.1.25/go.mod h1:bPDLeHnStXmXAq1m/Ch/hvfNHr14JKNPMBo3VZKjuso= diff --git a/internal/cookies/cookies.go b/internal/cookies/cookies.go new file mode 100644 index 000000000..250e45397 --- /dev/null +++ b/internal/cookies/cookies.go @@ -0,0 +1,36 @@ +package cookies + +import ( + "net/textproto" + "strings" +) + +// ParseCookies parses cookies and splits in name, value pairs. Won't check for valid names nor values. +// If there are multiple cookies with the same name, it will append to the list with the same name key. +// Loosely based in the stdlib src/net/http/cookie.go +func ParseCookies(rawCookies string) map[string][]string { + cookies := make(map[string][]string) + + rawCookies = textproto.TrimString(rawCookies) + + if rawCookies == "" { + return cookies + } + + var part string + for len(rawCookies) > 0 { // continue since we have rest + part, rawCookies, _ = strings.Cut(rawCookies, ";") + part = textproto.TrimString(part) + if part == "" { + continue + } + name, val, _ := strings.Cut(part, "=") + name = textproto.TrimString(name) + // if name is empty (eg: "Cookie: =foo;") skip it + if name == "" { + continue + } + cookies[name] = append(cookies[name], val) + } + return cookies +} diff --git a/internal/cookies/cookies_test.go b/internal/cookies/cookies_test.go new file mode 100644 index 000000000..83ad1466c --- /dev/null +++ b/internal/cookies/cookies_test.go @@ -0,0 +1,97 @@ +package cookies + +import ( + "testing" +) + +func equalMaps(map1 map[string][]string, map2 map[string][]string) bool { + if len(map1) != len(map2) { + return false + } + + // Iterate through the key-value pairs of the first map + for key, slice1 := range map1 { + // Check if the key exists in the second map + slice2, ok := map2[key] + if !ok { + return false + } + + // Compare the values of the corresponding keys + for i, val1 := range slice1 { + val2 := slice2[i] + + // Compare the elements + if val1 != val2 { + return false + } + } + } + + return true +} + +func TestParseCookies(t *testing.T) { + type args struct { + rawCookies string + } + tests := []struct { + name string + args args + want map[string][]string + }{ + { + name: "EmptyString", + args: args{rawCookies: " "}, + want: map[string][]string{}, + }, + { + name: "SimpleCookie", + args: args{rawCookies: "test=test_value"}, + want: map[string][]string{"test": {"test_value"}}, + }, + { + name: "MultipleCookies", + args: args{rawCookies: "test1=test_value1; test2=test_value2"}, + want: map[string][]string{"test1": {"test_value1"}, "test2": {"test_value2"}}, + }, + { + name: "SpacesInCookieName", + args: args{rawCookies: " test1 =test_value1; test2 =test_value2"}, + want: map[string][]string{"test1": {"test_value1"}, "test2": {"test_value2"}}, + }, + { + name: "SpacesInCookieValue", + args: args{rawCookies: "test1=test _value1; test2 =test_value2"}, + want: map[string][]string{"test1": {"test _value1"}, "test2": {"test_value2"}}, + }, + { + name: "EmptyCookie", + args: args{rawCookies: ";;foo=bar"}, + want: map[string][]string{"foo": {"bar"}}, + }, + { + name: "EmptyName", + args: args{rawCookies: "=bar;"}, + want: map[string][]string{}, + }, + { + name: "MultipleEqualsInValues", + args: args{rawCookies: "test1=val==ue1;test2=value2"}, + want: map[string][]string{"test1": {"val==ue1"}, "test2": {"value2"}}, + }, + { + name: "RepeatedCookieNameShouldGiveList", + args: args{rawCookies: "test1=value1;test1=value2"}, + want: map[string][]string{"test1": {"value1", "value2"}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ParseCookies(tt.args.rawCookies) + if !equalMaps(got, tt.want) { + t.Errorf("ParseCookies() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index da65096f3..720c78272 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -22,6 +22,7 @@ import ( "github.com/corazawaf/coraza/v3/internal/auditlog" "github.com/corazawaf/coraza/v3/internal/bodyprocessors" "github.com/corazawaf/coraza/v3/internal/collections" + "github.com/corazawaf/coraza/v3/internal/cookies" "github.com/corazawaf/coraza/v3/internal/corazarules" "github.com/corazawaf/coraza/v3/internal/corazatypes" stringsutil "github.com/corazawaf/coraza/v3/internal/strings" @@ -320,9 +321,21 @@ func (tx *Transaction) AddRequestHeader(key string, value string) { tx.variables.reqbodyProcessor.Set("MULTIPART") } case "cookie": - // Cookies use the same syntax as GET params but with semicolon (;) separator - // WithoutUnescape is used to avoid implicitly performing an URL decode on the cookies - values := urlutil.ParseQueryWithoutUnescape(value, ';') + // 4.2. Cookie + // + // 4.2.1. Syntax + // + // The user agent sends stored cookies to the origin server in the + // Cookie header. If the server conforms to the requirements in + // Section 4.1 (and the user agent conforms to the requirements in + // Section 5), the user agent will send a Cookie header that conforms to + // the following grammar: + // + // cookie-header = "Cookie:" OWS cookie-string OWS + // cookie-string = cookie-pair *( ";" SP cookie-pair ) + // + // There is no URL Decode performed no the cookies + values := cookies.ParseCookies(value) for k, vr := range values { for _, v := range vr { tx.variables.requestCookies.Add(k, v) diff --git a/internal/corazawaf/transaction_test.go b/internal/corazawaf/transaction_test.go index da57336ae..34ccb9a3e 100644 --- a/internal/corazawaf/transaction_test.go +++ b/internal/corazawaf/transaction_test.go @@ -835,6 +835,28 @@ func TestCookiesNotUrldecoded(t *testing.T) { } } +func TestMultipleCookiesWithSpaceBetweenThem(t *testing.T) { + waf := NewWAF() + tx := waf.NewTransaction() + multipleCookies := "cookie1=value1; cookie2=value2; cookie1=value2" + tx.AddRequestHeader("cookie", multipleCookies) + v11 := tx.variables.requestCookies.Get("cookie1")[0] + if v11 != "value1" { + t.Errorf("failed to set cookie, got %q", v11) + } + v12 := tx.variables.requestCookies.Get("cookie1")[1] + if v12 != "value2" { + t.Errorf("failed to set cookie, got %q", v12) + } + v2 := tx.variables.requestCookies.Get("cookie2")[0] + if v2 != "value2" { + t.Errorf("failed to set cookie, got %q", v2) + } + if err := tx.Close(); err != nil { + t.Error(err) + } +} + func collectionValues(t *testing.T, col collection.Collection) []string { t.Helper() var values []string diff --git a/internal/url/url.go b/internal/url/url.go index c6087d41e..ea6dca12f 100644 --- a/internal/url/url.go +++ b/internal/url/url.go @@ -13,11 +13,6 @@ func ParseQuery(query string, separator byte) map[string][]string { return doParseQuery(query, separator, true) } -// ParseQueryWithoutUnescape is a sibling of ParseQuery, but without performing URL unescape of keys and values. -func ParseQueryWithoutUnescape(query string, separator byte) map[string][]string { - return doParseQuery(query, separator, false) -} - func doParseQuery(query string, separator byte, urlUnescape bool) map[string][]string { m := make(map[string][]string) for query != "" { From b887a580da2c8d76e0abbf8f5e029735d0f89313 Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Wed, 3 Jan 2024 16:29:11 +0100 Subject: [PATCH 5/5] fix: more forgiving base64 transformation [custom implementation] (#944) * more forgiving base64 transformation * better comment about second DecodeString call * custom implementation * comments * address review, minor refactor * moves new line characters check * Extends tests with golang base64 tests * updates link with the permalink to the exact borrowed payloads * adds note about using custom implementation --- internal/transformations/base64decode.go | 82 ++++++++- internal/transformations/base64decode_test.go | 170 +++++++++++++++++- 2 files changed, 236 insertions(+), 16 deletions(-) diff --git a/internal/transformations/base64decode.go b/internal/transformations/base64decode.go index c4be9897c..76345599d 100644 --- a/internal/transformations/base64decode.go +++ b/internal/transformations/base64decode.go @@ -3,18 +3,82 @@ package transformations -import ( - "encoding/base64" +import "strings" - stringsutil "github.com/corazawaf/coraza/v3/internal/strings" -) +var base64DecMap = []byte{ + 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, + 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, + 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, + 127, 127, 127, 127, 127, 127, 127, 127, 127, 127, + 127, 127, 127, 62, 127, 127, 127, 63, 52, 53, + 54, 55, 56, 57, 58, 59, 60, 61, 127, 127, + 127, 64, 127, 127, 127, 0, 1, 2, 3, 4, + 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, + 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, + 25, 127, 127, 127, 127, 127, 127, 26, 27, 28, + 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, + 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, + 49, 50, 51, 127, 127, 127, 127, 127, +} // base64decode decodes a Base64-encoded string. +// Padding is optional. +// Partial decoding is returned up to the first invalid character (if any). +// New line characters (\r and \n) are ignored. +// Note: a custom base64 decoder is used in order to return partial decoding when an error arises. It +// would be possible to use the standard library only relying on undocumented behaviors of the decoder. +// For more context, see https://github.com/corazawaf/coraza/pull/940 func base64decode(data string) (string, bool, error) { - dec, err := base64.StdEncoding.DecodeString(data) - if err != nil { - // Forgiving implementation, which ignores invalid characters - return data, false, nil + res := doBase64decode(data) + return res, true, nil +} + +func doBase64decode(src string) string { + slen := len(src) + if slen == 0 { + return src + } + + var n, x int + var dst strings.Builder + dst.Grow(slen) + + for i := 0; i < slen; i++ { + currChar := src[i] + // new line characters are ignored. + if currChar == '\r' || currChar == '\n' { + continue + } + // If invalid character or padding reached, we stop decoding + if currChar == '=' || currChar == ' ' || currChar > 127 { + break + } + decodedChar := base64DecMap[currChar] + // Another condition of invalid character + if decodedChar == 127 { + break + } + + x = (x << 6) | int(decodedChar&0x3F) + n++ + if n == 4 { + dst.WriteByte(byte(x >> 16)) + dst.WriteByte(byte(x >> 8)) + dst.WriteByte(byte(x)) + n = 0 + x = 0 + } } - return stringsutil.WrapUnsafe(dec), true, nil + + // Handle any remaining characters + if n == 2 { + x <<= 12 + dst.WriteByte(byte(x >> 16)) + } else if n == 3 { + x <<= 6 + dst.WriteByte(byte(x >> 16)) + dst.WriteByte(byte(x >> 8)) + } + + return dst.String() } diff --git a/internal/transformations/base64decode_test.go b/internal/transformations/base64decode_test.go index d022b1659..f6842c07d 100644 --- a/internal/transformations/base64decode_test.go +++ b/internal/transformations/base64decode_test.go @@ -10,17 +10,173 @@ import ( "testing" ) -var b64DecodeTests = []string{ - "VGVzdENhc2U=", - "P.HNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==", - "VGVzdABDYXNl", +var b64DecodeTests = []struct { + name string + input string + expected string +}{ + { + name: "Valid", + input: "VGVzdENhc2U=", + expected: "TestCase", + }, + { + name: "Valid with \u0000", + input: "VGVzdABDYXNl", + expected: "Test\x00Case", + }, + { + name: "Valid without padding", + input: "VGVzdENhc2U", + expected: "TestCase", + }, + { + name: "Valid without longer padding", + input: "PA==", + expected: "<", + }, + { + name: "valid ", + input: "PFRFU1Q+", + expected: "", + }, + { + name: "Malformed base64 encoding", + input: "PHNjcmlwd", + expected: "