Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Disables implicit Cookies url decoding #928

Merged
merged 13 commits into from
Dec 11, 2023
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ coraza-waf
__debug_bin

build/

go.work.sum
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL
golang.org/x/net v0.0.0-20190923162816-aa69164e4478/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
golang.org/x/net v0.0.0-20210726213435-c6fcb2dbf985/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/net v0.18.0 h1:mIYleuAkSbHh0tCv7RvjL3F6ZVbLjq4+R7zbOn3Kokg=
golang.org/x/net v0.18.0/go.mod h1:/czyP5RqHAH4odGYxBJ1qz0+CE5WZ+2j1YgoEo8F2jQ=
golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c=
golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand All @@ -49,8 +47,6 @@ golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.14.0 h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q=
golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc=
golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
Expand Down
130 changes: 0 additions & 130 deletions go.work.sum

This file was deleted.

3 changes: 1 addition & 2 deletions http/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ SecRule &REQUEST_HEADERS:Transfer-Encoding "!@eq 0" "id:1,phase:1,deny"
}
if it == nil {
t.Fatal("Expected interruption")
}
if it.RuleID != 1 {
} else if it.RuleID != 1 {
t.Fatalf("Expected rule 1 to be triggered, got rule %d", it.RuleID)
}
if err := tx.Close(); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions internal/bodyprocessors/urlencoded.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
"github.com/corazawaf/coraza/v3/internal/collections"
"github.com/corazawaf/coraza/v3/internal/url"
urlutil "github.com/corazawaf/coraza/v3/internal/url"
)

type urlencodedBodyProcessor struct {
Expand All @@ -23,7 +23,7 @@ func (*urlencodedBodyProcessor) ProcessRequest(reader io.Reader, v plugintypes.T
}

b := buf.String()
values := url.ParseQuery(b, '&')
values := urlutil.ParseQuery(b, '&')
argsCol := v.ArgsPost()
for k, vs := range values {
argsCol.Set(k, vs)
Expand Down
5 changes: 3 additions & 2 deletions internal/corazawaf/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,8 @@ func (tx *Transaction) AddRequestHeader(key string, value string) {
}
case "cookie":
// Cookies use the same syntax as GET params but with semicolon (;) separator
values := urlutil.ParseQuery(value, ';')
// WithoutUnescape is used to avoid implicitly performing an URL decode on the cookies
values := urlutil.ParseQueryWithoutUnescape(value, ';')
for k, vr := range values {
for _, v := range vr {
tx.variables.requestCookies.Add(k, v)
Expand Down Expand Up @@ -535,7 +536,7 @@ func (tx *Transaction) GetStopWatch() string {
}

// GetField Retrieve data from collections applying exceptions
// In future releases we may remove de exceptions slice and
// In future releases we may remove the exceptions slice and
// make it easier to use
func (tx *Transaction) GetField(rv ruleVariableParams) []types.MatchData {
col := tx.Collection(rv.Variable)
Expand Down
29 changes: 29 additions & 0 deletions internal/corazawaf/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,35 @@ func TestHeaderSetters(t *testing.T) {
}
}

func TestCookiesNotUrldecoded(t *testing.T) {
waf := NewWAF()
tx := waf.NewTransaction()
fullCookie := "abc=%7Bd+e+f%7D;hij=%7Bklm%7D"
expectedUrlencodedAbcCookieValue := "%7Bd+e+f%7D"
unexpectedUrldencodedAbcCookieValue := "{d e f}"
tx.AddRequestHeader("cookie", fullCookie)
c := tx.variables.requestCookies.Get("abc")[0]
if c != expectedUrlencodedAbcCookieValue {
if c == unexpectedUrldencodedAbcCookieValue {
t.Errorf("failed to set cookie, unexpected urldecoding. Got: %q, expected: %q", unexpectedUrldencodedAbcCookieValue, expectedUrlencodedAbcCookieValue)
} else {
t.Errorf("failed to set cookie, got %q", c)
}
}
if tx.variables.requestHeaders.Get("cookie")[0] != fullCookie {
t.Errorf("failed to set request header, got: %q, expected: %q", tx.variables.requestHeaders.Get("cookie")[0], fullCookie)
}
if !utils.InSlice("cookie", collectionValues(t, tx.variables.requestHeadersNames)) {
t.Error("failed to set header name", collectionValues(t, tx.variables.requestHeadersNames))
}
if !utils.InSlice("abc", collectionValues(t, tx.variables.requestCookiesNames)) {
t.Error("failed to set cookie name")
}
if err := tx.Close(); err != nil {
t.Error(err)
}
}

func collectionValues(t *testing.T, col collection.Collection) []string {
t.Helper()
var values []string
Expand Down
Loading