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 != "" {