fix: correctness gaps in response header/intervention handling

[fzipi]

Summary

• Content-Length: 0 was never forwarded to the WAF: the check was content_length_n > 0, dropping the legitimate zero-length case. A RESPONSE_HEADERS:Content-Length rule could never match an empty body.
• The synthesized Server header used sizeof() on a string literal, which includes the trailing NUL, embedding a literal \0 into the header value handed to the WAF — breaking exact-match rules on RESPONSE_HEADERS:Server.
• The synthesized Connection (and Keep-Alive) response header was fed to the WAF unconditionally, including on HTTP/2, where nginx never actually sends either header (RFC 9113 §8.2.2). A RESPONSE_HEADERS:Connection rule would false-positive on every HTTP/2 response.
• The response protocol string passed to msc_process_response_headers() only distinguished HTTP/1.1 and HTTP/2.0, defaulting to HTTP/1.1 for HTTP/3 requests — unlike the request side, which already reports the real protocol generically.
• When a phase:3 redirect: action turns an already-populated 200 response into a 3xx, only the Location header was synthesized; the discarded response's Content-Length/Content-Type/Last-Modified/ETag/Content-Encoding/Accept-Ranges were left in place, describing a body that is no longer sent (RFC 9110 §15.4 / §8.3-8.8).
• modsecurity_transaction_id was declared NGX_CONF_1MORE but its handler only ever reads the first argument, so a config with extra arguments was silently accepted with the extras discarded instead of rejected.
• ngx_http_modsecurity_create_ctx() allocated an unused sizeof(ctx) data buffer per request via ngx_pool_cleanup_add()'s size argument; nothing reads it since cln->data is set to the already-existing ctx pointer separately. Matches the ngx_pool_cleanup_add(cf->pool, 0) pattern already used elsewhere in this file.

Added: WAF-triggered redirects were silently dropped entirely

While building a local libmodsecurity + nginx stack to actually verify the entity-header-clearing fix above (rather than just reasoning about the code), I found the redirect path doesn't work at all, which makes that fix a no-op on the one path it was meant to cover.

ngx_http_modsecurity_header_filter() reports a redirect via ngx_http_filter_finalize_request(), which calls nginx's own ngx_http_clean_header() — this ngx_memzeros the entire headers_out struct, Location included, before nginx regenerates its own generic redirect page. Confirmed empirically: the identical redirect: rule at phase:1/2 (via access.c, which returns straight from the phase handler and never touches ngx_http_filter_finalize_request) produces a correct Location header; the same rule at phase:3 (via header_filter.c) produces a Location-less, boilerplate "302 Found" page — response is byte-identical whether or not the entity-header-clearing fix is present.

nginx's own fix for this same problem (ngx_http_send_error_page(), used by the error_page directive) isn't reusable here — it calls the static ngx_http_send_special_response(), which isn't part of the public module API.

Fix: when process_intervention() has built a Location header, skip ngx_http_filter_finalize_request() and forward the already-correct headers_out through the normal chain instead. The original response body may already be flowing from the content handler by this point, so body_filter() now drops it (ctx->response_replaced) rather than sending it alongside the redirect. Added tests/modsecurity-response-redirect.t, confirmed to fail on master (missing Location, stale Content-Length) and pass with this fix.

Test plan

