## Root Cause File: `internal/corazawaf/transaction.go`, lines 770–808 (since commit 2fd87b89, PR #812, 2023-06-14) ```go func (tx *Transaction) AddGetRequestArgument(key string, value string) { if tx.checkArgumentLimit(tx.variables.argsGet) { tx.debugLogger.Warn().Msg("skipping get request argument, over limit") return } tx.variables.argsGet.Add(key, value) } func (tx *Transaction) checkArgumentLimit(c *collections.NamedCollection) bool { return c.Len() >= tx.WAF.ArgumentLimit } ``` `AddGetRequestArgument`, `AddPostRequestArgument`, and `AddPathRequestArgument` silently `return` once the per-collection argument count reaches `WAF.ArgumentLimit` (default `1000`, see `internal/corazawaf/waf.go:346`). No error variable is set, no transaction flag is raised, and no rule can observe that a drop occurred. Worse, `ExtractGetArguments` (`transaction.go:761`) iterates the `map[string][]string` returned by `urlutil.ParseQuery`: ```go func (tx *Transaction) ExtractGetArguments(uri string) { data := urlutil.ParseQuery(uri, '&') for k, vs := range data { // Go map iteration order is randomized for _, v := range vs { tx.AddGetRequestArgument(k, v) } } } ``` Because Go randomizes map iteration order, which of the caller-supplied arguments survive the limit is non-deterministic. An attacker can pad the URI with filler arguments; any one of them — including the malicious payload — may be the one silently discarded, and therefore invisible to every `SecRule` targeting `ARGS`, `ARGS_GET`, or `ARGS_NAMES`. ### Secondary finding — POST urlencoded processor bypasses the cap entirely `internal/bodyprocessors/urlencoded.go:29` populates `ARGS_POST` without invoking `checkArgumentLimit`: ```go values := urlutil.ParseQuery(b, '&') argsCol := v.ArgsPost() for k, vs := range values { argsCol.Set(k, vs) // direct write, no limit check } ``` So `AddPostRequestArgument`'s cap is effectively dead code for real urlencoded bodies. `ARGS_POST` grows unbounded — both a bypass surface and a memory-DoS surface. ## Impact Any `SecRule` or CRS rule that inspects `ARGS`, `ARGS_GET`, `ARGS_NAMES`, `ARGS_GET_NAMES`, or `ARGS_PATH` can be evaded by inflating the request's argument count past `SecArgumentsLimit` (default `1000`). The bypass probability per request scales with overflow: | Total args in request | Observed bypass rate of ARGS rule | |---|---| | 1000 (at limit) | 0 / 50 (0.0%) | | 1001 (1 over) | 0 / 2000 (< 0.1%) | | 1100 (100 over) | 45 / 500 (9.0%) | | 2000 (1000 over) | 106 / 200 (53.0%) | | 10000 (10× limit) | 47 / 50 (94.0%) | The rate matches the theoretical model `(N − limit) / N`. An attacker flooding with 10000 arguments lands a bypass on ~94% of requests; one failed attempt costs them nothing, so a handful of retries yields a near-certain evasion against any ARGS-targeted rule, including the OWASP CRS SQLi, XSS, RCE, and LFI detection families. The issue is silent — operators see no audit-log entry, no error, and no `MULTIPART_STRICT_ERROR`-style flag variable, because none exists. ## Proof of Concept Start a Coraza-wrapped HTTP server with a trivial ARGS rule: ```conf SecRuleEngine On SecRule ARGS "@contains ATTACK_HERE_XYZ" "id:9001,phase:2,deny,status:403,msg:'Attack detected'" ``` Baseline sanity checks pass: ``` $ curl -s -o /dev/null -w '%{http_code}\n' 'http://127.0.0.1:8090/?evil=ATTACK_HERE_XYZ' 403 ``` Now pad the URI with 9999 filler parameters and one malicious parameter placed at a random offset. Running 50 such trials against a real HTTP listener with real `curl`: ``` === 10000 args (attacker adds 9999 filler parameters) === total_args=10000 trials=50 BLOCKED=3 BYPASS=47 (94.0%) ``` 47 of 50 attack requests were served `HTTP 200` despite the payload being present in the URI. The defending rule never fired because Coraza discarded the argument before phase:2 evaluation. ## Comparison with ModSecurity v3 The engine-level bug is present in ModSecurity v3 as well — `src/transaction.cc:282-291` has the equivalent silent-drop: ```cpp bool Transaction::addArgument(...) { if (m_rules->m_argumentsLimit.m_set && m_variableArgs.size() >= m_rules->m_argumentsLimit.m_value) { ms_dbg(4, "Skipping request argument, over limit (...)") return false; // return value is ignored at the GET callsite } ... } ``` ModSecurity is in fact *more deterministic* than Coraza — its query-string parser (`extractArguments`, `transaction.cc:254`) splits with an ordered `ssplit`, so it is the *tail* of the query that silently drops. An attacker places the payload first and pads the tail; no retry loop needed. **However, ModSecurity's default configuration papers over the engine bug.** `modsecurity.conf-recommended` ships: ```conf SecArgumentsLimit 1000 # If SecArgumentsLimit has been set, you probably want to reject any # request body that has only been partly parsed. The value used in this # rule should match what was used with SecArgumentsLimit SecRule &ARGS "@ge 1000" \ "id:'200007',phase:2,t:none,log,deny,status:400,msg:'Failed to fully parse request body due to large argument count',severity:2" ``` Because `addArgument` caps the single `m_variableArgs` collection at exactly the limit, `&ARGS == limit` iff the limit was hit — rule 200007 converts silent-drop into explicit `HTTP 400`. ModSecurity also has a complementary `REQBODY_ERROR` path: its JSON processor cancels parsing on `addArgument` failure, and rule 200002 denies on `REQBODY_ERROR` (verified by `test/test-cases/regression/secargumentslimit.json`, test 2/2). **Coraza's `coraza.conf-recommended` ships no equivalent rule.** That is what makes the bug exploitable out-of-the-box in Coraza and not in ModSecurity. | | Silent-drop at engine | Compensating default rule | Exploitable out-of-the-box | |---|---|---|---| | ModSecurity v3 | yes | **yes** (`id:200007`, `&ARGS @ge 1000`) | no — denies at limit | | Coraza v3 | yes | no | **yes** | ## Mitigation Recommended fixes, in the order they should be applied. Config-layer (#1) closes the default-install exposure quickly; engine-layer (#2, #3) is the durable fix. ### 1. Ship compensating rules in `coraza.conf-recommended` (config-layer, immediate) Port the ModSecurity guard, but **keyed per-collection** — Coraza caps `ARGS_GET`, `ARGS_POST`, and `ARGS_PATH` independently, unlike ModSecurity's unified `m_variableArgs`. A single `&ARGS @ge 1000` check on the concatenated collection would false-positive at e.g. GET=500 + POST=500 (no drops occurred but aggregate == 1000): ```conf SecRule &ARGS_GET "@ge 1000" \ "id:200007,phase:2,t:none,log,deny,status:400,msg:'ARGS_GET over SecArgumentsLimit; request partially parsed'" SecRule &ARGS_POST "@ge 1000" \ "id:200008,phase:2,t:none,log,deny,status:400,msg:'ARGS_POST over SecArgumentsLimit; request partially parsed'" SecRule &ARGS_PATH "@ge 1000" \ "id:200009,phase:2,t:none,log,deny,status:400,msg:'ARGS_PATH over SecArgumentsLimit; request partially parsed'" ``` Both `&VAR` (variable count, `internal/seclang/rule_parser.go:40`) and `@ge` (`internal/operators/testdata/ge.json`) are supported. Thresholds must track `SecArgumentsLimit` if the operator overrides it. ### 2. Expose a transaction-visible flag (engine-layer, durable) Introduce an `ARGUMENTS_LIMIT_REACHED` collection variable, analogous to `MULTIPART_STRICT_ERROR` and `URLENCODED_ERROR`, set to `1` by `AddGetRequestArgument` / `AddPostRequestArgument` / `AddPathRequestArgument` whenever they drop. Replace the config rules above with a single engine-backed check: ```conf SecRule ARGUMENTS_LIMIT_REACHED "@eq 1" \ "id:200006,phase:1,t:none,log,deny,status:413,msg:'Argument limit reached; request rejected'" ``` This protects operators with hand-rolled configurations, not just those who use the recommended file. ### 3. Close the POST urlencoded body-processor gap `internal/bodyprocessors/urlencoded.go` should route through `AddPostRequestArgument` (or invoke `checkArgumentLimit` explicitly) so the cap is actually enforced for urlencoded request bodies. Currently a 10000-arg POST body populates `ARGS_POST` in full, regardless of `SecArgumentsLimit`. ### 4. Make `ExtractGetArguments` order-deterministic Replace the `urlutil.ParseQuery → map → range` pattern with an ordered slice-based parse. Combined with #2, this means when the limit is hit the outcome is at least deterministic (fail-closed via the flag) rather than a probabilistic game. ## Affected versions All releases since `v3.0.0` that ship the `SecArgumentsLimit` directive (introduced in PR #812, commit `2fd87b89`, June 2023). Confirmed reproducible on `main` at commit `599ae64a` with default configuration. ## References - `internal/corazawaf/transaction.go` lines 770–808 - `internal/corazawaf/waf.go` line 346 (`ArgumentLimit: 1000`) - `internal/bodyprocessors/urlencoded.go` line 29 (POST-side gap) - `internal/seclang/rule_parser.go:40` (`&VAR` count syntax) - `internal/collections/concat_test.go:20` (`ARGS` as `ConcatCollection` of `ARGS_GET`/`ARGS_POST`/`ARGS_PATH`) - PR #812 — introduction of `SecArgumentsLimit` - ModSecurity v3 `src/transaction.cc:282-291` (same silent-drop) - ModSecurity v3 `modsecurity.conf-recommended` rule `id:200007` (compensating config-layer deny) ## Resolution (2026-07-28) Fixed in https://github.com/corazawaf/coraza-ghsa-6r3q-mjv7-xr8m/pull/1, implementing all four mitigation steps above, plus additional gaps found while verifying the fix (see below): 1. **Compensating `coraza.conf-recommended` rules** — shipped as `ARGUMENTS_LIMIT_REACHED`-based rules (`id:200004`/`200005`, phase:1 for GET/PATH and phase:2 for POST), per-flag rather than the originally-sketched per-collection `&ARGS_GET`/`&ARGS_POST`/`&ARGS_PATH` counts, since the flag (below) already distinguishes GET/PATH-time drops from POST-time drops without needing separate threshold rules per collection. 2. **`ARGUMENTS_LIMIT_REACHED` transaction variable** — added, set by every argument-adding path that can drop: `AddGetRequestArgument`, `AddPostRequestArgument`, `AddPathRequestArgument`, `AddResponseArgument`, the urlencoded body processor, and (see below) the JSON body processor and the query-string/urlencoded parser itself. 3. **`internal/bodyprocessors/urlencoded.go` now enforces the limit** — threads `ArgumentLimit` through `BodyProcessorOptions` into the body processor, closing the POST-side gap. 4. **`ExtractGetArguments` is now order-deterministic** — via `ParseQueryOrdered`, so when the limit is hit the tail is dropped predictably instead of a randomized subset. ### Additional gaps found while verifying the fix (folded into the same PR) While confirming this fix actually closed the class of bug, three more instances of the same underlying "argument limit isn't really enforced" problem turned up, overlapping with an independently-reported advisory, **GHSA-3ww9-vw83-9w5x** (JSON/urlencoded body processors ignore SecArgumentsLimit, enabling memory-exhaustion DoS): - **The JSON body processor had zero enforcement at all** (GHSA-3ww9's actual reported bug, with a working PoC: a small body decoding to a wide flat JSON array like `[1,1,1,...]` expanded into millions of `ARGS_POST` entries, exhausting memory on a single request). `readJSON`/`readItems` now stop flattening once `ArgumentLimit` entries are collected, for both request (`ARGS_POST`) and response (`RESPONSE_ARGS`) bodies — response previously received an empty `BodyProcessorOptions{}` with no limit at all. - **`ParseQuery`/`ParseQueryOrdered` built their entire result before any caller-side cap ran.** Even after fixing (3)/(4) above, a query string or urlencoded body with millions of pairs still spent the memory during parsing itself, before any limit check downstream ever got a chance to run. Both now accept a `limit` and stop parsing immediately once reached. - **`checkArgumentLimit` (and `AddResponseArgument`'s equivalent) compared against `Len()`, which counts distinct keys, not total values.** `Map.Add` appends repeated-key values into the same map entry without growing it, so `a=1&a=1&a=1...` never tripped the limit no matter how large it grew — confirmed empirically: 1,000,000 repeats of `a=1&` via the already-"protected" GET-argument path produced ~60MB of unbounded heap growth despite `SecArgumentsLimit 1000`. Added `Map.TotalValues()` (cheap even under this attack — it sums `len(slice)` per key, so cost is bounded by distinct keys present, not by how many values piled up under any single one of them) and switched both checks to use it. Verified before/after with heap measurements and direct parser unit tests (exactly `limit` entries returned regardless of a 1,000,000-entry adversarial input, for both repeated-key and distinct-key shapes). Full repo `go test ./... -race`, `go vet`, `gofmt`, `golangci-lint` all clean. `BenchmarkReadJSONArgumentLimit` shows the fix also cuts CPU time ~17x on a 100k-element flat array (741µs vs 12.5ms), since capped iteration stops early instead of walking the whole structure. GHSA-3ww9-vw83-9w5x's own description has been updated to point here rather than duplicating this fix in a second PR. ## Follow-up (2026-09-30): byte-budget bypass in the array-length write path The "Resolution" section above states that `readJSON`/`readItems` "stop flattening once `ArgumentLimit` entries are collected". That is true for every per-leaf write, but not for the array-length summary entry written after each `gjson.ForEach` call returns (`internal/bodyprocessors/json.go`, the `if arrayLen > 0` block). That write checked `argumentLimit` but never `byteBudget`: ```go if arrayLen > 0 { if argumentLimit > 0 && *argCount >= argumentLimit { iterationTruncated = true } else { k := string(objKey) lenStr := strconv.Itoa(arrayLen) res[k] = append(res[k], lenStr) *usedBytes += len(objKey) + len(lenStr) // accounted for, but never checked against byteBudget first *argCount++ } } ``` `objKey` (the full flattened path) grows by roughly a fixed amount per nesting level, while `argCount` grows by only one per level. That is exactly the amplification `byteBudget` exists to bound (see `flattenBytesFactor`), but only the per-leaf write inside the `ForEach` callback checks it before writing; this post-`ForEach` write does not. A long property name nested under many single-element arrays inflates memory far past the configured byte budget while `argumentLimit` alone never trips, because each nesting level contributes only one argument, however long its path. ### PoC ```go const keyLen = 20000 const depth = 200 body := `{"` + strings.Repeat("a", keyLen) + `":` + strings.Repeat("[", depth) + strings.Repeat("]", depth) + `}` res, truncated, err := readJSON(body, 1024, 1000) ``` Against `main` at commit `19b86824`: a 20,405-byte body produces 199 entries totalling 4,020,596 bytes of flattened keys (~197x the body size) and `truncated=false, err=nil` -- the byte budget for a body this size is `len(body) * flattenBytesFactor` (~163 KB), so this is roughly 25x over budget with no signal to the caller. A reviewer measured +961 MB heap growth end-to-end for a 1 MB body with a long property name. The recommended `SecRequestBodyLimit` (12.5 MiB) admits proportionally larger amplification. Because every `ARGS_NAMES`-targeted regex in CRS scans these flattened keys, this is also a CPU cost, not just memory. `ProcessResponse` shares the same `readJSON`/`readItems` code path, so `RESPONSE_ARGS` is affected identically. ### Fix Move the byte-budget check into the same `else` branch as the argument-limit check, computing `lenStr` first so its length is known before the check (mirroring the per-leaf write's own check three lines above it): ```go if argumentLimit > 0 && *argCount >= argumentLimit { iterationTruncated = true } else { lenStr := strconv.Itoa(arrayLen) if byteBudget > 0 && *usedBytes+len(objKey)+len(lenStr) > byteBudget { iterationTruncated = true } else { k := string(objKey) res[k] = append(res[k], lenStr) *usedBytes += len(objKey) + len(lenStr) *argCount++ } } ``` Verified: the PoC above now returns `truncated=true`, with total flattened bytes bounded by the byte budget (163,160 bytes measured, vs. 4,020,596 before the fix). ### AI involvement disclosure - **AI tools/models used:** Claude Sonnet 5 (Anthropic), via Claude Code. - **What was generated/assisted:** the vulnerability hypothesis and repro shape were supplied by the reporter as an existing written finding; Claude Sonnet 5 independently re-derived the root cause by reading the current source, wrote and ran a fresh PoC and heap measurement against commit `19b86824`, confirmed the amplification and lack of truncation, verified the fix closes the gap, and drafted this addendum. - **Review performed:** reproduced by hand by running the PoC above against a clean checkout of commit `19b86824` before and after the fix, comparing entry count, total flattened bytes, and the `truncated` flag; added and ran `TestReadJSONArrayLengthWriteRespectsByteBudget`, confirmed it fails against the pre-fix code (4,020,596 bytes, `truncated=false`) and passes against the fix; ran the full test suite, the build-tag matrix (`coraza.no_memoize`, `coraza.rule.multiphase_evaluation`, `coraza.rule.no_regex_multiline`), and the `testing/coreruleset` CRS regression suite, all green; reviewed by a human maintainer (fzipi) before this addendum was submitted. Fix: https://github.com/corazawaf/coraza-ghsa-6r3q-mjv7-xr8m/pull/2 ### Patched in 3.8.1 The 3.8.0 fix was incomplete. 3.8.1 completes it: array-length entries produced while flattening JSON bodies are now held to the flattening byte budget (previously they could amplify a small body into a large memory allocation), a body over that budget now sets `REQBODY_ERROR`, and array-length entries no longer count toward `SecArgumentsLimit`. Upgrade to 3.8.1; 3.8.0 is listed as affected. ### Severity (revised 2026-10-02) `CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:C/C:N/I:L/A:L` (7.2, High). Attack Complexity is Low: filler arguments alone trigger the drop, on any deployment, and since 3.8.0 the drop is deterministic. Availability is Low because this advisory also covers unbounded `ARGS_POST` growth from urlencoded bodies and, in 3.8.1, JSON flattening amplification, both of which consume memory per request. The previous vector scored Confidentiality Low and Availability None. Impact metrics follow the convention used across Coraza's WAF-bypass advisories: the vulnerable component is Coraza, but the impact lands on the protected application, so Scope is Changed. The bypass hides a payload from inspection; the application still has to be vulnerable to it, so Integrity is Low and Confidentiality is not scored separately. _AI involvement in this section: Claude Opus 5.5 (Anthropic), via Claude Code, re-derived the CVSS vector from the project's triage guidance (AGENTS.md, "CVSS preconditions get verified, not copied from the report") and drafted this text. A human maintainer (fzipi) chose the `S:C/I:L` impact convention and directed this update._