Skip to content

fix: read the request body back from disk in modsecurity_request_body_end_raw - #3670

Open
fzipi wants to merge 1 commit into
owasp-modsecurity:v2/masterfrom
fzipi:v2/issue-3300-reqbody-disk
Open

fzipi wants to merge 1 commit into
owasp-modsecurity:v2/masterfrom
fzipi:v2/issue-3300-reqbody-disk

Conversation

@fzipi

@fzipi fzipi commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

What

modsecurity_request_body_end_raw (apache2/msc_reqbody.c) now handles a body that was moved to disk: it reads the temporary file back through the existing modsecurity_request_body_retrieve_* API into the raw buffer, sized from the file (msc_reqbody_length only counts the bytes that were stored in memory before the spill), sets msc_reqbody_length to the real size, and leaves the storage mode on disk. modsecurity_request_body_clear frees that buffer, which in this case is not owned by a chunk.

Why

Once a request body exceeds SecRequestBodyInMemoryLimit it is written to a temporary file and the in-memory chunk array is released (msc_reqbody_chunks = NULL). modsecurity_request_body_end_raw still walked that array, so any processor that needs the raw body for such a request crashed the worker with a NULL dereference. In practice that is URLENCODED forced through ctl:requestBodyProcessor on a non form content type, which is what the backtrace in #3300 shows. It only triggers for bodies above the in-memory limit, hence the "only some POSTs" symptom.

Tests

New tests/regression/misc/40-reqbody-disk.t (auto-discovered by the runner): SecRequestBodyInMemoryLimit 64, a text/plain POST of ~215 bytes switched to URLENCODED by ctl, with a rule on ARGS:a and one on REQUEST_BODY.

Verified in a Debian bookworm container (apache2 2.4, --enable-assertions --enable-pcre-study --enable-pcre-jit):

  • untouched v2/master: both cases fail with child pid ... exit signal Segmentation fault (11) in error.log;
  • this branch: both cases return 403 (String match "1" at ARGS:a, String match "&b=2" at REQUEST_BODY).

References

Fixes #3300

🤖 Generated with Claude Code

…_end_raw

Once a request body exceeds SecRequestBodyInMemoryLimit it is written to
a temporary file and the in-memory chunk array is released
(msc_reqbody_chunks = NULL). modsecurity_request_body_end_raw still
walked that array to build the single raw buffer, so any processor that
needs the raw body for such a request, in practice URLENCODED forced
through ctl:requestBodyProcessor on a non form content type, crashed the
worker with a NULL dereference.

When the storage is on disk the body is now read back through the
existing modsecurity_request_body_retrieve API into the raw buffer, the
storage mode is left on disk, and modsecurity_request_body_clear frees
the buffer that in this case is not owned by a chunk.

Fixes owasp-modsecurity#3300

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0181jBv5JdNQviwDzfnkf1Gy
@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a4034b15-0483-4133-82a8-72f07fcff6d8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant