Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a682f13
Allow partial processing of multipart, JSON, and XML request body
hnakamur Dec 25, 2025
55270e5
Adjust test since ProcessPartial no more causes "no final boundary mi…
hnakamur Dec 30, 2025
586e1ef
Fix json test match_log
hnakamur Dec 30, 2025
e9dca3c
Fix expected error message in regression test
hnakamur Dec 24, 2025
0e4728f
Accept partial epilogue in body larger than limit for ProcessPartial
hnakamur Dec 31, 2025
24508d6
Fix indent in apache2/msc_multipart.c
hnakamur Dec 31, 2025
bea943b
Fix indent in apache2/msc_json.c
hnakamur Dec 31, 2025
9b2a5fe
Add tests for url-encoded, JSON, and XML with ProcessPartial
hnakamur Jan 1, 2026
0bfb828
Refine tests for multipart with ProcessPartial
hnakamur Jan 1, 2026
43f95fd
Modify url-encoded reqbody tests for ProcesPartial
hnakamur Jan 2, 2026
3d73c02
Support partial processing of url-encoded reqbody
hnakamur Jan 2, 2026
d914f23
Add tests for MULTIPART_PART_HEADERS with ProcessPartial
hnakamur Jan 3, 2026
55a1828
Reject invalid final boundary for multipart with ProcessPartial
hnakamur Jan 3, 2026
0b599ad
Fix indent in apache2/msc_multipart.c
hnakamur Jan 3, 2026
cb95a24
Modify tests for multipart with ProcessPartial
hnakamur Jan 3, 2026
61d4e45
Modify multipart_complete for incomplete final boundary
hnakamur Jan 3, 2026
be5feee
Process an incomplete boundary as a non-final boundary
hnakamur Jan 3, 2026
50de8bb
Update tests/regression/rule/15-json.t
hnakamur Jan 23, 2026
a64b544
Update tests/regression/rule/15-json.t
hnakamur Jan 23, 2026
a83c26c
Fix spelling of reqbody_partial_processing_enabled
hnakamur Jan 23, 2026
da8b174
Fix indentations in test files
hnakamur Jan 23, 2026
f679e11
Add rules to check multipart error in tests
hnakamur Jan 23, 2026
9277589
Fix indentations in tests/regression/rule/10-xml.t
hnakamur Jan 23, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions apache2/apache2_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -299,15 +299,16 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) {
#endif
}

if (msr->reqbody_length + buflen > (apr_size_t)msr->txcfg->reqbody_limit && msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_PARTIAL) {
buflen = (apr_size_t)msr->txcfg->reqbody_limit - msr->reqbody_length;
finished_reading = 1;
modsecurity_request_body_enable_partial_processing(msr);
}

msr->reqbody_length += buflen;

