From 8e990900678654f145716046bb2cef8546d20fec Mon Sep 17 00:00:00 2001 From: brectanus Date: Wed, 17 Oct 2007 22:41:37 +0000 Subject: [PATCH] Add the input filter if we have read the body (even if a sub-request). See #335. --- CHANGES | 2 +- apache2/apache2_io.c | 5 +++++ apache2/mod_security2.c | 27 ++++++++++++++------------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/CHANGES b/CHANGES index 93a140f7..8e261a3b 100644 --- a/CHANGES +++ b/CHANGES @@ -21,7 +21,7 @@ a level 1-3 message to the debug log are now marked as 'relevant' and may generate an audit log entry. - * Do not process subrequests in phase 2-4. + * Do not process subrequests in phase 2-4, but do hand off the request data. * Fixed deprecatevar:var=N/S action so that it decrements N every S seconds as documented instead of decrementing by a rate. diff --git a/apache2/apache2_io.c b/apache2/apache2_io.c index ea73f71f..a0c0632c 100644 --- a/apache2/apache2_io.c +++ b/apache2/apache2_io.c @@ -41,6 +41,11 @@ apr_status_t input_filter(ap_filter_t *f, apr_bucket_brigade *bb_out, msr->r = f->r; + if (msr->phase > PHASE_REQUEST_BODY) { + msr_log(msr, 1, "Internal error: Still in input filter in phase %d", msr->phase); + return APR_EGENERAL; + } + if ((msr->if_status == IF_STATUS_COMPLETE)||(msr->if_status == IF_STATUS_NONE)) { if (msr->txcfg->debuglog_level >= 4) { msr_log(msr, 4, "Input filter: Input forwarding already complete, skipping (f %x, r %x).", f, f->r); diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c index d834b438..b95bc1a5 100644 --- a/apache2/mod_security2.c +++ b/apache2/mod_security2.c @@ -888,6 +888,19 @@ static int hook_log_transaction(request_rec *r) { static void hook_insert_filter(request_rec *r) { modsec_rec *msr = NULL; + /* Find the transaction context first. */ + msr = retrieve_tx_context(r); + if (msr == NULL) return; + + /* Add the input filter, but only if we need it to run. */ + if (msr->if_status == IF_STATUS_WANTS_TO_RUN) { + if (msr->txcfg->debuglog_level >= 4) { + msr_log(msr, 4, "Hook insert_filter: Adding input forwarding filter %s(r %x).", (((r->main != NULL)||(r->prev != NULL)) ? "for subrequest " : ""), r); + } + + ap_add_input_filter("MODSECURITY_IN", msr, r, r->connection); + } + /* This function needs to run only once per transaction * (i.e. subrequests and redirects are excluded). */ @@ -895,10 +908,6 @@ static void hook_insert_filter(request_rec *r) { return; } - /* Find the transaction context first. */ - msr = retrieve_tx_context(r); - if (msr == NULL) return; - /* We always add the PDF XSS protection filter. */ if (msr->txcfg->debuglog_level >= 4) { msr_log(msr, 4, "Hook insert_filter: Adding PDF XSS protection output filter (r %x).", r); @@ -907,6 +916,7 @@ static void hook_insert_filter(request_rec *r) { ap_add_output_filter("PDFP_OUT", msr, r, r->connection); /* Only proceed to add the second filter if the engine is enabled. */ + // TODO: Do we need this anymore? if (msr->txcfg->is_enabled == 0) { if (msr->txcfg->debuglog_level >= 4) { msr_log(msr, 4, "Hook insert_filter: Processing disabled, skipping."); @@ -915,15 +925,6 @@ static void hook_insert_filter(request_rec *r) { return; } - /* Add the input filter, but only if we need it to run. */ - if (msr->if_status == IF_STATUS_WANTS_TO_RUN) { - if (msr->txcfg->debuglog_level >= 4) { - msr_log(msr, 4, "Hook insert_filter: Adding input forwarding filter (r %x).", r); - } - - ap_add_input_filter("MODSECURITY_IN", msr, r, r->connection); - } - /* We always add the output filter because that's where we need to * initiate our 3rd and 4th processing phases from. The filter is * smart enough not to buffer the data if it is not supposed to.