From 19b9672ccd470a8b340737d1c7479dc13bbed016 Mon Sep 17 00:00:00 2001 From: Felipe Zipitria Date: Sat, 21 Feb 2026 19:36:20 -0300 Subject: [PATCH 1/5] feat: add ARGS_RAW, ARGS_GET_RAW, ARGS_POST_RAW, and ARGS_NAMES_RAW collections Add raw (non-URL-decoded) argument collections that preserve wire-format values before any percent-decoding. This enables rules to safely apply t:urlDecodeUni without double-decoding, detect double URL encoding, and behave consistently across integrations. Closes #1491 --- .../plugins/plugintypes/transaction.go | 4 + internal/bodyprocessors/json.go | 3 + internal/bodyprocessors/multipart.go | 3 + internal/bodyprocessors/urlencoded.go | 10 ++ internal/corazawaf/transaction.go | 71 ++++++++- internal/operators/validate_schema_test.go | 4 + internal/url/url.go | 6 + internal/url/url_test.go | 73 +++++++++ internal/variables/variables.go | 8 + internal/variables/variablesmap.gen.go | 20 +++ testing/engine/args_raw.go | 138 ++++++++++++++++++ types/variables/variables.go | 8 + 12 files changed, 347 insertions(+), 1 deletion(-) create mode 100644 testing/engine/args_raw.go diff --git a/experimental/plugins/plugintypes/transaction.go b/experimental/plugins/plugintypes/transaction.go index 092030d0c..4c0edd851 100644 --- a/experimental/plugins/plugintypes/transaction.go +++ b/experimental/plugins/plugintypes/transaction.go @@ -115,5 +115,9 @@ type TransactionVariables interface { ArgsNames() collection.Keyed ArgsGetNames() collection.Keyed ArgsPostNames() collection.Keyed + ArgsRaw() collection.Keyed + ArgsGetRaw() collection.Map + ArgsPostRaw() collection.Map + ArgsNamesRaw() collection.Keyed MultipartStrictError() collection.Single } diff --git a/internal/bodyprocessors/json.go b/internal/bodyprocessors/json.go index 101851f7e..fc2d43e71 100644 --- a/internal/bodyprocessors/json.go +++ b/internal/bodyprocessors/json.go @@ -27,12 +27,15 @@ func (js *jsonBodyProcessor) ProcessRequest(reader io.Reader, v plugintypes.Tran // Process as normal col := v.ArgsPost() + colRaw := v.ArgsPostRaw() data, err := readJSON(ss) if err != nil { return err } for key, value := range data { col.SetIndex(key, 0, value) + // JSON values are not URL-encoded, so raw == cooked + colRaw.SetIndex(key, 0, value) } // Store the raw JSON in the TX variable for validateSchema diff --git a/internal/bodyprocessors/multipart.go b/internal/bodyprocessors/multipart.go index ded38b2b1..a9c61fe2c 100644 --- a/internal/bodyprocessors/multipart.go +++ b/internal/bodyprocessors/multipart.go @@ -36,6 +36,7 @@ func (mbp *multipartBodyProcessor) ProcessRequest(reader io.Reader, v plugintype filesTmpNamesCol := v.FilesTmpNames() fileSizesCol := v.FilesSizes() postCol := v.ArgsPost() + postRawCol := v.ArgsPostRaw() filesCombinedSizeCol := v.FilesCombinedSize() filesNamesCol := v.FilesNames() headersNames := v.MultipartPartHeaders() @@ -94,6 +95,8 @@ func (mbp *multipartBodyProcessor) ProcessRequest(reader io.Reader, v plugintype } totalSize += int64(len(data)) postCol.Add(p.FormName(), string(data)) + // Multipart fields are not URL-encoded, so raw == cooked + postRawCol.Add(p.FormName(), string(data)) } filesCombinedSizeCol.(*collections.Single).Set(fmt.Sprintf("%d", totalSize)) } diff --git a/internal/bodyprocessors/urlencoded.go b/internal/bodyprocessors/urlencoded.go index fe0176bc4..6a7fd37d6 100644 --- a/internal/bodyprocessors/urlencoded.go +++ b/internal/bodyprocessors/urlencoded.go @@ -23,11 +23,21 @@ func (*urlencodedBodyProcessor) ProcessRequest(reader io.Reader, v plugintypes.T } b := buf.String() + + // Cooked (URL-decoded) values values := urlutil.ParseQuery(b, '&') argsCol := v.ArgsPost() for k, vs := range values { argsCol.Set(k, vs) } + + // Raw (non-decoded) values + rawValues := urlutil.ParseQueryRaw(b, '&') + argsRawCol := v.ArgsPostRaw() + for k, vs := range rawValues { + argsRawCol.Set(k, vs) + } + v.RequestBody().(*collections.Single).Set(b) v.RequestBodyLength().(*collections.Single).Set(strconv.Itoa(len(b))) return nil diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index 730a850b1..ac38394a0 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -257,6 +257,14 @@ func (tx *Transaction) Collection(idx variables.RuleVariable) collection.Collect return tx.variables.argsGetNames case variables.ArgsPostNames: return tx.variables.argsPostNames + case variables.ArgsRaw: + return tx.variables.argsRaw + case variables.ArgsGetRaw: + return tx.variables.argsGetRaw + case variables.ArgsPostRaw: + return tx.variables.argsPostRaw + case variables.ArgsNamesRaw: + return tx.variables.argsNamesRaw case variables.ResBodyProcessor: return tx.variables.resBodyProcessor case variables.TX: @@ -693,7 +701,7 @@ func (tx *Transaction) ProcessConnection(client string, cPort int, server string tx.variables.serverPort.Set(p2) } -// ExtractGetArguments transforms an url encoded string to a map and creates ARGS_GET +// ExtractGetArguments transforms an url encoded string to a map and creates ARGS_GET and ARGS_GET_RAW func (tx *Transaction) ExtractGetArguments(uri string) { data := urlutil.ParseQuery(uri, '&') for k, vs := range data { @@ -701,6 +709,13 @@ func (tx *Transaction) ExtractGetArguments(uri string) { tx.AddGetRequestArgument(k, v) } } + // Populate raw (non-decoded) collection + rawData := urlutil.ParseQueryRaw(uri, '&') + for k, vs := range rawData { + for _, v := range vs { + tx.addGetRequestArgumentRaw(k, v) + } + } } // AddGetRequestArgument @@ -730,6 +745,13 @@ func (tx *Transaction) AddPathRequestArgument(key string, value string) { tx.variables.argsPath.Add(key, value) } +func (tx *Transaction) addGetRequestArgumentRaw(key string, value string) { + if tx.checkArgumentLimit(tx.variables.argsGetRaw) { + return + } + tx.variables.argsGetRaw.Add(key, value) +} + func (tx *Transaction) checkArgumentLimit(c *collections.NamedCollection) bool { return c.Len() >= tx.WAF.ArgumentLimit } @@ -1659,10 +1681,14 @@ type TransactionVariables struct { argsCombinedSize *collections.SizeCollection argsGet *collections.NamedCollection argsGetNames collection.Keyed + argsGetRaw *collections.NamedCollection argsNames *collections.ConcatKeyed + argsNamesRaw *collections.ConcatKeyed argsPath *collections.NamedCollection argsPost *collections.NamedCollection argsPostNames collection.Keyed + argsPostRaw *collections.NamedCollection + argsRaw *collections.ConcatKeyed duration *collections.Single env *collections.Map files *collections.Map @@ -1829,10 +1855,14 @@ func NewTransactionVariables() *TransactionVariables { v.argsGet = collections.NewCaseSensitiveNamedCollection(variables.ArgsGet) v.argsPost = collections.NewCaseSensitiveNamedCollection(variables.ArgsPost) v.argsPath = collections.NewCaseSensitiveNamedCollection(variables.ArgsPath) + v.argsGetRaw = collections.NewCaseSensitiveNamedCollection(variables.ArgsGetRaw) + v.argsPostRaw = collections.NewCaseSensitiveNamedCollection(variables.ArgsPostRaw) } else { v.argsGet = collections.NewNamedCollection(variables.ArgsGet) v.argsPost = collections.NewNamedCollection(variables.ArgsPost) v.argsPath = collections.NewNamedCollection(variables.ArgsPath) + v.argsGetRaw = collections.NewNamedCollection(variables.ArgsGetRaw) + v.argsPostRaw = collections.NewNamedCollection(variables.ArgsPostRaw) } v.argsGetNames = v.argsGet.Names(variables.ArgsGetNames) @@ -1851,6 +1881,17 @@ func NewTransactionVariables() *TransactionVariables { // Only used in a concatenating collection so variable name doesn't matter. v.argsPath.Names(variables.Unknown), ) + v.argsRaw = collections.NewConcatKeyed( + variables.ArgsRaw, + v.argsGetRaw, + v.argsPostRaw, + ) + v.argsNamesRaw = collections.NewConcatKeyed( + variables.ArgsNamesRaw, + // Only used in a concatenating collection so variable name doesn't matter. + v.argsGetRaw.Names(variables.Unknown), + v.argsPostRaw.Names(variables.Unknown), + ) return v } @@ -2138,6 +2179,22 @@ func (v *TransactionVariables) ArgsPostNames() collection.Keyed { return v.argsPostNames } +func (v *TransactionVariables) ArgsRaw() collection.Keyed { + return v.argsRaw +} + +func (v *TransactionVariables) ArgsGetRaw() collection.Map { + return v.argsGetRaw +} + +func (v *TransactionVariables) ArgsPostRaw() collection.Map { + return v.argsPostRaw +} + +func (v *TransactionVariables) ArgsNamesRaw() collection.Keyed { + return v.argsNamesRaw +} + func (v *TransactionVariables) ResBodyError() collection.Single { return v.resBodyError } @@ -2187,6 +2244,18 @@ func (v *TransactionVariables) All(f func(v variables.RuleVariable, col collecti if !f(variables.ArgsPostNames, v.argsPostNames) { return } + if !f(variables.ArgsPostRaw, v.argsPostRaw) { + return + } + if !f(variables.ArgsRaw, v.argsRaw) { + return + } + if !f(variables.ArgsGetRaw, v.argsGetRaw) { + return + } + if !f(variables.ArgsNamesRaw, v.argsNamesRaw) { + return + } if !f(variables.Duration, v.duration) { return } diff --git a/internal/operators/validate_schema_test.go b/internal/operators/validate_schema_test.go index 60e5c1c3f..82bb699ed 100644 --- a/internal/operators/validate_schema_test.go +++ b/internal/operators/validate_schema_test.go @@ -417,6 +417,10 @@ func (m *mockTransaction) ArgsPath() collection.Map { ret func (m *mockTransaction) ArgsNames() collection.Keyed { return nil } func (m *mockTransaction) ArgsGetNames() collection.Keyed { return nil } func (m *mockTransaction) ArgsPostNames() collection.Keyed { return nil } +func (m *mockTransaction) ArgsRaw() collection.Keyed { return nil } +func (m *mockTransaction) ArgsGetRaw() collection.Map { return nil } +func (m *mockTransaction) ArgsPostRaw() collection.Map { return nil } +func (m *mockTransaction) ArgsNamesRaw() collection.Keyed { return nil } func (m *mockTransaction) Duration() collection.Single { return nil } func (m *mockTransaction) Files() collection.Map { return nil } func (m *mockTransaction) FilesNames() collection.Map { return nil } diff --git a/internal/url/url.go b/internal/url/url.go index ea6dca12f..7c3f97bf8 100644 --- a/internal/url/url.go +++ b/internal/url/url.go @@ -13,6 +13,12 @@ func ParseQuery(query string, separator byte) map[string][]string { return doParseQuery(query, separator, true) } +// ParseQueryRaw splits a URL-encoded query string into key/value pairs +// WITHOUT percent-decoding, preserving the original wire-format encoding. +func ParseQueryRaw(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 != "" { diff --git a/internal/url/url_test.go b/internal/url/url_test.go index 7194f4a53..13022d75b 100644 --- a/internal/url/url_test.go +++ b/internal/url/url_test.go @@ -49,3 +49,76 @@ func BenchmarkQueryUnescape(b *testing.B) { } } } + +func TestParseQueryRaw(t *testing.T) { + tests := []struct { + name string + input string + want map[string][]string + }{ + { + name: "preserves percent-encoded values", + input: "key=%3Cscript%3E&other=hello", + want: map[string][]string{ + "key": {"%3Cscript%3E"}, + "other": {"hello"}, + }, + }, + { + name: "preserves double encoding", + input: "password=Secret%2500", + want: map[string][]string{ + "password": {"Secret%2500"}, + }, + }, + { + name: "preserves plus signs", + input: "q=hello+world", + want: map[string][]string{ + "q": {"hello+world"}, + }, + }, + { + name: "preserves encoded key names", + input: "p%61ssword=test", + want: map[string][]string{ + "p%61ssword": {"test"}, + }, + }, + { + name: "empty value", + input: "key=", + want: map[string][]string{ + "key": {""}, + }, + }, + { + name: "no value (no equals)", + input: "key", + want: map[string][]string{ + "key": {""}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ParseQueryRaw(tt.input, '&') + for k, wantVals := range tt.want { + gotVals, ok := got[k] + if !ok { + t.Errorf("missing key %q", k) + continue + } + if len(gotVals) != len(wantVals) { + t.Errorf("key %q: got %d values, want %d", k, len(gotVals), len(wantVals)) + continue + } + for i, wv := range wantVals { + if gotVals[i] != wv { + t.Errorf("key %q[%d]: got %q, want %q", k, i, gotVals[i], wv) + } + } + } + }) + } +} diff --git a/internal/variables/variables.go b/internal/variables/variables.go index ba57269a2..75d089802 100644 --- a/internal/variables/variables.go +++ b/internal/variables/variables.go @@ -166,6 +166,14 @@ const ( ArgsGetNames // CanBeSelected // ArgsPostNames contains the names of the POST arguments ArgsPostNames // CanBeSelected + // ArgsRaw contains copies of ArgsGetRaw and ArgsPostRaw (wire-format, not URL-decoded) + ArgsRaw // CanBeSelected + // ArgsGetRaw contains the GET (URL) arguments without URL decoding + ArgsGetRaw // CanBeSelected + // ArgsPostRaw contains the POST (BODY) arguments without URL decoding + ArgsPostRaw // CanBeSelected + // ArgsNamesRaw contains the names of the arguments without URL decoding + ArgsNamesRaw // CanBeSelected // TX contains transaction specific variables created with setvar TX // CanBeSelected // Rule contains rule metadata diff --git a/internal/variables/variablesmap.gen.go b/internal/variables/variablesmap.gen.go index d1bad5831..c167a6f45 100644 --- a/internal/variables/variablesmap.gen.go +++ b/internal/variables/variablesmap.gen.go @@ -140,6 +140,14 @@ func (v RuleVariable) Name() string { return "ARGS_GET_NAMES" case ArgsPostNames: return "ARGS_POST_NAMES" + case ArgsRaw: + return "ARGS_RAW" + case ArgsGetRaw: + return "ARGS_GET_RAW" + case ArgsPostRaw: + return "ARGS_POST_RAW" + case ArgsNamesRaw: + return "ARGS_NAMES_RAW" case TX: return "TX" case Rule: @@ -279,6 +287,14 @@ func (v RuleVariable) CanBeSelected() bool { return true case ArgsPostNames: return true + case ArgsRaw: + return true + case ArgsGetRaw: + return true + case ArgsPostRaw: + return true + case ArgsNamesRaw: + return true case TX: return true case Rule: @@ -366,6 +382,10 @@ var rulemapRev = map[string]RuleVariable{ "ARGS_NAMES": ArgsNames, "ARGS_GET_NAMES": ArgsGetNames, "ARGS_POST_NAMES": ArgsPostNames, + "ARGS_RAW": ArgsRaw, + "ARGS_GET_RAW": ArgsGetRaw, + "ARGS_POST_RAW": ArgsPostRaw, + "ARGS_NAMES_RAW": ArgsNamesRaw, "TX": TX, "RULE": Rule, "JSON": JSON, diff --git a/testing/engine/args_raw.go b/testing/engine/args_raw.go new file mode 100644 index 000000000..a52eef8d6 --- /dev/null +++ b/testing/engine/args_raw.go @@ -0,0 +1,138 @@ +// Copyright 2024 Juan Pablo Tosso and the OWASP Coraza contributors +// SPDX-License-Identifier: Apache-2.0 + +package engine + +import ( + "github.com/corazawaf/coraza/v3/testing/profile" +) + +var _ = profile.RegisterProfile(profile.Profile{ + Meta: profile.Meta{ + Author: "fzipi", + Description: "Test ARGS_RAW, ARGS_GET_RAW, ARGS_POST_RAW, and ARGS_NAMES_RAW collections", + Enabled: true, + Name: "args_raw.yaml", + }, + Tests: []profile.Test{ + { + Title: "ARGS_GET_RAW preserves URL-encoded values", + Stages: []profile.Stage{ + { + Stage: profile.SubStage{ + Input: profile.StageInput{ + URI: "/index.php?key=%3Cscript%3E&other=hello", + Method: "GET", + }, + Output: profile.ExpectedOutput{ + TriggeredRules: []int{ + 100, // ARGS_GET_RAW:key matches %3Cscript%3E (raw) + 101, // ARGS_GET:key matches