From f47a1c0e64da5f6c1c3b2e27d015efd9f05e0135 Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Sun, 22 Oct 2023 14:21:31 +0200 Subject: [PATCH] chore: setvar minor fix, tests, added warning when missing variable --- examples/http-server/go.mod | 2 +- experimental/plugins/macro/macro.go | 2 + go.work.sum | 1 - internal/actions/setvar.go | 30 +++---- internal/actions/setvar_test.go | 118 +++++++++++++++++++++++++++- testing/coreruleset/go.mod | 6 +- testing/coreruleset/go.sum | 12 +-- 7 files changed, 140 insertions(+), 31 deletions(-) diff --git a/examples/http-server/go.mod b/examples/http-server/go.mod index 08acd1e8c..5423bbdf3 100644 --- a/examples/http-server/go.mod +++ b/examples/http-server/go.mod @@ -1,6 +1,6 @@ module github.com/corazawaf/coraza/v3/examples/http-server -go 1.18 +go 1.19 require github.com/corazawaf/coraza/v3 v3.0.0-20220914101451-05d352c89b24 diff --git a/experimental/plugins/macro/macro.go b/experimental/plugins/macro/macro.go index 6fc10ce91..ec756f0d8 100644 --- a/experimental/plugins/macro/macro.go +++ b/experimental/plugins/macro/macro.go @@ -82,6 +82,8 @@ func expandToken(tx plugintypes.TransactionState, token macroToken) string { } } + // If the variable is known (e.g. TX) but the key is not found, we return the original text + tx.DebugLogger().Warn().Str("variable", token.variable.Name()).Str("key", token.key).Msg("key not found in collection, returning the original text") return token.text } diff --git a/go.work.sum b/go.work.sum index 569ff9723..050519ddf 100644 --- a/go.work.sum +++ b/go.work.sum @@ -1,6 +1,5 @@ golang.org/x/crypto v0.14.0 h1:wBqGXzWJW6m1XrIKlAH0Hs1JJ7+9KBwnIO8v66Q9cHc= golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= -golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= diff --git a/internal/actions/setvar.go b/internal/actions/setvar.go index a372b4410..2c3635206 100644 --- a/internal/actions/setvar.go +++ b/internal/actions/setvar.go @@ -136,20 +136,19 @@ func (a *setvarFn) evaluateTxCollection(r plugintypes.RuleMetadata, tx plugintyp col.Remove(key) return } - res := "" + currentVal := "" if r := col.Get(key); len(r) > 0 { - res = r[0] + currentVal = r[0] } var err error switch { case len(value) == 0: // if nothing to input col.Set(key, []string{""}) - case value[0] == '+': - // if we want to sum - sum := 0 + case value[0] == '+', value[0] == '-': // Increment or decrement, arithmetical operations + val := 0 if len(value) > 1 { - sum, err = strconv.Atoi(value[1:]) + val, err = strconv.Atoi(value[1:]) if err != nil { tx.DebugLogger().Error(). Str("var_value", value). @@ -159,26 +158,23 @@ func (a *setvarFn) evaluateTxCollection(r plugintypes.RuleMetadata, tx plugintyp return } } - val := 0 - if res != "" { - val, err = strconv.Atoi(res) + currentValInt := 0 + if currentVal != "" { + currentValInt, err = strconv.Atoi(currentVal) if err != nil { tx.DebugLogger().Error(). - Str("var_key", res). + Str("var_key", currentVal). Int("rule_id", r.ID()). Err(err). Msg("Invalid value") return } } - col.Set(key, []string{strconv.Itoa(sum + val)}) - case value[0] == '-': - me, _ := strconv.Atoi(value[1:]) - txv, err := strconv.Atoi(res) - if err != nil { - return + if value[0] == '+' { + col.Set(key, []string{strconv.Itoa(currentValInt + val)}) + } else { + col.Set(key, []string{strconv.Itoa(currentValInt - val)}) } - col.Set(key, []string{strconv.Itoa(txv - me)}) default: col.Set(key, []string{value}) } diff --git a/internal/actions/setvar_test.go b/internal/actions/setvar_test.go index c512c83a6..8e4358dfc 100644 --- a/internal/actions/setvar_test.go +++ b/internal/actions/setvar_test.go @@ -4,19 +4,26 @@ package actions import ( + "bytes" + "strings" "testing" + + "github.com/corazawaf/coraza/v3/collection" + "github.com/corazawaf/coraza/v3/debuglog" + "github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes" + "github.com/corazawaf/coraza/v3/internal/corazawaf" ) type md struct { } -func (_ md) ID() int { +func (md) ID() int { return 0 } -func (_ md) ParentID() int { +func (md) ParentID() int { return 0 } -func (_ md) Status() int { +func (md) Status() int { return 0 } @@ -46,3 +53,108 @@ func TestSetvarInit(t *testing.T) { } }) } + +var invalidSyntaxAtoiError = "invalid syntax" +var warningKeyNotFoundInCollection = "key not found in collection" + +func TestSetvarEvaluateErrors(t *testing.T) { + tests := []struct { + name string + init string + init2 string + expectInvalidSyntaxError bool + expectNewVarValue string + }{ + { + name: "Numerical operation + with existing variable", + init: "TX.var=5", + init2: "TX.newvar=+%{tx.var}", + expectInvalidSyntaxError: false, + expectNewVarValue: "5", + }, + { + name: "Numerical operation - with existing variable", + init: "TX.var=5", + init2: "TX.newvar=-%{tx.var}", + expectInvalidSyntaxError: false, + expectNewVarValue: "-5", + }, + { + name: "Numerical operation + with missing (or non-numerical) variable", + init: "TX.newvar=+%{tx.missingvar}", + expectInvalidSyntaxError: true, + }, + { + name: "Numerical operation - with missing (or non-numerical) variable", + init: "TX.newvar=-%{tx.missingvar}", + expectInvalidSyntaxError: true, + }, + } + + for _, tt := range tests { + logsBuf := &bytes.Buffer{} + + logger := debuglog.Default().WithLevel(debuglog.LevelWarn).WithOutput(logsBuf) + + t.Run(tt.name, func(t *testing.T) { + defer logsBuf.Reset() + a := setvar() + metadata := &md{} + if err := a.Init(metadata, tt.init); err != nil { + t.Error("unexpected error during setvar init") + } + waf := corazawaf.NewWAF() + waf.Logger = logger + tx := waf.NewTransaction() + a.Evaluate(metadata, tx) + if tt.expectInvalidSyntaxError { + if logsBuf.Len() == 0 { + t.Fatal("expected error") + } + if !strings.Contains(logsBuf.String(), invalidSyntaxAtoiError) { + t.Errorf("expected error containing %q, got %q", invalidSyntaxAtoiError, logsBuf.String()) + } + if !strings.Contains(logsBuf.String(), warningKeyNotFoundInCollection) { + t.Errorf("expected error containing %q, got %q", warningKeyNotFoundInCollection, logsBuf.String()) + } + } + if logsBuf.Len() != 0 && !tt.expectInvalidSyntaxError { + t.Fatalf("unexpected error: %s", logsBuf.String()) + } + + if tt.init2 != "" { + if err := a.Init(metadata, tt.init2); err != nil { + t.Fatal("unexpected error during setvar init") + } + a.Evaluate(metadata, tx) + if logsBuf.Len() != 0 && !tt.expectInvalidSyntaxError { + t.Fatalf("unexpected error: %s", logsBuf.String()) + } + } + if tt.expectNewVarValue != "" { + checkCollectionValue(t, a.(*setvarFn), tx, "newvar", tt.expectNewVarValue) + } + }) + } +} + +func checkCollectionValue(t *testing.T, a *setvarFn, tx plugintypes.TransactionState, key string, expected string) { + t.Helper() + var col collection.Map + if c, ok := tx.Collection(a.collection).(collection.Map); !ok { + t.Fatal("collection in setvar is not a map") + return + } else { + col = c + } + if col == nil { + t.Fatal("collection in setvar is nil") + return + } + if col == nil { + t.Fatal("collection is nil") + } + if col.Get(key)[0] != expected { + t.Errorf("key %q: expected %q, got %q", key, expected, col.Get(key)) + } +} diff --git a/testing/coreruleset/go.mod b/testing/coreruleset/go.mod index e44a834eb..be4b7e831 100644 --- a/testing/coreruleset/go.mod +++ b/testing/coreruleset/go.mod @@ -35,9 +35,9 @@ require ( github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.1 // indirect github.com/yargevad/filepathx v1.0.0 // indirect - golang.org/x/crypto v0.12.0 // indirect - golang.org/x/net v0.14.0 // indirect - golang.org/x/sys v0.11.0 // indirect + golang.org/x/crypto v0.14.0 // indirect + golang.org/x/net v0.17.0 // indirect + golang.org/x/sys v0.13.0 // indirect golang.org/x/tools v0.6.0 // indirect golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/testing/coreruleset/go.sum b/testing/coreruleset/go.sum index 8d30bf4db..ff20fd632 100644 --- a/testing/coreruleset/go.sum +++ b/testing/coreruleset/go.sum @@ -314,8 +314,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20190923035154-9ee001bba392/go.mod h1:/lpIB1dKB+9EgE3H3cr1v9wB50oz8l4C4h62xy7jSTY= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/crypto v0.12.0 h1:tFM/ta59kqch6LlvYnPa0yx5a83cL2nHflFhYKvv9Yk= -golang.org/x/crypto v0.12.0/go.mod h1:NF0Gs7EO5K4qLn+Ylc+fih8BSTeIjAP05siRnAh98yw= +golang.org/x/crypto v0.14.0 h1:wBqGXzWJW6m1XrIKlAH0Hs1JJ7+9KBwnIO8v66Q9cHc= +golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= @@ -343,8 +343,8 @@ golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwY golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210410081132-afb366fc7cd1/go.mod h1:9tjilg8BloeKEkVJvy7fQ90B1CfIiPueXVOjqfkSzI8= -golang.org/x/net v0.14.0 h1:BONx9s002vGdD9umnlX1Po8vOZmrgH34qlHcD1MfK14= -golang.org/x/net v0.14.0/go.mod h1:PpSgVXXLK0OxS0F31C1/tv6XNguvCrnXIDrFMspZIUI= +golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= +golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -391,8 +391,8 @@ golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM= -golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= +golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20181227161524-e6919f6577db/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=