From f3a8854fe9754027386e6f84457c818358cd3807 Mon Sep 17 00:00:00 2001 From: brectanus Date: Thu, 27 Sep 2007 21:18:23 +0000 Subject: [PATCH] Mark any error conditions/alerts as 'relevant'. Clean up/add error messages where this can happen. --- CHANGES | 3 + apache2/apache2.h | 2 + apache2/apache2_io.c | 99 +++++++++------------------- apache2/apache2_util.c | 43 ++++++++++++ apache2/mod_security2.c | 10 ++- apache2/modsecurity.c | 9 ++- apache2/modsecurity.h | 12 ++-- apache2/msc_logging.c | 12 ++-- apache2/msc_reqbody.c | 141 +++++++++++++++++++++++++--------------- apache2/pdf_protect.c | 22 +------ 10 files changed, 200 insertions(+), 153 deletions(-) diff --git a/CHANGES b/CHANGES index 8d71b81c..9c9b0d2f 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,9 @@ ?? ??? 2007 - 2.5.0-dev3 ------------------------ + * Situations where ModSecurity will intercept, generate an error or log + an alert to the debug log are now marked as 'relevant'. + * Do not process subrequests in phase 2-4. * Fixed deprecatevar:var=N/S action so that it decrements N every S seconds diff --git a/apache2/apache2.h b/apache2/apache2.h index 4cd86c4c..e9157a81 100644 --- a/apache2/apache2.h +++ b/apache2/apache2.h @@ -69,6 +69,8 @@ apr_status_t DSOLOCAL read_request_body(modsec_rec *msr, char **error_msg); int DSOLOCAL perform_interception(modsec_rec *msr); +apr_status_t DSOLOCAL send_error_bucket(modsec_rec *msr, ap_filter_t *f, int status); + int DSOLOCAL apache2_exec(modsec_rec *msr, const char *command, const char **argv, char **output); void DSOLOCAL record_time_checkpoint(modsec_rec *msr, int checkpoint_no); diff --git a/apache2/apache2_io.c b/apache2/apache2_io.c index 882ea875..4d098277 100644 --- a/apache2/apache2_io.c +++ b/apache2/apache2_io.c @@ -30,6 +30,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; + char *my_error_msg = NULL; if (msr == NULL) { ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, f->r->server, @@ -55,16 +56,20 @@ apr_status_t input_filter(ap_filter_t *f, apr_bucket_brigade *bb_out, if (msr->if_started_forwarding == 0) { msr->if_started_forwarding = 1; - rc = modsecurity_request_body_retrieve_start(msr); + rc = modsecurity_request_body_retrieve_start(msr, &my_error_msg); if (rc == -1) { - // TODO err + if (my_error_msg != NULL) { + msr_log(msr, 1, "%s", my_error_msg); + } return APR_EGENERAL; } } - rc = modsecurity_request_body_retrieve(msr, &chunk, (unsigned int)nbytes); + rc = modsecurity_request_body_retrieve(msr, &chunk, (unsigned int)nbytes, &my_error_msg); if (rc == -1) { - // TODO err + if (my_error_msg != NULL) { + msr_log(msr, 1, "%s", my_error_msg); + } return APR_EGENERAL; } @@ -151,8 +156,7 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) { msr_log(msr, 4, "Input filter: Reading request body."); } - if (modsecurity_request_body_start(msr) < 0) { - // TODO err + if (modsecurity_request_body_start(msr, error_msg) < 0) { return -1; } @@ -169,25 +173,18 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) { */ switch(rc) { case APR_TIMEUP : + *error_msg = apr_psprintf(msr->mp, "Error reading request body: %s", get_apr_error(msr->mp, rc)); return -4; - break; case -3 : *error_msg = apr_psprintf(msr->mp, "Error reading request body: HTTP Error 413 - Request entity too large. (Most likely.)"); - rc = -3; - break; + return -3; case APR_EGENERAL : *error_msg = apr_psprintf(msr->mp, "Error reading request body: Client went away."); - rc = -2; - break; + return -2; default : *error_msg = apr_psprintf(msr->mp, "Error reading request body: %s", get_apr_error(msr->mp, rc)); - rc = -1; - break; + return -1; } - - if (*error_msg) msr_log(msr, 1, "%s", *error_msg); - - return rc; } /* Loop through the buckets in the brigade in order @@ -202,8 +199,7 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) { rc = apr_bucket_read(bucket, &buf, &buflen, APR_BLOCK_READ); if (rc != APR_SUCCESS) { - msr_log(msr, 1, "Input filter: Failed reading input / bucket (%i): %s", - rc, get_apr_error(msr->mp, rc)); + *error_msg = apr_psprintf(msr->mp, "Failed reading input / bucket (%i): %s", rc, get_apr_error(msr->mp, rc)); return -1; } @@ -220,8 +216,7 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) { } if (buflen != 0) { - if (modsecurity_request_body_store(msr, buf, buflen) < 0) { - // TODO err + if (modsecurity_request_body_store(msr, buf, buflen, error_msg) < 0) { return -1; } @@ -236,7 +231,8 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) { apr_brigade_cleanup(bb_in); } while(!seen_eos); - modsecurity_request_body_end(msr); + // TODO: Why ignore the return code here? + modsecurity_request_body_end(msr, error_msg); if (msr->txcfg->debuglog_level >= 4) { msr_log(msr, 4, "Input filter: Completed receiving request body (length %lu).", @@ -251,41 +247,6 @@ apr_status_t read_request_body(modsec_rec *msr, char **error_msg) { /* -- Output filter -- */ -/** - * Sends a brigade with an error bucket down the filter chain. - */ -static apr_status_t send_error_bucket(ap_filter_t *f, int status) { - apr_bucket_brigade *brigade = NULL; - apr_bucket *bucket = NULL; - - /* Set the status line explicitly for the error document */ - f->r->status_line = ap_get_status_line(status); - - brigade = apr_brigade_create(f->r->pool, f->r->connection->bucket_alloc); - if (brigade == NULL) return APR_EGENERAL; - - bucket = ap_bucket_error_create(status, NULL, f->r->pool, f->r->connection->bucket_alloc); - if (bucket == NULL) return APR_EGENERAL; - - APR_BRIGADE_INSERT_TAIL(brigade, bucket); - - bucket = apr_bucket_eos_create(f->r->connection->bucket_alloc); - if (bucket == NULL) return APR_EGENERAL; - - APR_BRIGADE_INSERT_TAIL(brigade, bucket); - - ap_pass_brigade(f->next, brigade); - - /* NOTE: - * It may not matter what we do from the filter as it may be too - * late to even generate an error (already sent to client). Nick Kew - * recommends to return APR_EGENERAL in hopes that the handler in control - * will notice and do The Right Thing. So, that is what we do now. - */ - - return APR_EGENERAL; -} - /** * Examines the configuration and the response MIME type * in order to determine whether output buffering should @@ -505,7 +466,7 @@ apr_status_t output_filter(ap_filter_t *f, apr_bucket_brigade *bb_in) { ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, f->r->server, "ModSecurity: Internal Error: msr is null in output filter."); ap_remove_output_filter(f); - return send_error_bucket(f, HTTP_INTERNAL_SERVER_ERROR); + return send_error_bucket(msr, f, HTTP_INTERNAL_SERVER_ERROR); } msr->r = r; @@ -528,13 +489,13 @@ apr_status_t output_filter(ap_filter_t *f, apr_bucket_brigade *bb_in) { rc = modsecurity_process_phase(msr, PHASE_RESPONSE_HEADERS); if (rc < 0) { /* error */ ap_remove_output_filter(f); - return send_error_bucket(f, HTTP_INTERNAL_SERVER_ERROR); + return send_error_bucket(msr, f, HTTP_INTERNAL_SERVER_ERROR); } if (rc > 0) { /* transaction needs to be interrupted */ int status = perform_interception(msr); if (status != DECLINED) { /* DECLINED means we allow-ed the request. */ ap_remove_output_filter(f); - return send_error_bucket(f, status); + return send_error_bucket(msr, f, status); } } @@ -547,7 +508,7 @@ apr_status_t output_filter(ap_filter_t *f, apr_bucket_brigade *bb_in) { ap_remove_output_filter(f); msr->of_status = OF_STATUS_COMPLETE; msr->resbody_status = RESBODY_STATUS_ERROR; - return send_error_bucket(f, HTTP_INTERNAL_SERVER_ERROR); + return send_error_bucket(msr, f, HTTP_INTERNAL_SERVER_ERROR); case 0 : /* We do not want to observe this response body * but we need to remain attached to observe @@ -626,7 +587,7 @@ apr_status_t output_filter(ap_filter_t *f, apr_bucket_brigade *bb_in) { rc, get_apr_error(r->pool, rc)); ap_remove_output_filter(f); - return send_error_bucket(f, HTTP_INTERNAL_SERVER_ERROR); + return send_error_bucket(msr, f, HTTP_INTERNAL_SERVER_ERROR); } if (msr->txcfg->debuglog_level >= 9) { @@ -649,7 +610,7 @@ apr_status_t output_filter(ap_filter_t *f, apr_bucket_brigade *bb_in) { msr->resbody_status = RESBODY_STATUS_PARTIAL; ap_remove_output_filter(f); - return send_error_bucket(f, HTTP_INTERNAL_SERVER_ERROR); + return send_error_bucket(msr, f, HTTP_INTERNAL_SERVER_ERROR); } else { /* Process partial response. */ start_skipping = 1; @@ -699,18 +660,18 @@ apr_status_t output_filter(ap_filter_t *f, apr_bucket_brigade *bb_in) { /* Do we need to process a partial response? */ if (start_skipping) { if (flatten_response_body(msr) < 0) { - return send_error_bucket(f, HTTP_INTERNAL_SERVER_ERROR); + return send_error_bucket(msr, f, HTTP_INTERNAL_SERVER_ERROR); } /* Process phase RESPONSE_BODY */ rc = modsecurity_process_phase(msr, PHASE_RESPONSE_BODY); if (rc < 0) { - return send_error_bucket(f, HTTP_INTERNAL_SERVER_ERROR); + return send_error_bucket(msr, f, HTTP_INTERNAL_SERVER_ERROR); } if (rc > 0) { int status = perform_interception(msr); if (status != DECLINED) { /* DECLINED means we allow-ed the request. */ - return send_error_bucket(f, status); + return send_error_bucket(msr, f, status); } } @@ -756,17 +717,17 @@ apr_status_t output_filter(ap_filter_t *f, apr_bucket_brigade *bb_in) { */ if (msr->phase < PHASE_RESPONSE_BODY) { if (flatten_response_body(msr) < 0) { - return send_error_bucket(f, HTTP_INTERNAL_SERVER_ERROR); + return send_error_bucket(msr, f, HTTP_INTERNAL_SERVER_ERROR); } rc = modsecurity_process_phase(msr, PHASE_RESPONSE_BODY); if (rc < 0) { - return send_error_bucket(f, HTTP_INTERNAL_SERVER_ERROR); + return send_error_bucket(msr, f, HTTP_INTERNAL_SERVER_ERROR); } if (rc > 0) { int status = perform_interception(msr); if (status != DECLINED) { /* DECLINED means we allow-ed the request. */ - return send_error_bucket(f, status); + return send_error_bucket(msr, f, status); } } } diff --git a/apache2/apache2_util.c b/apache2/apache2_util.c index f60d1ce8..0b4e9aca 100644 --- a/apache2/apache2_util.c +++ b/apache2/apache2_util.c @@ -13,6 +13,46 @@ #include "http_core.h" #include "util_script.h" +/** + * Sends a brigade with an error bucket down the filter chain. + */ +apr_status_t send_error_bucket(modsec_rec *msr, ap_filter_t *f, int status) { + apr_bucket_brigade *brigade = NULL; + apr_bucket *bucket = NULL; + + /* Force relevancy for all errors */ + if (msr != NULL) { + msr->is_relevant++; + } + + /* Set the status line explicitly for the error document */ + f->r->status_line = ap_get_status_line(status); + + brigade = apr_brigade_create(f->r->pool, f->r->connection->bucket_alloc); + if (brigade == NULL) return APR_EGENERAL; + + bucket = ap_bucket_error_create(status, NULL, f->r->pool, f->r->connection->bucket_alloc); + if (bucket == NULL) return APR_EGENERAL; + + APR_BRIGADE_INSERT_TAIL(brigade, bucket); + + bucket = apr_bucket_eos_create(f->r->connection->bucket_alloc); + if (bucket == NULL) return APR_EGENERAL; + + APR_BRIGADE_INSERT_TAIL(brigade, bucket); + + ap_pass_brigade(f->next, brigade); + + /* NOTE: + * It may not matter what we do from the filter as it may be too + * late to even generate an error (already sent to client). Nick Kew + * recommends to return APR_EGENERAL in hopes that the handler in control + * will notice and do The Right Thing. So, that is what we do now. + */ + + return APR_EGENERAL; +} + /** * Execute system command. First line of the output will be returned in * the "output" parameter. @@ -234,6 +274,9 @@ void internal_log(request_rec *r, directory_config *dcfg, modsec_rec *msr, /* Add this message to the list. */ if (msr != NULL) { + /* Force relevency if this is an alert */ + msr->is_relevant++; + *(const char **)apr_array_push(msr->alerts) = apr_pstrdup(msr->mp, str1); } } diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c index 61710c04..d1d69284 100644 --- a/apache2/mod_security2.c +++ b/apache2/mod_security2.c @@ -639,14 +639,22 @@ static int hook_request_late(request_rec *r) { if (rc < 0) { switch(rc) { case -1 : - msr_log(msr, 1, "%s", my_error_msg); + if ((my_error_msg != NULL)||(msr->is_relevant == 0)) { + msr_log(msr, 1, "%s", my_error_msg); + } return HTTP_INTERNAL_SERVER_ERROR; break; case -4 : /* Timeout. */ + if ((my_error_msg != NULL)||(msr->is_relevant == 0)) { + msr_log(msr, 4, "%s", my_error_msg); + } r->connection->keepalive = AP_CONN_CLOSE; return HTTP_REQUEST_TIME_OUT; break; case -5 : /* Request body limit reached. */ + if ((my_error_msg != NULL)||(msr->is_relevant == 0)) { + msr_log(msr, 1, "%s", my_error_msg); + } r->connection->keepalive = AP_CONN_CLOSE; return HTTP_REQUEST_ENTITY_TOO_LARGE; break; diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c index 9ec5685d..c6722398 100644 --- a/apache2/modsecurity.c +++ b/apache2/modsecurity.c @@ -136,6 +136,7 @@ static apr_status_t modsecurity_tx_cleanup(void *data) { apr_table_entry_t *te; int collect_garbage = 0; int i; + char *my_error_msg = NULL; if (msr == NULL) return APR_SUCCESS; @@ -167,7 +168,11 @@ static apr_status_t modsecurity_tx_cleanup(void *data) { if (msr->xml != NULL) xml_cleanup(msr); #endif - modsecurity_request_body_clear(msr); + // TODO: Why do we ignore return code here? + modsecurity_request_body_clear(msr, &my_error_msg); + if (my_error_msg != NULL) { + msr_log(msr, 1, "%s", my_error_msg); + } return APR_SUCCESS; } @@ -431,7 +436,7 @@ static apr_status_t modsecurity_process_phase_logging(modsec_rec *msr) { break; default : - return HTTP_INTERNAL_SERVER_ERROR; + msr_log(msr, 1, "Internal error: Could not determine if auditing is needed, so forcing auditing."); break; } diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h index b4905541..a7470676 100644 --- a/apache2/modsecurity.h +++ b/apache2/modsecurity.h @@ -503,14 +503,14 @@ apr_status_t DSOLOCAL modsecurity_process_phase(modsec_rec *msr, int phase); /* Request body functions */ -apr_status_t DSOLOCAL modsecurity_request_body_start(modsec_rec *msr); +apr_status_t DSOLOCAL modsecurity_request_body_start(modsec_rec *msr, char **error_msg); apr_status_t DSOLOCAL modsecurity_request_body_store(modsec_rec *msr, - const char *data, apr_size_t length); + const char *data, apr_size_t length, char **error_msg); -apr_status_t DSOLOCAL modsecurity_request_body_end(modsec_rec *msr); +apr_status_t DSOLOCAL modsecurity_request_body_end(modsec_rec *msr, char **error_msg); -apr_status_t DSOLOCAL modsecurity_request_body_retrieve_start(modsec_rec *msr); +apr_status_t DSOLOCAL modsecurity_request_body_retrieve_start(modsec_rec *msr, char **error_msg); apr_status_t DSOLOCAL modsecurity_request_body_retrieve_end(modsec_rec *msr); @@ -519,7 +519,7 @@ apr_status_t DSOLOCAL modsecurity_request_body_retrieve_end(modsec_rec *msr); * nbytes will contain the number of bytes stored in the buffer. */ apr_status_t DSOLOCAL modsecurity_request_body_retrieve(modsec_rec *msr, msc_data_chunk **chunk, - long int nbytes); + long int nbytes, char **error_msg); void DSOLOCAL msc_add(modsec_rec *msr, int level, msre_actionset *actionset, const char *action_message, const char *rule_message); @@ -527,6 +527,6 @@ void DSOLOCAL msc_add(modsec_rec *msr, int level, msre_actionset *actionset, void DSOLOCAL msc_alert(modsec_rec *msr, int level, msre_actionset *actionset, const char *action_message, const char *rule_message); -apr_status_t DSOLOCAL modsecurity_request_body_clear(modsec_rec *msr); +apr_status_t DSOLOCAL modsecurity_request_body_clear(modsec_rec *msr, char **error_msg); #endif diff --git a/apache2/msc_logging.c b/apache2/msc_logging.c index 9935ef57..bcf1f1de 100644 --- a/apache2/msc_logging.c +++ b/apache2/msc_logging.c @@ -465,6 +465,7 @@ void sec_audit_logger(modsec_rec *msr) { unsigned int offset = 0, last_offset = 0; msc_arg *nextarg = NULL; int sanitise = 0; /* IMP1 Use constants for "sanitise" values. */ + char *my_error_msg = NULL; sorted_args = apr_array_make(msr->mp, 25, sizeof(const msc_arg *)); @@ -521,9 +522,9 @@ void sec_audit_logger(modsec_rec *msr) { * sanitise data in pieces. */ - rc = modsecurity_request_body_retrieve_start(msr); + rc = modsecurity_request_body_retrieve_start(msr, &my_error_msg); if (rc < 0) { - msr_log(msr, 1, "Audit log: Failed retrieving request body."); + msr_log(msr, 1, "Audit log: %s", my_error_msg); } else { msc_data_chunk *chunk = NULL; unsigned int chunk_offset = 0; @@ -534,7 +535,7 @@ void sec_audit_logger(modsec_rec *msr) { sec_auditlog_write(msr, text, strlen(text)); for(;;) { - rc = modsecurity_request_body_retrieve(msr, &chunk, -1); + rc = modsecurity_request_body_retrieve(msr, &chunk, -1, &my_error_msg); if (chunk != NULL) { /* Anything greater than 1 means we have more data to sanitise. */ while (sanitise > 1) { @@ -594,7 +595,10 @@ void sec_audit_logger(modsec_rec *msr) { chunk_offset += chunk->length; } - if (rc <= 0) break; + if (rc <= 0) { + msr_log(msr, 1, "Audit log: %s", my_error_msg); + break; + } } modsecurity_request_body_retrieve_end(msr); diff --git a/apache2/msc_reqbody.c b/apache2/msc_reqbody.c index 111aabdb..76603764 100644 --- a/apache2/msc_reqbody.c +++ b/apache2/msc_reqbody.c @@ -16,23 +16,30 @@ /** * Prepare to accept the request body (part 2). */ -static apr_status_t modsecurity_request_body_start_init(modsec_rec *msr) { +static apr_status_t modsecurity_request_body_start_init(modsec_rec *msr, char **error_msg) { if(msr->msc_reqbody_storage == MSC_REQBODY_MEMORY) { /* Prepare to store request body in memory. */ msr->msc_reqbody_chunks = apr_array_make(msr->msc_reqbody_mp, 32, sizeof(msc_data_chunk *)); - if (msr->msc_reqbody_chunks == NULL) return -1; + if (msr->msc_reqbody_chunks == NULL) { + + *error_msg = apr_pstrdup(msr->mp, "Input filter: Failed to prepare in-memory storage."); + return -1; + } } else { /* Prepare to store request body on disk. */ msr->msc_reqbody_filename = apr_psprintf(msr->mp, "%s/%s-%s-request_body-XXXXXX", msr->txcfg->tmp_dir, current_filetime(msr->mp), msr->txid); - if (msr->msc_reqbody_filename == NULL) return -1; + if (msr->msc_reqbody_filename == NULL) { + *error_msg = apr_pstrdup(msr->mp, "Input filter: Failed to generate an on-disk filename."); + return -1; + } msr->msc_reqbody_fd = msc_mkstemp((char *)msr->msc_reqbody_filename); if (msr->msc_reqbody_fd < 0) { - msr_log(msr, 1, "Input filter: Failed to create temporary file: %s", + *error_msg = apr_psprintf(msr->mp, "Input filter: Failed to create temporary file: %s", msr->msc_reqbody_filename); return -1; } @@ -47,7 +54,7 @@ static apr_status_t modsecurity_request_body_start_init(modsec_rec *msr) { /** * Prepare to accept the request body (part 1). */ -apr_status_t modsecurity_request_body_start(modsec_rec *msr) { +apr_status_t modsecurity_request_body_start(modsec_rec *msr, char **error_msg) { msr->msc_reqbody_length = 0; /* Create a separate memory pool that will be used @@ -63,7 +70,7 @@ apr_status_t modsecurity_request_body_start(modsec_rec *msr) { if (strcmp(msr->msc_reqbody_processor, "MULTIPART") == 0) { if (multipart_init(msr, &my_error_msg) < 0) { - msr_log(msr, 1, "Multipart parser init failed: %s", my_error_msg); + *error_msg = apr_psprintf(msr->mp, "Multipart parser init failed: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = my_error_msg; } @@ -72,7 +79,7 @@ apr_status_t modsecurity_request_body_start(modsec_rec *msr) { else if (strcmp(msr->msc_reqbody_processor, "XML") == 0) { if (xml_init(msr, &my_error_msg) < 0) { - msr_log(msr, 1, "XML parser init failed: %s", my_error_msg); + *error_msg = apr_psprintf(msr->mp, "XML parser init failed: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = my_error_msg; } @@ -83,23 +90,23 @@ apr_status_t modsecurity_request_body_start(modsec_rec *msr) { /* Do nothing, URLENCODED processor does not support streaming yet. */ } else { - msr_log(msr, 1, "Unknown request body processor: %s", msr->msc_reqbody_processor); + *error_msg = apr_psprintf(msr->mp, "Unknown request body processor: %s", msr->msc_reqbody_processor); return -1; } } - return modsecurity_request_body_start_init(msr); + return modsecurity_request_body_start_init(msr, error_msg); } /** * Stores a chunk of request body data to disk. */ static apr_status_t modsecurity_request_body_store_disk(modsec_rec *msr, - const char *data, apr_size_t length) + const char *data, apr_size_t length, char **error_msg) { apr_size_t i = write(msr->msc_reqbody_fd, data, length); if (i != length) { - msr_log(msr, 1, "Input filter: Failed writing %lu bytes to temporary file (rc %lu).", i); + *error_msg = apr_psprintf(msr->mp, "Input filter: Failed writing %" APR_SIZE_T_FMT " bytes to temporary file (rc %" APR_SIZE_T_FMT ").", length, i); return -1; } @@ -110,7 +117,7 @@ static apr_status_t modsecurity_request_body_store_disk(modsec_rec *msr, * Stores one chunk of request body data in memory. */ static apr_status_t modsecurity_request_body_store_memory(modsec_rec *msr, - const char *data, apr_size_t length) + const char *data, apr_size_t length, char **error_msg) { /* Would storing this chunk mean going over the limit? */ if ((msr->msc_reqbody_spilltodisk) @@ -129,14 +136,14 @@ static apr_status_t modsecurity_request_body_store_memory(modsec_rec *msr, /* Initialise disk storage */ msr->msc_reqbody_storage = MSC_REQBODY_DISK; - if (modsecurity_request_body_start_init(msr) < 0) return -1; + if (modsecurity_request_body_start_init(msr, error_msg) < 0) return -1; /* Write the data we keep in memory */ chunks = (msc_data_chunk **)msr->msc_reqbody_chunks->elts; for(i = 0; i < msr->msc_reqbody_chunks->nelts; i++) { disklen += chunks[i]->length; - if (modsecurity_request_body_store_disk(msr, chunks[i]->data, chunks[i]->length) < 0) { + if (modsecurity_request_body_store_disk(msr, chunks[i]->data, chunks[i]->length, error_msg) < 0) { return -1; } @@ -152,10 +159,10 @@ static apr_status_t modsecurity_request_body_store_memory(modsec_rec *msr, msr->msc_reqbody_chunks = NULL; apr_pool_clear(msr->msc_reqbody_mp); - msr_log(msr, 4, "Input filter: Wrote %lu bytes from memory to disk.", disklen); + msr_log(msr, 4, "Input filter: Wrote %" APR_SIZE_T_FMT " bytes from memory to disk.", disklen); /* Continue with disk storage from now on */ - return modsecurity_request_body_store_disk(msr, data, length); + return modsecurity_request_body_store_disk(msr, data, length, error_msg); } /* If we're here that means we are not over the @@ -180,10 +187,16 @@ static apr_status_t modsecurity_request_body_store_memory(modsec_rec *msr, if (msr->msc_reqbody_chunk_current == NULL) { msr->msc_reqbody_chunk_current = (msc_data_chunk *) apr_pcalloc(msr->msc_reqbody_mp, sizeof(msc_data_chunk)); - if (msr->msc_reqbody_chunk_current == NULL) return -1; + if (msr->msc_reqbody_chunk_current == NULL) { + *error_msg = apr_psprintf(msr->mp, "Input filter: Failed to allocate %d bytes for request body chunk.", sizeof(msc_data_chunk)); + return -1; + } msr->msc_reqbody_chunk_current->data = malloc(CHUNK_CAPACITY); - if (msr->msc_reqbody_chunk_current->data == NULL) return -1; + if (msr->msc_reqbody_chunk_current->data == NULL) { + *error_msg = apr_psprintf(msr->mp, "Input filter: Failed to allocate %d bytes for request body chunk data.", CHUNK_CAPACITY); + return -1; + } msr->msc_reqbody_chunk_current->length = 0; msr->msc_reqbody_chunk_current->is_permanent = 1; @@ -227,7 +240,7 @@ static apr_status_t modsecurity_request_body_store_memory(modsec_rec *msr, * Stores one chunk of request body data. Returns -1 on error. */ apr_status_t modsecurity_request_body_store(modsec_rec *msr, - const char *data, apr_size_t length) + const char *data, apr_size_t length, char **error_msg) { /* If we have a processor for this request body send * data to it first (but only if it did not report an @@ -238,18 +251,18 @@ apr_status_t modsecurity_request_body_store(modsec_rec *msr, if (strcmp(msr->msc_reqbody_processor, "MULTIPART") == 0) { if (multipart_process_chunk(msr, data, length, &my_error_msg) < 0) { + *error_msg = apr_psprintf(msr->mp, "Request body processor error: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = my_error_msg; - msr_log(msr, 1, "Request body processor error: %s", my_error_msg); } } #ifdef WITH_LIBXML2 else if (strcmp(msr->msc_reqbody_processor, "XML") == 0) { if (xml_process_chunk(msr, data, length, &my_error_msg) < 0) { + *error_msg = apr_psprintf(msr->mp, "Request body processor error: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = my_error_msg; - msr_log(msr, 1, "Request body processor error: %s", my_error_msg); } } #endif @@ -258,22 +271,22 @@ apr_status_t modsecurity_request_body_store(modsec_rec *msr, /* Do nothing, URLENCODED processor does not support streaming. */ } else { - msr_log(msr, 1, "Unknown request body processor: %s", msr->msc_reqbody_processor); + *error_msg = apr_psprintf(msr->mp, "Unknown request body processor: %s", msr->msc_reqbody_processor); return -1; } } /* Store data. */ if (msr->msc_reqbody_storage == MSC_REQBODY_MEMORY) { - return modsecurity_request_body_store_memory(msr, data, length); + return modsecurity_request_body_store_memory(msr, data, length, error_msg); } else if (msr->msc_reqbody_storage == MSC_REQBODY_DISK) { - return modsecurity_request_body_store_disk(msr, data, length); + return modsecurity_request_body_store_disk(msr, data, length, error_msg); } /* Should never happen. */ - msr_log(msr, 1, "Internal Error: Unknown value for msc_reqbody_storage: %i", + *error_msg = apr_psprintf(msr->mp, "Internal error, unknown value for msc_reqbody_storage: %d", msr->msc_reqbody_storage); return -1; } @@ -281,7 +294,7 @@ apr_status_t modsecurity_request_body_store(modsec_rec *msr, /** * */ -static apr_status_t modsecurity_request_body_end_urlencoded(modsec_rec *msr) { +static apr_status_t modsecurity_request_body_end_urlencoded(modsec_rec *msr, char **error_msg) { msc_data_chunk **chunks, *one_chunk; char *d; int i, sofar; @@ -289,10 +302,13 @@ static apr_status_t modsecurity_request_body_end_urlencoded(modsec_rec *msr) { /* Allocate a buffer large enough to hold the request body. */ - if (msr->msc_reqbody_length + 1 == 0) return -1; + if (msr->msc_reqbody_length + 1 == 0) { + *error_msg = apr_psprintf(msr->mp, "Internal error, request body length will overflow: %d", msr->msc_reqbody_length); + return -1; + } msr->msc_reqbody_buffer = malloc(msr->msc_reqbody_length + 1); if (msr->msc_reqbody_buffer == NULL) { - msr_log(msr, 1, "Unable to allocate memory to hold request body. Asked for %lu bytes.", + *error_msg = apr_psprintf(msr->mp, "Unable to allocate memory to hold request body. Asked for %" APR_SIZE_T_FMT " bytes.", msr->msc_reqbody_length + 1); return -1; } @@ -309,7 +325,7 @@ static apr_status_t modsecurity_request_body_end_urlencoded(modsec_rec *msr) { d += chunks[i]->length; sofar += chunks[i]->length; } else { - msr_log(msr, 1, "Internal error, request body buffer overflow."); + *error_msg = apr_psprintf(msr->mp, "Internal error, request body buffer overflow."); return -1; } } @@ -325,7 +341,10 @@ static apr_status_t modsecurity_request_body_end_urlencoded(modsec_rec *msr) { /* Create a new array with only one chunk in it. */ msr->msc_reqbody_chunks = apr_array_make(msr->msc_reqbody_mp, 2, sizeof(msc_data_chunk *)); - if (msr->msc_reqbody_chunks == NULL) return -1; + if (msr->msc_reqbody_chunks == NULL) { + *error_msg = apr_pstrdup(msr->mp, "Failed to create structure to hold request body."); + return -1; + } one_chunk = (msc_data_chunk *)apr_pcalloc(msr->msc_reqbody_mp, sizeof(msc_data_chunk)); one_chunk->data = msr->msc_reqbody_buffer; one_chunk->length = msr->msc_reqbody_length; @@ -337,7 +356,7 @@ static apr_status_t modsecurity_request_body_end_urlencoded(modsec_rec *msr) { if (parse_arguments(msr, msr->msc_reqbody_buffer, msr->msc_reqbody_length, msr->txcfg->argument_separator, "BODY", msr->arguments, &invalid_count) < 0) { - msr_log(msr, 1, "Initialisation: Error occurred while parsing BODY arguments."); + *error_msg = apr_pstrdup(msr->mp, "Initialisation: Error occurred while parsing BODY arguments."); return -1; } @@ -347,7 +366,7 @@ static apr_status_t modsecurity_request_body_end_urlencoded(modsec_rec *msr) { /** * Stops receiving the request body. */ -apr_status_t modsecurity_request_body_end(modsec_rec *msr) { +apr_status_t modsecurity_request_body_end(modsec_rec *msr, char **error_msg) { /* Close open file descriptors, if any. */ if (msr->msc_reqbody_storage == MSC_REQBODY_DISK) { @@ -364,22 +383,22 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr) { if (strcmp(msr->msc_reqbody_processor, "MULTIPART") == 0) { if (multipart_complete(msr, &my_error_msg) < 0) { + *error_msg = apr_psprintf(msr->mp, "Multipart error: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = my_error_msg; - msr_log(msr, 1, "Multipart error: %s", my_error_msg); return -1; } if (multipart_get_arguments(msr, "BODY", msr->arguments) < 0) { + *error_msg = apr_psprintf(msr->mp, "Multipart error: %s", my_error_msg); msr->msc_reqbody_error = 1; msr->msc_reqbody_error_msg = "Error retrieving arguments."; - msr_log(msr, 1, "Multipart error: %s", my_error_msg); return -1; } } else if (strcmp(msr->msc_reqbody_processor, "URLENCODED") == 0) { - return modsecurity_request_body_end_urlencoded(msr); + return modsecurity_request_body_end_urlencoded(msr, error_msg); } #ifdef WITH_LIBXML2 else @@ -400,27 +419,36 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr) { /** * Prepares to forward the request body. */ -apr_status_t modsecurity_request_body_retrieve_start(modsec_rec *msr) { +apr_status_t modsecurity_request_body_retrieve_start(modsec_rec *msr, char **error_msg) { if (msr->msc_reqbody_storage == MSC_REQBODY_MEMORY) { msr->msc_reqbody_chunk_position = 0; msr->msc_reqbody_chunk_offset = 0; msr->msc_reqbody_disk_chunk = apr_pcalloc(msr->msc_reqbody_mp, sizeof(msc_data_chunk)); - if (msr->msc_reqbody_disk_chunk == NULL) return -1; + if (msr->msc_reqbody_disk_chunk == NULL) { + *error_msg = apr_psprintf(msr->mp, "Failed to allocate %d bytes for request body disk chunk.", sizeof(msc_data_chunk)); + return -1; + } msr->msc_reqbody_disk_chunk->is_permanent = 1; } else if (msr->msc_reqbody_storage == MSC_REQBODY_DISK) { msr->msc_reqbody_disk_chunk = apr_pcalloc(msr->msc_reqbody_mp, sizeof(msc_data_chunk)); - if (msr->msc_reqbody_disk_chunk == NULL) return -1; + if (msr->msc_reqbody_disk_chunk == NULL) { + *error_msg = apr_psprintf(msr->mp, "Failed to allocate %d bytes for request body disk chunk.", sizeof(msc_data_chunk)); + return -1; + } msr->msc_reqbody_disk_chunk->is_permanent = 0; msr->msc_reqbody_disk_chunk->data = apr_palloc(msr->msc_reqbody_mp, CHUNK_CAPACITY); - if (msr->msc_reqbody_disk_chunk->data == NULL) return -1; + if (msr->msc_reqbody_disk_chunk->data == NULL) { + *error_msg = apr_psprintf(msr->mp, "Failed to allocate %d bytes for request body disk chunk data.", CHUNK_CAPACITY); + return -1; + } msr->msc_reqbody_fd = open(msr->msc_reqbody_filename, O_RDONLY | O_BINARY); if (msr->msc_reqbody_fd < 0) { - msr_log(msr, 1, "Input filter: Failed to open temporary file for reading: %s", + *error_msg = apr_psprintf(msr->mp, "Failed to open temporary file for reading: %s", msr->msc_reqbody_filename); return -1; } @@ -453,11 +481,14 @@ apr_status_t modsecurity_request_body_retrieve_end(modsec_rec *msr) { * a non-negative value in nbytes. */ apr_status_t modsecurity_request_body_retrieve(modsec_rec *msr, - msc_data_chunk **chunk, long int nbytes) + msc_data_chunk **chunk, long int nbytes, char **error_msg) { msc_data_chunk **chunks; - if (chunk == NULL) return -1; + if (chunk == NULL) { + *error_msg = apr_pstrdup(msr->mp, "Internal error, retrieving request body chunk."); + return -1; + } *chunk = NULL; if (msr->msc_reqbody_storage == MSC_REQBODY_MEMORY) { @@ -527,7 +558,7 @@ apr_status_t modsecurity_request_body_retrieve(modsec_rec *msr, i = read(msr->msc_reqbody_fd, msr->msc_reqbody_disk_chunk->data, my_nbytes); if (i < 0) { - msr_log(msr, 1, "Input filter: Error reading from temporary file: %s", + *error_msg = apr_psprintf(msr->mp, "Input filter: Error reading from temporary file: %s", strerror(errno)); return -1; } @@ -540,7 +571,7 @@ apr_status_t modsecurity_request_body_retrieve(modsec_rec *msr, return 1; /* More data available. */ } - msr_log(msr, 1, "Internal error, invalid msc_reqbody_storage value: %i", + *error_msg = apr_psprintf(msr->mp, "Internal error, invalid msc_reqbody_storage value: %d", msr->msc_reqbody_storage); return -1; @@ -549,7 +580,7 @@ apr_status_t modsecurity_request_body_retrieve(modsec_rec *msr, /** * */ -apr_status_t modsecurity_request_body_clear(modsec_rec *msr) { +apr_status_t modsecurity_request_body_clear(modsec_rec *msr, char **error_msg) { /* Release memory we used to store request body data. */ if (msr->msc_reqbody_chunks != NULL) { msc_data_chunk **chunks = (msc_data_chunk **)msr->msc_reqbody_chunks->elts; @@ -574,7 +605,7 @@ apr_status_t modsecurity_request_body_clear(modsec_rec *msr) { if (msr->txcfg->upload_dir != NULL) { keep_body = 1; } else { - msr_log(msr, 1, "Input filter: SecUploadDir is undefined, " + *error_msg = apr_psprintf(msr->mp, "Input filter: SecUploadDir is undefined, " "unable to store PUT file."); } } @@ -589,20 +620,26 @@ apr_status_t modsecurity_request_body_clear(modsec_rec *msr) { /* Construct the new filename. */ put_basename = file_basename(msr->msc_reqbody_mp, msr->msc_reqbody_filename); - if (put_basename == NULL) return -1; + if (put_basename == NULL) { + *error_msg = apr_psprintf(msr->mp, "Input filter: Failed to generate basename to PUT file \"%s\"", log_escape(msr->msc_reqbody_mp, msr->msc_reqbody_filename)); + return -1; + } put_filename = apr_psprintf(msr->msc_reqbody_mp, "%s/%s", msr->txcfg->upload_dir, put_basename); - if (put_filename == NULL) return -1; + if (put_filename == NULL) { + *error_msg = apr_psprintf(msr->mp, "Input filter: Failed to generate filename to PUT file \"%s\"", log_escape(msr->msc_reqbody_mp, msr->msc_reqbody_filename)); + return -1; + } if (apr_file_rename(msr->msc_reqbody_filename, put_filename, msr->msc_reqbody_mp) != APR_SUCCESS) { - msr_log(msr, 1, "Failed to rename file from \"%s\" to \"%s\".", + *error_msg = apr_psprintf(msr->mp, "Input filter: Failed to rename file from \"%s\" to \"%s\".", log_escape(msr->msc_reqbody_mp, msr->msc_reqbody_filename), log_escape(msr->msc_reqbody_mp, put_filename)); return -1; } else { - msr_log(msr, 4, "Moved file from \"%s\" to \"%s\".", + msr_log(msr, 4, "Input filter: Moved file from \"%s\" to \"%s\".", log_escape(msr->msc_reqbody_mp, msr->msc_reqbody_filename), log_escape(msr->msc_reqbody_mp, put_filename)); } @@ -611,8 +648,8 @@ apr_status_t modsecurity_request_body_clear(modsec_rec *msr) { if (apr_file_remove(msr->msc_reqbody_filename, msr->msc_reqbody_mp) != APR_SUCCESS) { - msr_log(msr, 1, "Failed to delete temporary file: %s", - msr->msc_reqbody_filename); + *error_msg = apr_psprintf(msr->mp, "Input filter: Failed to delete temporary file: %s", + log_escape(msr->mp, msr->msc_reqbody_filename)); return -1; } diff --git a/apache2/pdf_protect.c b/apache2/pdf_protect.c index 15c2caaf..cc64f23c 100644 --- a/apache2/pdf_protect.c +++ b/apache2/pdf_protect.c @@ -9,6 +9,7 @@ * */ #include "modsecurity.h" +#include "apache2.h" #include "pdf_protect.h" #include @@ -197,23 +198,6 @@ static int verify_token(modsec_rec *msr, const char *token, char **error_msg) { return 1; } -/** - * - */ -static apr_status_t send_error_bucket(ap_filter_t *f, int status) { - apr_bucket_brigade *brigade = NULL; - apr_bucket *bucket = NULL; - - brigade = apr_brigade_create(f->r->pool, f->r->connection->bucket_alloc); - if (brigade == NULL) return APR_EGENERAL; - bucket = ap_bucket_error_create(status, NULL, f->r->pool, f->r->connection->bucket_alloc); - if (bucket == NULL) return APR_EGENERAL; - APR_BRIGADE_INSERT_TAIL(brigade, bucket); - - f->r->connection->keepalive = AP_CONN_CLOSE; - return ap_pass_brigade(f->next, brigade); -} - /** * */ @@ -226,7 +210,7 @@ apr_status_t pdfp_output_filter(ap_filter_t *f, apr_bucket_brigade *bb_in) { ap_remove_output_filter(f); - return send_error_bucket(f, HTTP_INTERNAL_SERVER_ERROR); + return send_error_bucket(msr, f, HTTP_INTERNAL_SERVER_ERROR); } if (msr->txcfg->pdfp_enabled == 1) { @@ -328,7 +312,7 @@ apr_status_t pdfp_output_filter(ap_filter_t *f, apr_bucket_brigade *bb_in) { apr_table_set(r->headers_out, "Location", new_uri); - return send_error_bucket(f, REDIRECT_STATUS); + return send_error_bucket(msr, f, REDIRECT_STATUS); } } else { /* Token found. */ char *my_error_msg = NULL;