if (buflen != 0) {
int rcbs = modsecurity_request_body_store(msr, buf, buflen, error_msg);

if (msr->reqbody_length > (apr_size_t)msr->txcfg->reqbody_limit && msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_PARTIAL) {
finished_reading = 1;
}

if (rcbs < 0) {
if (rcbs == -5) {
if((msr->txcfg->is_enabled == MODSEC_ENABLED) && (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT)) {
Expand Down Expand Up @@ -351,11 +352,13 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) {

msr->if_status = IF_STATUS_WANTS_TO_RUN;

if (rcbe == -5) {
return HTTP_REQUEST_ENTITY_TOO_LARGE;
}
if (rcbe < 0) {
return HTTP_INTERNAL_SERVER_ERROR;
if (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT) {
if (rcbe == -5) {
return HTTP_REQUEST_ENTITY_TOO_LARGE;
}
if (rcbe < 0) {
return HTTP_INTERNAL_SERVER_ERROR;
}
}
Comment on lines +355 to 362
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling logic has been modified to only return error codes when if_limit_action is REQUEST_BODY_LIMIT_ACTION_REJECT. However, this means that when if_limit_action is REQUEST_BODY_LIMIT_ACTION_PARTIAL, errors from modsecurity_request_body_end (rcbe) are silently ignored and APR_SUCCESS is returned. This could mask legitimate parsing errors that occur during partial processing. Consider returning errors for non -5 error codes even in ProcessPartial mode.

Suggested change
if (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT) {
if (rcbe == -5) {
return HTTP_REQUEST_ENTITY_TOO_LARGE;
}
if (rcbe < 0) {
return HTTP_INTERNAL_SERVER_ERROR;
}
}
/* Handle request body processing result:
* - rcbe == -5: body too large -> 413 only in REJECT mode, ignore otherwise.
* - other rcbe < 0: treat as internal error in all modes.
*/
if (rcbe == -5) {
if (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT) {
return HTTP_REQUEST_ENTITY_TOO_LARGE;
}
} else if (rcbe < 0) {
return HTTP_INTERNAL_SERVER_ERROR;
}

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change causes a parser error returns a response with status code 500, which is not wanted since this case is not an internal server error.
In case of a parse error, REQBODY_ERROR is set to 1, which can be used by a rule to return a 400 Bad Request response, for example:

SecRule REQBODY_ERROR "!@eq 0" "id:'200001', phase:2,t:none,log,deny,status:400,msg:'Failed to parse request body.',logdata:'%{reqbody_error_msg}',severity:2"

Side note: However, I found that no rule in OWASP CRS 4.22.0 checks REQBODY_ERROR.

This is wanted behavior for both of REQUEST_BODY_LIMIT_ACTION_REJECT and REQUEST_BODY_LIMIT_ACTION_PARTIAL.

Therefore I think the right modification here is:

    if (rcbe == -5 && msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT) {
        return HTTP_REQUEST_ENTITY_TOO_LARGE;
    }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change causes a parser error returns a response with status code 500, which is not wanted since this case is not an internal server error.

okay, then we can make this as resolved. Thanks.

Side note: However, I found that no rule in OWASP CRS 4.22.0 checks REQBODY_ERROR.

This is wanted behavior for both of REQUEST_BODY_LIMIT_ACTION_REJECT and REQUEST_BODY_LIMIT_ACTION_PARTIAL.

Good advice - would you mind to open a PR or just an issue under coreruleset?

Therefore I think the right modification here is:

    if (rcbe == -5 && msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT) {
        return HTTP_REQUEST_ENTITY_TOO_LARGE;
    }

Do you want to add/change this one too? I'm waiting for your response, until that I don't resolve this comment.

return APR_SUCCESS;
}
Expand Down
3 changes: 3 additions & 0 deletions apache2/modsecurity.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ struct modsec_rec {
unsigned int if_started_forwarding;

apr_size_t reqbody_length;
unsigned int reqbody_partial_processing_enabled;

apr_bucket_brigade *of_brigade;
unsigned int of_status;
Expand Down Expand Up @@ -736,6 +737,8 @@ apr_status_t DSOLOCAL modsecurity_request_body_start(modsec_rec *msr, char **err
apr_status_t DSOLOCAL modsecurity_request_body_store(modsec_rec *msr,
const char *data, apr_size_t length, char **error_msg);

void DSOLOCAL modsecurity_request_body_enable_partial_processing(modsec_rec *msr);

apr_status_t DSOLOCAL modsecurity_request_body_end(modsec_rec *msr, char **error_msg);

apr_status_t DSOLOCAL modsecurity_request_body_to_stream(modsec_rec *msr, const char *buffer, int buflen, char **error_msg);
Expand Down
40 changes: 22 additions & 18 deletions apache2/msc_json.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ static int yajl_start_array(void *ctx) {
msr->json->current_depth++;
if (msr->json->current_depth > msr->txcfg->reqbody_json_depth_limit) {
msr->json->depth_limit_exceeded = 1;
return 0;
return 0;
}

if (msr->txcfg->debuglog_level >= 9) {
Expand Down Expand Up @@ -262,7 +262,7 @@ static int yajl_start_map(void *ctx)
msr->json->current_depth++;
if (msr->json->current_depth > msr->txcfg->reqbody_json_depth_limit) {
msr->json->depth_limit_exceeded = 1;
return 0;
return 0;
}

if (msr->txcfg->debuglog_level >= 9) {
Expand Down Expand Up @@ -367,6 +367,10 @@ int json_init(modsec_rec *msr, char **error_msg) {
return 1;
}

void json_allow_partial_values(modsec_rec *msr) {
(void)yajl_config(msr->json->handle, yajl_allow_partial_values, 1);
}

/**
* Feed one chunk of data to the JSON parser.
*/
Expand All @@ -380,16 +384,16 @@ int json_process_chunk(modsec_rec *msr, const char *buf, unsigned int size, char
/* Feed our parser and catch any errors */
msr->json->status = yajl_parse(msr->json->handle, buf, size);
if (msr->json->status != yajl_status_ok) {
if (msr->json->depth_limit_exceeded) {
*error_msg = "JSON depth limit exceeded";
} else {
if (msr->json->yajl_error) *error_msg = msr->json->yajl_error;
else {
char* yajl_err = yajl_get_error(msr->json->handle, 0, buf, size);
*error_msg = apr_pstrdup(msr->mp, yajl_err);
yajl_free_error(msr->json->handle, yajl_err);
if (msr->json->depth_limit_exceeded) {
*error_msg = "JSON depth limit exceeded";
} else {
if (msr->json->yajl_error) *error_msg = msr->json->yajl_error;
else {
char* yajl_err = yajl_get_error(msr->json->handle, 0, buf, size);
*error_msg = apr_pstrdup(msr->mp, yajl_err);
yajl_free_error(msr->json->handle, yajl_err);
}
}
}
return -1;
}

Expand All @@ -409,13 +413,13 @@ int json_complete(modsec_rec *msr, char **error_msg) {
/* Wrap up the parsing process */
msr->json->status = yajl_complete_parse(msr->json->handle);
if (msr->json->status != yajl_status_ok) {
if (msr->json->depth_limit_exceeded) {
*error_msg = "JSON depth limit exceeded";
} else {
char *yajl_err = yajl_get_error(msr->json->handle, 0, NULL, 0);
*error_msg = apr_pstrdup(msr->mp, yajl_err);
yajl_free_error(msr->json->handle, yajl_err);
}
if (msr->json->depth_limit_exceeded) {
*error_msg = "JSON depth limit exceeded";
} else {
char *yajl_err = yajl_get_error(msr->json->handle, 0, NULL, 0);
*error_msg = apr_pstrdup(msr->mp, yajl_err);
yajl_free_error(msr->json->handle, yajl_err);
}

return -1;
}
Expand Down
2 changes: 2 additions & 0 deletions apache2/msc_json.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ struct json_data {

int DSOLOCAL json_init(modsec_rec *msr, char **error_msg);

void DSOLOCAL json_allow_partial_values(modsec_rec *msr);

int DSOLOCAL json_process(modsec_rec *msr, const char *buf,
unsigned int size, char **error_msg);

Expand Down
110 changes: 83 additions & 27 deletions apache2/msc_multipart.c
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ static int multipart_process_part_header(modsec_rec *msr, char **error_msg) {
if (data == msr->mpd->buf) {
*error_msg = apr_psprintf(msr->mp, "Multipart: Invalid part header (header name missing).");

return -1;
return -1;
}

/* check if multipart header contains any invalid characters */
Expand Down Expand Up @@ -1023,38 +1023,94 @@ int multipart_complete(modsec_rec *msr, char **error_msg) {
* processed yet) in the buffer.
*/
if (msr->mpd->buf_contains_line) {
if ( ((unsigned int)(MULTIPART_BUF_SIZE - msr->mpd->bufleft) == (4 + strlen(msr->mpd->boundary)))
/*
* Note that the buffer may end with the final boundary followed by only CR,
* coming from the [CRLF epilogue], when allow_process_partial == 1 (which is
* set when SecRequestBodyLimitAction is ProcessPartial and the request body
* length exceeds SecRequestBodyLimit).
*
* The following definitions are copied from RFC 2046:
*
* dash-boundary := "--" boundary
*
* delimiter := CRLF dash-boundary
*
* close-delimiter := delimiter "--"
*
* multipart-body := [preamble CRLF]
* dash-boundary transport-padding CRLF
* body-part *encapsulation
* close-delimiter transport-padding
* [CRLF epilogue]
*/
unsigned int buf_data_len = (unsigned int)(MULTIPART_BUF_SIZE - msr->mpd->bufleft);
size_t boundary_len = strlen(msr->mpd->boundary);
if ( (buf_data_len >= 2 + boundary_len)
&& (*(msr->mpd->buf) == '-')
&& (*(msr->mpd->buf + 1) == '-')
&& (strncmp(msr->mpd->buf + 2, msr->mpd->boundary, strlen(msr->mpd->boundary)) == 0)
&& (*(msr->mpd->buf + 2 + strlen(msr->mpd->boundary)) == '-')
&& (*(msr->mpd->buf + 2 + strlen(msr->mpd->boundary) + 1) == '-') )
&& (strncmp(msr->mpd->buf + 2, msr->mpd->boundary, boundary_len) == 0) )
{
if ((msr->mpd->crlf_state_buf_end == 2) && (msr->mpd->flag_lf_line != 1)) {
msr->mpd->flag_lf_line = 1;
if (msr->mpd->flag_crlf_line) {
msr_log(msr, 4, "Multipart: Warning: mixed line endings used (CRLF/LF).");
} else {
msr_log(msr, 4, "Multipart: Warning: incorrect line endings used (LF).");
if ( (buf_data_len >= 2 + boundary_len + 2)
&& (*(msr->mpd->buf + 2 + boundary_len) == '-')
&& (*(msr->mpd->buf + 2 + boundary_len + 1) == '-') )
{
/* If body fits in limit and ends with final boundary plus just CR, reject it. */
if ( (msr->mpd->allow_process_partial == 0)
&& (buf_data_len == 2 + boundary_len + 2 + 1)
&& (*(msr->mpd->buf + 2 + boundary_len + 2) == '\r') )
{
*error_msg = apr_psprintf(msr->mp, "Multipart: Invalid epilogue after final boundary.");
return -1;
}

if ((msr->mpd->crlf_state_buf_end == 2) && (msr->mpd->flag_lf_line != 1)) {
msr->mpd->flag_lf_line = 1;
if (msr->mpd->flag_crlf_line) {
msr_log(msr, 4, "Multipart: Warning: mixed line endings used (CRLF/LF).");
} else {
msr_log(msr, 4, "Multipart: Warning: incorrect line endings used (LF).");
}
}
if (msr->mpd->mpp_substate_part_data_read == 0) {
/* it looks like the final boundary, but it's where part data should begin */
msr->mpd->flag_invalid_part = 1;
msr_log(msr, 4, "Multipart: Warning: Invalid part (data contains final boundary)");
}
/* Looks like the final boundary - process it. */
if (multipart_process_boundary(msr, 1 /* final */, error_msg) < 0) {
msr->mpd->flag_error = 1;
return -1;
}

/* The payload is complete after all. */
msr->mpd->is_complete = 1;
}
if (msr->mpd->mpp_substate_part_data_read == 0) {
/* it looks like the final boundary, but it's where part data should begin */
msr->mpd->flag_invalid_part = 1;
msr_log(msr, 4, "Multipart: Warning: Invalid part (data contains final boundary)");
}
/* Looks like the final boundary - process it. */
if (multipart_process_boundary(msr, 1 /* final */, error_msg) < 0) {
msr->mpd->flag_error = 1;
return -1;
else if (msr->mpd->allow_process_partial == 1) {
if (buf_data_len >= 2 + boundary_len + 1) {
if (*(msr->mpd->buf + 2 + boundary_len) == '-') {
if ( (buf_data_len >= 2 + boundary_len + 2)
&& (*(msr->mpd->buf + 2 + boundary_len + 1) != '-') ) {
*error_msg = apr_psprintf(msr->mp, "Multipart: Invalid final boundary.");
return -1;
}
}
else if ( (*(msr->mpd->buf + 2 + boundary_len) != '\r')
|| ((buf_data_len >= 2 + boundary_len + 2)
&& (*(msr->mpd->buf + 2 + boundary_len + 1) != '\n')) ) {
*error_msg = apr_psprintf(msr->mp, "Multipart: Invalid boundary.");
return -1;
}
}
/* process it as a non-final boundary to avoid building a new part. */
if (multipart_process_boundary(msr, 0, error_msg) < 0) {
msr->mpd->flag_error = 1;
return -1;
}
}

/* The payload is complete after all. */
msr->mpd->is_complete = 1;
}
}

if (msr->mpd->is_complete == 0) {
if (msr->mpd->is_complete == 0 && msr->mpd->allow_process_partial == 0) {
*error_msg = apr_psprintf(msr->mp, "Multipart: Final boundary missing.");
return -1;
}
Expand Down Expand Up @@ -1296,10 +1352,10 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf,
if (c == 0x0a) {
if (msr->mpd->crlf_state == 1) {
msr->mpd->crlf_state = 3;
} else {
} else {
msr->mpd->crlf_state = 2;
}
}
}
}
msr->mpd->crlf_state_buf_end = msr->mpd->crlf_state;
}

Expand Down
1 change: 1 addition & 0 deletions apache2/msc_multipart.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ struct multipart_data {

int seen_data;
int is_complete;
int allow_process_partial;

int flag_error;
int flag_data_before;
Expand Down
4 changes: 2 additions & 2 deletions apache2/msc_parsers.c
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ int parse_arguments(modsec_rec *msr, const char *s, apr_size_t inputlength,
value = &buf[j];
}
}
else {
else if (i < inputlength || msr->reqbody_partial_processing_enabled == 0) {
arg->value_len = urldecode_nonstrict_inplace_ex((unsigned char *)value, arg->value_origin_len, invalid_count, &changed);
arg->value = apr_pstrmemdup(msr->mp, value, arg->value_len);

Expand All @@ -330,7 +330,7 @@ int parse_arguments(modsec_rec *msr, const char *s, apr_size_t inputlength,
}

/* the last parameter was empty */
if (status == 1) {
if (status == 1 && msr->reqbody_partial_processing_enabled == 0) {
arg->value_len = 0;
arg->value = "";

Expand Down
31 changes: 27 additions & 4 deletions apache2/msc_reqbody.c
Original file line number Diff line number Diff line change
Expand Up @@ -413,12 +413,13 @@ apr_status_t modsecurity_request_body_store(modsec_rec *msr,
msr_log(msr, 1, "%s", *error_msg);
}

msr->msc_reqbody_error = 1;
if (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT)
msr->msc_reqbody_error = 1;

if ((msr->txcfg->is_enabled == MODSEC_ENABLED) && (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT)) {
if ((msr->txcfg->is_enabled == MODSEC_ENABLED) && (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_REJECT)) {
return -5;
} else if (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_PARTIAL) {
if(msr->txcfg->is_enabled == MODSEC_ENABLED)
} else if (msr->txcfg->if_limit_action == REQUEST_BODY_LIMIT_ACTION_PARTIAL) {
if (msr->txcfg->is_enabled == MODSEC_ENABLED)
return -5;
}
}
Expand All @@ -438,6 +439,28 @@ apr_status_t modsecurity_request_body_store(modsec_rec *msr,
return -1;
}

/**
* Enable partial processing of request body data.
*/
void modsecurity_request_body_enable_partial_processing(modsec_rec *msr) {
msr->reqbody_partial_processing_enabled = 1;
if (strcmp(msr->msc_reqbody_processor, "MULTIPART") == 0) {
msr->mpd->allow_process_partial = 1;
msr_log(msr, 4, "Multipart: Allow partial processing of request body");
}
else if (strcmp(msr->msc_reqbody_processor, "XML") == 0) {
msr->xml->allow_ill_formed = 1;
msr_log(msr, 4, "XML: Allow partial processing of request body");
}
else if (strcmp(msr->msc_reqbody_processor, "JSON") == 0) {
json_allow_partial_values(msr);
msr_log(msr, 4, "JSON: Allow partial processing of request body");
}
else if (strcmp(msr->msc_reqbody_processor, "URLENCODED") == 0) {
msr_log(msr, 4, "URLENCODED: Allow partial processing of request body");
}
}

apr_status_t modsecurity_request_body_to_stream(modsec_rec *msr, const char *buffer, int buflen, char **error_msg) {
assert(msr != NULL);
assert(error_msg != NULL);
Expand Down
4 changes: 2 additions & 2 deletions apache2/msc_xml.c
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ int xml_process_chunk(modsec_rec *msr, const char *buf, unsigned int size, char
if (msr->xml->parsing_ctx != NULL &&
msr->txcfg->parse_xml_into_args != MSC_XML_ARGS_ONLYARGS) {
xmlParseChunk(msr->xml->parsing_ctx, buf, size, 0);
if (msr->xml->parsing_ctx->wellFormed != 1) {
if (!msr->xml->allow_ill_formed && msr->xml->parsing_ctx->wellFormed != 1) {
*error_msg = apr_psprintf(msr->mp, "XML: Failed to parse document.");
return -1;
}
Expand Down Expand Up @@ -318,7 +318,7 @@ int xml_complete(modsec_rec *msr, char **error_msg) {
msr->xml->parsing_ctx = NULL;
msr_log(msr, 4, "XML: Parsing complete (well_formed %u).", msr->xml->well_formed);

if (msr->xml->well_formed != 1) {
if (!msr->xml->allow_ill_formed && msr->xml->well_formed != 1) {
*error_msg = apr_psprintf(msr->mp, "XML: Failed to parse document.");
return -1;
}
Expand Down
1 change: 1 addition & 0 deletions apache2/msc_xml.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ struct xml_data {
xmlDocPtr doc;

unsigned int well_formed;
unsigned int allow_ill_formed;

/* error reporting and XML array flag */
char *xml_error;
Expand Down
Loading
Loading