From 7ee6b1a0637d649d403e86c277a2187f19fa525a Mon Sep 17 00:00:00 2001 From: Rainer Jung Date: Wed, 15 May 2019 01:31:43 +0200 Subject: [PATCH 1/3] When the input filter finishes, check whether we returned data during the last read and if not, delegate to the remaining filter chain. Without that, ProcessPartial for the request body breaks forwarding of uploaded files using mod_proxy_ajp and mod_wl. See issue #2091. --- apache2/apache2_io.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apache2/apache2_io.c b/apache2/apache2_io.c index 66b865f37e..f6c785e8d2 100644 --- a/apache2/apache2_io.c +++ b/apache2/apache2_io.c @@ -36,6 +36,7 @@ apr_status_t input_filter(ap_filter_t *f, apr_bucket_brigade *bb_out, msc_data_chunk *chunk = NULL; apr_bucket *bucket; apr_status_t rc; + int no_data = 1; char *my_error_msg = NULL; if (msr == NULL) { @@ -110,6 +111,7 @@ apr_status_t input_filter(ap_filter_t *f, apr_bucket_brigade *bb_out, if (bucket == NULL) return APR_EGENERAL; APR_BRIGADE_INSERT_TAIL(bb_out, bucket); + no_data = 0; if (msr->txcfg->debuglog_level >= 4) { msr_log(msr, 4, "Input filter: Forwarded %" APR_SIZE_T_FMT " bytes.", chunk->length); @@ -130,6 +132,7 @@ apr_status_t input_filter(ap_filter_t *f, apr_bucket_brigade *bb_out, if (bucket == NULL) return APR_EGENERAL; APR_BRIGADE_INSERT_TAIL(bb_out, bucket); + no_data = 0; if (msr->txcfg->debuglog_level >= 4) { msr_log(msr, 4, "Input stream filter: Forwarded %" APR_SIZE_T_FMT " bytes.", msr->stream_input_length); @@ -145,6 +148,7 @@ apr_status_t input_filter(ap_filter_t *f, apr_bucket_brigade *bb_out, bucket = apr_bucket_eos_create(f->r->connection->bucket_alloc); if (bucket == NULL) return APR_EGENERAL; APR_BRIGADE_INSERT_TAIL(bb_out, bucket); + no_data = 0; if (msr->txcfg->debuglog_level >= 4) { msr_log(msr, 4, "Input filter: Sent EOS."); @@ -158,6 +162,10 @@ apr_status_t input_filter(ap_filter_t *f, apr_bucket_brigade *bb_out, if (msr->txcfg->debuglog_level >= 4) { msr_log(msr, 4, "Input filter: Input forwarding complete."); } + + if (no_data) { + return ap_get_brigade(f->next, bb_out, mode, block, nbytes); + } } return APR_SUCCESS; From fc00f4828ccb29666608b30eee3fbdfdefaeed77 Mon Sep 17 00:00:00 2001 From: Rainer Jung Date: Wed, 15 May 2019 01:38:43 +0200 Subject: [PATCH 2/3] Only run completion checks for request bodies, if we have actually seen all afo the body. In case ProcessPartial is configured and our input filter only processed part of the request, checking for a complete request body makes no sense. See issue #2093. --- apache2/msc_reqbody.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apache2/msc_reqbody.c b/apache2/msc_reqbody.c index e84c33a391..a5f290c19b 100644 --- a/apache2/msc_reqbody.c +++ b/apache2/msc_reqbody.c @@ -690,7 +690,7 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr, char **error_msg) { } // TODO: All these below need to be registered in the same way as above else if (strcmp(msr->msc_reqbody_processor, "MULTIPART") == 0) { - if (multipart_complete(msr, &my_error_msg) < 0) { + if (multipart_complete(msr, &my_error_msg) < 0 && msr->if_seen_eos) { *error_msg = apr_psprintf(msr->mp, "Multipart parsing error: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = *error_msg; @@ -710,7 +710,7 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr, char **error_msg) { } else if (strcmp(msr->msc_reqbody_processor, "JSON") == 0) { #ifdef WITH_YAJL - if (json_complete(msr, &my_error_msg) < 0 && msr->msc_reqbody_length > 0) { + if (json_complete(msr, &my_error_msg) < 0 && msr->msc_reqbody_length > 0 && msr->if_seen_eos) { *error_msg = apr_psprintf(msr->mp, "JSON parser error: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = *error_msg; @@ -730,7 +730,7 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr, char **error_msg) { return modsecurity_request_body_end_urlencoded(msr, error_msg); } else if (strcmp(msr->msc_reqbody_processor, "XML") == 0) { - if (xml_complete(msr, &my_error_msg) < 0) { + if (xml_complete(msr, &my_error_msg) < 0 && msr->if_seen_eos) { *error_msg = apr_psprintf(msr->mp, "XML parser error: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = *error_msg; From c7e110360713e11c95ad51ff7231a98d411e62a2 Mon Sep 17 00:00:00 2001 From: Rainer Jung Date: Wed, 15 May 2019 01:43:40 +0200 Subject: [PATCH 3/3] Revert commit, not meant for this branch. --- apache2/msc_reqbody.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apache2/msc_reqbody.c b/apache2/msc_reqbody.c index a5f290c19b..e84c33a391 100644 --- a/apache2/msc_reqbody.c +++ b/apache2/msc_reqbody.c @@ -690,7 +690,7 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr, char **error_msg) { } // TODO: All these below need to be registered in the same way as above else if (strcmp(msr->msc_reqbody_processor, "MULTIPART") == 0) { - if (multipart_complete(msr, &my_error_msg) < 0 && msr->if_seen_eos) { + if (multipart_complete(msr, &my_error_msg) < 0) { *error_msg = apr_psprintf(msr->mp, "Multipart parsing error: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = *error_msg; @@ -710,7 +710,7 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr, char **error_msg) { } else if (strcmp(msr->msc_reqbody_processor, "JSON") == 0) { #ifdef WITH_YAJL - if (json_complete(msr, &my_error_msg) < 0 && msr->msc_reqbody_length > 0 && msr->if_seen_eos) { + if (json_complete(msr, &my_error_msg) < 0 && msr->msc_reqbody_length > 0) { *error_msg = apr_psprintf(msr->mp, "JSON parser error: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = *error_msg; @@ -730,7 +730,7 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr, char **error_msg) { return modsecurity_request_body_end_urlencoded(msr, error_msg); } else if (strcmp(msr->msc_reqbody_processor, "XML") == 0) { - if (xml_complete(msr, &my_error_msg) < 0 && msr->if_seen_eos) { + if (xml_complete(msr, &my_error_msg) < 0) { *error_msg = apr_psprintf(msr->mp, "XML parser error: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = *error_msg;