• Built libmodsecurity v3 + nginx locally (macOS) against both master and this branch; confirmed the redirect fix end-to-end with curl (status, headers, and body-suppression) and confirmed no regression on plain deny (still goes through ngx_http_filter_finalize_request as before) and plain passthrough requests.
• tests/modsecurity-response-redirect.t added; ran it directly against both binaries via nginx-tests — fails on master (2/5 assertions), passes on this branch (5/5).
• CI: run the full existing tests/*.t suite (prove modsecurity*.t) on Linux to confirm no platform-specific regression (local verification was on macOS/kqueue, CI runs Linux/epoll).
• Manually verify modsecurity_transaction_id "a" "b"; is now rejected at config-test time.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[fzipi]

Summary

• Content-Length: 0 was never forwarded to the WAF: the check was content_length_n > 0, dropping the legitimate zero-length case. A RESPONSE_HEADERS:Content-Length rule could never match an empty body.
• The synthesized Server header used sizeof() on a string literal, which includes the trailing NUL, embedding a literal \0 into the header value handed to the WAF — breaking exact-match rules on RESPONSE_HEADERS:Server.
• The synthesized Connection (and Keep-Alive) response header was fed to the WAF unconditionally, including on HTTP/2, where nginx never actually sends either header (RFC 9113 §8.2.2). A RESPONSE_HEADERS:Connection rule would false-positive on every HTTP/2 response.
• The response protocol string passed to msc_process_response_headers() only distinguished HTTP/1.1 and HTTP/2.0, defaulting to HTTP/1.1 for HTTP/3 requests — unlike the request side, which already reports the real protocol generically.
• When a phase:3 redirect: action turns an already-populated 200 response into a 3xx, only the Location header was synthesized; the discarded response's Content-Length/Content-Type/Last-Modified/ETag/Content-Encoding/Accept-Ranges were left in place, describing a body that is no longer sent (RFC 9110 §15.4 / §8.3-8.8).
• modsecurity_transaction_id was declared NGX_CONF_1MORE but its handler only ever reads the first argument, so a config with extra arguments was silently accepted with the extras discarded instead of rejected.
• ngx_http_modsecurity_create_ctx() allocated an unused sizeof(ctx) data buffer per request via ngx_pool_cleanup_add()'s size argument; nothing reads it since cln->data is set to the already-existing ctx pointer separately. Matches the ngx_pool_cleanup_add(cf->pool, 0) pattern already used elsewhere in this file.

Added: WAF-triggered redirects were silently dropped entirely

While building a local libmodsecurity + nginx stack to actually verify the entity-header-clearing fix above (rather than just reasoning about the code), I found the redirect path doesn't work at all, which makes that fix a no-op on the one path it was meant to cover.

ngx_http_modsecurity_header_filter() reports a redirect via ngx_http_filter_finalize_request(), which calls nginx's own ngx_http_clean_header() — this ngx_memzeros the entire headers_out struct, Location included, before nginx regenerates its own generic redirect page. Confirmed empirically: the identical redirect: rule at phase:1/2 (via access.c, which returns straight from the phase handler and never touches ngx_http_filter_finalize_request) produces a correct Location header; the same rule at phase:3 (via header_filter.c) produces a Location-less, boilerplate "302 Found" page — response is byte-identical whether or not the entity-header-clearing fix is present.

nginx's own fix for this same problem (ngx_http_send_error_page(), used by the error_page directive) isn't reusable here — it calls the static ngx_http_send_special_response(), which isn't part of the public module API.

Fix: when process_intervention() has built a Location header, skip ngx_http_filter_finalize_request() and forward the already-correct headers_out through the normal chain instead. The original response body may already be flowing from the content handler by this point, so body_filter() now drops it (ctx->response_replaced) rather than sending it alongside the redirect. Added tests/modsecurity-response-redirect.t, confirmed to fail on master (missing Location, stale Content-Length) and pass with this fix.

Test plan

• Built libmodsecurity v3 + nginx locally (macOS) against both master and this branch; confirmed the redirect fix end-to-end with curl (status, headers, and body-suppression) and confirmed no regression on plain deny (still goes through ngx_http_filter_finalize_request as before) and plain passthrough requests.
• tests/modsecurity-response-redirect.t added; ran it directly against both binaries via nginx-tests — fails on master (2/5 assertions), passes on this branch (5/5).
• CI: run the full existing tests/*.t suite (prove modsecurity*.t) on Linux to confirm no platform-specific regression (local verification was on macOS/kqueue, CI runs Linux/epoll).
• Manually verify modsecurity_transaction_id "a" "b"; is now rejected at config-test time.

[fzipi]

Note on the failing Windows checks: they're unrelated to this PR's changes and split into two separate, pre-existing issues in test_new.yml's build-windows/windows-nginx-* jobs:

1. Flaky tarball download — gzip: stdin: unexpected end of file / tar: Child returned status 1 fetching pcre2/zlib/openssl in the "Set up third-party libraries" step. Intermittent; a retry can pass.
2. yajl/CMake incompatibility — once past (1), the Windows runner's current toolchain invokes cmake -G "Visual Studio 18 2026", and that CMake version has dropped support for CMP0026's OLD behavior, which yajl/2.1.0's Conan Center recipe (pulled in by owasp-modsecurity/ModSecurity's own vcbuild.bat) still relies on. This one is deterministic, not flaky — retries won't help until it's fixed upstream (pin/patch yajl's Conan recipe, or pin an older CMake for the Windows build in libmodsecurity).

Neither is something a ModSecurity-nginx source change can fix; the Linux checks (which do exercise the actual code changes here) are green.

[fzipi]

Filed upstream: owasp-modsecurity/ModSecurity#3604 (yajl/CMake CMP0026 incompatibility) and #388 (flaky tarball download, this repo's own test_new.yml). Both confirmed unrelated to this PR's changes.