diff --git a/CHANGES b/CHANGES index 9fc4d5f7..1bf685b3 100644 --- a/CHANGES +++ b/CHANGES @@ -1,8 +1,13 @@ 04 Nov 2009 - 2.5.11 -------------------- - * Cleanup persistence database locking code which could deadlock - under high stress. + * Added a new multipart flag, MULTIPART_INVALID_QUOTING, which will be + set true if any invalid quoting is found during multipart parsing. + + * Fixed parsing quoted strings in multipart Content-Disposition headers. + Discovered by Stefan Esser. + + * Cleanup persistence database locking code. * Added warning during configure if libcurl is found linked against gnutls for SSL. The openssl lib is recommended as gnutls has diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index fb9c6c7d..545e169d 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -490,7 +490,7 @@ void init_directory_config(directory_config *dcfg) { if (dcfg->is_enabled == NOT_SET) dcfg->is_enabled = 0; if (dcfg->reqbody_access == NOT_SET) dcfg->reqbody_access = 0; - if (dcfg->reqbody_buffering == NOT_SET) dcfg->reqbody_buffering = 0; + if (dcfg->reqbody_buffering == NOT_SET) dcfg->reqbody_buffering = REQUEST_BODY_FORCEBUF_OFF; if (dcfg->reqbody_inmemory_limit == NOT_SET) dcfg->reqbody_inmemory_limit = REQUEST_BODY_DEFAULT_INMEMORY_LIMIT; if (dcfg->reqbody_limit == NOT_SET) dcfg->reqbody_limit = REQUEST_BODY_DEFAULT_LIMIT; diff --git a/apache2/autogen.sh b/apache2/autogen.sh index 5b28b040..197c2698 100755 --- a/apache2/autogen.sh +++ b/apache2/autogen.sh @@ -1,4 +1,6 @@ #!/bin/sh +rm -rf autom4te.cache + #automake --add-missing --copy autoreconf --install diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c index add28b38..9655783d 100644 --- a/apache2/modsecurity.c +++ b/apache2/modsecurity.c @@ -277,7 +277,7 @@ apr_status_t modsecurity_tx_init(modsec_rec *msr) { } /* Check if we are forcing buffering, then use memory only. */ - if (msr->txcfg->reqbody_buffering) { + if (msr->txcfg->reqbody_buffering != REQUEST_BODY_FORCEBUF_OFF) { msr->msc_reqbody_storage = MSC_REQBODY_MEMORY; msr->msc_reqbody_spilltodisk = 0; } diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h index 489d50ad..3c3e3ceb 100644 --- a/apache2/modsecurity.h +++ b/apache2/modsecurity.h @@ -90,6 +90,9 @@ typedef struct msc_string msc_string; #define RESPONSE_BODY_LIMIT_ACTION_REJECT 0 #define RESPONSE_BODY_LIMIT_ACTION_PARTIAL 1 +#define REQUEST_BODY_FORCEBUF_OFF 0 +#define REQUEST_BODY_FORCEBUF_ON 1 + #define SECACTION_TARGETS "REQUEST_URI" #define SECACTION_ARGS "@unconditionalMatch" diff --git a/apache2/msc_multipart.c b/apache2/msc_multipart.c index c87f7d1b..d5268eb7 100644 --- a/apache2/msc_multipart.c +++ b/apache2/msc_multipart.c @@ -45,7 +45,7 @@ static char *multipart_construct_filename(modsec_rec *msr) { */ p = filename = apr_pstrdup(msr->mp, q); while((c = *p) != 0) { - if (!( isalnum(c)||(c == '.') )) *p = '_'; + if (!( isalnum(c) || (c == '.') )) *p = '_'; p++; } @@ -67,7 +67,7 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value) /* see if there are any other parts to parse */ p = c_d_value + 9; - while((*p == '\t')||(*p == ' ')) p++; + while((*p == '\t') || (*p == ' ')) p++; if (*p == '\0') return 1; /* this is OK */ if (*p != ';') return -2; @@ -79,26 +79,35 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value) char *name = NULL, *value = NULL, *start = NULL; /* go over the whitespace */ - while((*p == '\t')||(*p == ' ')) p++; + while((*p == '\t') || (*p == ' ')) p++; if (*p == '\0') return -3; start = p; - while((*p != '\0')&&(*p != '=')&&(*p != '\t')&&(*p != ' ')) p++; + while((*p != '\0') && (*p != '=') && (*p != '\t') && (*p != ' ')) p++; if (*p == '\0') return -4; name = apr_pstrmemdup(msr->mp, start, (p - start)); - while((*p == '\t')||(*p == ' ')) p++; + while((*p == '\t') || (*p == ' ')) p++; if (*p == '\0') return -5; if (*p != '=') return -13; p++; - while((*p == '\t')||(*p == ' ')) p++; + while((*p == '\t') || (*p == ' ')) p++; if (*p == '\0') return -6; - if (*p == '"') { + /* Accept both quotes as some backends will accept them, but + * technically "'" is invalid and so flag_invalid_quoting is + * set so the user can deal with it in the rules if they so wish. + */ + if ((*p == '"') || (*p == '\'')) { /* quoted */ + char quote = *p; + + if (quote == '\'') { + msr->mpd->flag_invalid_quoting = 1; + } p++; if (*p == '\0') return -7; @@ -113,8 +122,8 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value) /* improper escaping */ return -8; } - /* only " and \ can be escaped */ - if ((*(p + 1) == '"')||(*(p + 1) == '\\')) { + /* only quote and \ can be escaped */ + if ((*(p + 1) == quote) || (*(p + 1) == '\\')) { p++; } else { @@ -128,8 +137,7 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value) */ } } - else - if (*p == '"') { + else if (*p == quote) { *t = '\0'; break; } @@ -144,14 +152,18 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value) /* not quoted */ start = p; - while((*p != '\0')&&(is_token_char(*p))) p++; + while((*p != '\0') && (is_token_char(*p))) p++; value = apr_pstrmemdup(msr->mp, start, (p - start)); } /* evaluate part */ if (strcmp(name, "name") == 0) { - if (msr->mpd->mpp->name != NULL) return -14; + if (msr->mpd->mpp->name != NULL) { + msr_log(msr, 4, "Multipart: Warning: Duplicate Content-Disposition name: %s", + log_escape_nq(msr->mp, value)); + return -14; + } msr->mpd->mpp->name = value; if (msr->txcfg->debuglog_level >= 9) { @@ -161,7 +173,11 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value) } else if (strcmp(name, "filename") == 0) { - if (msr->mpd->mpp->filename != NULL) return -15; + if (msr->mpd->mpp->filename != NULL) { + msr_log(msr, 4, "Multipart: Warning: Duplicate Content-Disposition filename: %s", + log_escape_nq(msr->mp, value)); + return -15; + } msr->mpd->mpp->filename = value; if (msr->txcfg->debuglog_level >= 9) { @@ -172,7 +188,7 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value) else return -11; if (*p != '\0') { - while((*p == '\t')||(*p == ' ')) p++; + while((*p == '\t') || (*p == ' ')) p++; /* the next character must be a zero or a semi-colon */ if (*p == '\0') return 1; /* this is OK */ if (*p != ';') return -12; @@ -263,7 +279,7 @@ static int multipart_process_part_header(modsec_rec *msr, char **error_msg) { } else { /* Header line. */ - if ((msr->mpd->buf[0] == '\t')||(msr->mpd->buf[0] == ' ')) { + if ((msr->mpd->buf[0] == '\t') || (msr->mpd->buf[0] == ' ')) { char *header_value, *new_value, *data; /* header folding, add data to the header we are building */ @@ -277,7 +293,7 @@ static int multipart_process_part_header(modsec_rec *msr, char **error_msg) { /* locate the beginning of data */ data = msr->mpd->buf; - while((*data == '\t')||(*data == ' ')) data++; + while((*data == '\t') || (*data == ' ')) data++; new_value = apr_pstrdup(msr->mp, data); remove_lf_crlf_inplace(new_value); @@ -303,7 +319,7 @@ static int multipart_process_part_header(modsec_rec *msr, char **error_msg) { /* new header */ data = msr->mpd->buf; - while((*data != ':')&&(*data != '\0')) data++; + while((*data != ':') && (*data != '\0')) data++; if (*data == '\0') { *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid part header (colon missing): %s.", log_escape_nq(msr->mp, msr->mpd->buf)); @@ -320,7 +336,7 @@ static int multipart_process_part_header(modsec_rec *msr, char **error_msg) { /* extract the value value */ data++; - while((*data == '\t')||(*data == ' ')) data++; + while((*data == '\t') || (*data == ' ')) data++; header_value = apr_pstrdup(msr->mp, data); remove_lf_crlf_inplace(header_value); @@ -713,7 +729,7 @@ int multipart_init(modsec_rec *msr, char **error_msg) { /* Check for extra characters before the boundary. */ for (p = (char *)(msr->request_content_type + 19); p < msr->mpd->boundary; p++) { if (!isspace(*p)) { - if ((seen_semicolon == 0)&&(*p == ';')) { + if ((seen_semicolon == 0) && (*p == ';')) { seen_semicolon = 1; /* It is OK to have one semicolon. */ } else { msr->mpd->flag_error = 1; @@ -761,7 +777,7 @@ int multipart_init(modsec_rec *msr, char **error_msg) { } /* Is the boundary quoted? */ - if ((len >= 2)&&(*b == '"')&&(*(b + len - 1) == '"')) { + if ((len >= 2) && (*b == '"') && (*(b + len - 1) == '"')) { /* Quoted. */ msr->mpd->boundary = apr_pstrndup(msr->mp, b + 1, len - 2); if (msr->mpd->boundary == NULL) return -1; @@ -771,7 +787,7 @@ int multipart_init(modsec_rec *msr, char **error_msg) { /* Test for partial quoting. */ if ( (*b == '"') - || ((len >= 2)&&(*(b + len - 1) == '"')) ) + || ((len >= 2) && (*(b + len - 1) == '"')) ) { msr->mpd->flag_error = 1; *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid boundary in C-T (quote)."); @@ -865,14 +881,15 @@ int multipart_complete(modsec_rec *msr, char **error_msg) { } } - if ((msr->mpd->seen_data != 0)&&(msr->mpd->is_complete == 0)) { + if ((msr->mpd->seen_data != 0) && (msr->mpd->is_complete == 0)) { if (msr->mpd->boundary_count > 0) { /* Check if we have the final boundary (that we haven't * 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))) - && (*(msr->mpd->buf) == '-')&&(*(msr->mpd->buf + 1) == '-') + && (*(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) == '-') ) @@ -939,7 +956,7 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf, char c = *inptr; int process_buffer = 0; - if ((c == '\r')&&(msr->mpd->bufleft == 1)) { + if ((c == '\r') && (msr->mpd->bufleft == 1)) { /* we don't want to take \r as the last byte in the buffer */ process_buffer = 1; } else { @@ -954,25 +971,26 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf, /* until we either reach the end of the line * or the end of our internal buffer */ - if ((c == '\n')||(msr->mpd->bufleft == 0)||(process_buffer)) { + if ((c == '\n') || (msr->mpd->bufleft == 0) || (process_buffer)) { int processed_as_boundary = 0; *(msr->mpd->bufptr) = 0; /* Do we have something that looks like a boundary? */ - if (msr->mpd->buf_contains_line + if ( msr->mpd->buf_contains_line && (strlen(msr->mpd->buf) > 3) - && (((*(msr->mpd->buf) == '-'))&&(*(msr->mpd->buf + 1) == '-')) ) + && (*(msr->mpd->buf) == '-') + && (*(msr->mpd->buf + 1) == '-') ) { /* Does it match our boundary? */ - if ((strlen(msr->mpd->buf) >= strlen(msr->mpd->boundary) + 2) + if ( (strlen(msr->mpd->buf) >= strlen(msr->mpd->boundary) + 2) && (strncmp(msr->mpd->buf + 2, msr->mpd->boundary, strlen(msr->mpd->boundary)) == 0) ) { char *boundary_end = msr->mpd->buf + 2 + strlen(msr->mpd->boundary); int is_final = 0; /* Is this the final boundary? */ - if ((*boundary_end == '-')&&(*(boundary_end + 1)== '-')) { + if ((*boundary_end == '-') && (*(boundary_end + 1)== '-')) { is_final = 1; boundary_end += 2; @@ -1057,7 +1075,8 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf, char *p = msr->mpd->buf; for(i = 0; i < len; i++) { - if ((p[i] == '-')&&(i + 1 < len)&&(p[i + 1] == '-')) { + if ((p[i] == '-') && (i + 1 < len) && (p[i + 1] == '-')) + { if (strncmp(p + i + 2, msr->mpd->boundary, strlen(msr->mpd->boundary)) == 0) { msr->mpd->flag_unmatched_boundary = 1; break; @@ -1077,7 +1096,7 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf, } } else { if (msr->mpd->mpp_state == 0) { - if ((msr->mpd->bufleft == 0)||(process_buffer)) { + if ((msr->mpd->bufleft == 0) || (process_buffer)) { /* part header lines must be shorter than * MULTIPART_BUF_SIZE bytes */ @@ -1115,7 +1134,7 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf, msr->mpd->buf_contains_line = (c == 0x0a) ? 1 : 0; } - if ((msr->mpd->is_complete)&&(inleft != 0)) { + if ((msr->mpd->is_complete) && (inleft != 0)) { msr->mpd->flag_data_after = 1; if (msr->txcfg->debuglog_level >= 4) { @@ -1189,7 +1208,9 @@ apr_status_t multipart_cleanup(modsec_rec *msr) { parts = (multipart_part **)msr->mpd->parts->elts; for(i = 0; i < msr->mpd->parts->nelts; i++) { - if ((parts[i]->type == MULTIPART_FILE)&&(parts[i]->tmp_file_size == 0)) { + if ( (parts[i]->type == MULTIPART_FILE) + && (parts[i]->tmp_file_size == 0)) + { /* Delete empty file. */ if (parts[i]->tmp_file_name != NULL) { /* make sure it is closed first */ @@ -1300,7 +1321,7 @@ char *multipart_reconstruct_urlencoded_body_sanitise(modsec_rec *msr) { /* allocate the buffer */ body = apr_palloc(msr->mp, body_len + 1); - if ((body == NULL)||(body_len + 1 == 0)) return NULL; + if ((body == NULL) || (body_len + 1 == 0)) return NULL; *body = 0; parts = (multipart_part **)msr->mpd->parts->elts; diff --git a/apache2/msc_multipart.h b/apache2/msc_multipart.h index 9cda6145..1cd4888e 100644 --- a/apache2/msc_multipart.h +++ b/apache2/msc_multipart.h @@ -117,6 +117,7 @@ struct multipart_data { int flag_unmatched_boundary; int flag_boundary_whitespace; int flag_missing_semicolon; + int flag_invalid_quoting; }; diff --git a/apache2/msc_release.h b/apache2/msc_release.h index ef9efc52..efd32ba8 100644 --- a/apache2/msc_release.h +++ b/apache2/msc_release.h @@ -48,7 +48,7 @@ extern DSOLOCAL modsec_build_type_rec modsec_build_type[]; #define MODSEC_VERSION_MAJOR "2" #define MODSEC_VERSION_MINOR "5" -#define MODSEC_VERSION_MAINT "10" +#define MODSEC_VERSION_MAINT "11" #define MODSEC_VERSION_TYPE "" #define MODSEC_VERSION_RELEASE "" diff --git a/apache2/msc_reqbody.c b/apache2/msc_reqbody.c index 0ad10008..9fc629e1 100644 --- a/apache2/msc_reqbody.c +++ b/apache2/msc_reqbody.c @@ -309,7 +309,7 @@ apr_status_t modsecurity_request_body_store(modsec_rec *msr, msr->msc_reqbody_processor); return -1; } - } else if (msr->txcfg->reqbody_buffering) { + } else if (msr->txcfg->reqbody_buffering != REQUEST_BODY_FORCEBUF_OFF) { /* Increase per-request data length counter if forcing buffering. */ msr->msc_reqbody_no_files_length += length; } @@ -479,7 +479,7 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr, char **error_msg) { return -1; } } - } else if (msr->txcfg->reqbody_buffering) { + } else if (msr->txcfg->reqbody_buffering != REQUEST_BODY_FORCEBUF_OFF) { /* Convert to a single continous buffer, but don't do anything else. */ return modsecurity_request_body_end_raw(msr, error_msg); } diff --git a/apache2/re_actions.c b/apache2/re_actions.c index e1909953..e6de3291 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -714,11 +714,10 @@ static char *msre_action_ctl_validate(msre_engine *engine, msre_action *action) return NULL; } else if (strcasecmp(name, "forceRequestBodyVariable") == 0) { - if (parse_boolean(value) == -1) { - return apr_psprintf(engine->mp, "Invalid setting for ctl name " - " forceRequestBodyVariable: %s", value); - } - return NULL; + if (strcasecmp(value, "on") == 0) return NULL; + if (strcasecmp(value, "off") == 0) return NULL; + return apr_psprintf(engine->mp, "Invalid setting for ctl name " + " forceRequestBodyVariable: %s", value); } else if (strcasecmp(name, "responseBodyAccess") == 0) { if (parse_boolean(value) == -1) { @@ -839,12 +838,17 @@ static apr_status_t msre_action_ctl_execute(modsec_rec *msr, apr_pool_t *mptmp, return 1; } else if (strcasecmp(name, "forceRequestBodyVariable") == 0) { - int pv = parse_boolean(value); + if (strcasecmp(value, "on") == 0) { + msr->txcfg->reqbody_buffering = REQUEST_BODY_FORCEBUF_ON; + msr->usercfg->reqbody_buffering = REQUEST_BODY_FORCEBUF_ON; + } + else + if (strcasecmp(value, "off") == 0) { + msr->txcfg->reqbody_buffering = REQUEST_BODY_FORCEBUF_OFF; + msr->usercfg->reqbody_buffering = REQUEST_BODY_FORCEBUF_OFF; + } - if (pv == -1) return -1; - msr->txcfg->reqbody_buffering = pv; - msr->usercfg->reqbody_buffering = pv; - msr_log(msr, 4, "Ctl: Set requestBodyAccess to %d.", pv); + msr_log(msr, 4, "Ctl: Set requestBodyAccess to %d.", msr->txcfg->reqbody_buffering); return 1; } else @@ -880,7 +884,7 @@ static apr_status_t msre_action_ctl_execute(modsec_rec *msr, apr_pool_t *mptmp, msr->usercfg->auditlog_flag = AUDITLOG_RELEVANT; } - msr_log(msr, 4, "Ctl: Set auditEngine to %d.", msr->txcfg->auditlog_flag); // TODO + msr_log(msr, 4, "Ctl: Set auditEngine to %d.", msr->txcfg->auditlog_flag); return 1; } else diff --git a/apache2/re_variables.c b/apache2/re_variables.c index 6c3d93c3..b8325d55 100644 --- a/apache2/re_variables.c +++ b/apache2/re_variables.c @@ -1366,6 +1366,18 @@ static int var_multipart_missing_semicolon_generate(modsec_rec *msr, msre_var *v } } +/* MULTIPART_INVALID_QUOTING */ + +static int var_multipart_invalid_quoting_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, + apr_table_t *vartab, apr_pool_t *mptmp) +{ + if ((msr->mpd != NULL)&&(msr->mpd->flag_invalid_quoting != 0)) { + return var_simple_generate(var, vartab, mptmp, "1"); + } else { + return var_simple_generate(var, vartab, mptmp, "0"); + } +} + /* MULTIPART_STRICT_ERROR */ static int var_multipart_strict_error_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, @@ -1381,6 +1393,7 @@ static int var_multipart_strict_error_generate(modsec_rec *msr, msre_var *var, m ||(msr->mpd->flag_header_folding != 0) ||(msr->mpd->flag_lf_line != 0) ||(msr->mpd->flag_missing_semicolon != 0) + ||(msr->mpd->flag_invalid_quoting != 0) ) { return var_simple_generate(var, vartab, mptmp, "1"); } @@ -2454,6 +2467,17 @@ void msre_engine_register_default_variables(msre_engine *engine) { PHASE_REQUEST_BODY ); + /* MULTIPART_INVALID_QUOTING */ + msre_engine_variable_register(engine, + "MULTIPART_INVALID_QUOTING", + VAR_SIMPLE, + 0, 0, + NULL, + var_multipart_invalid_quoting_generate, + VAR_DONT_CACHE, /* flag */ + PHASE_REQUEST_BODY + ); + /* MULTIPART_STRICT_ERROR */ msre_engine_variable_register(engine, "MULTIPART_STRICT_ERROR", diff --git a/apache2/t/regression/misc/00-multipart-parser.t b/apache2/t/regression/misc/00-multipart-parser.t index 850faedf..c1850536 100644 --- a/apache2/t/regression/misc/00-multipart-parser.t +++ b/apache2/t/regression/misc/00-multipart-parser.t @@ -404,3 +404,84 @@ ), ), }, +# Data following final boundary should set flag +{ + type => "misc", + comment => "multipart parser (data after final boundary)", + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + #SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny,status:403" + SecRule MULTIPART_DATA_AFTER "\@eq 1" "phase:2,deny,status:403" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny,status:403" + ), + match_log => { + debug => [ qr/name: a.*variable: 1.*Ignoring data after last boundary/s, 1 ], + -debug => [ qr/Adding request argument \(BODY\): name "b"/s, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => "multipart/form-data; boundary=---------------------------69343412719991675451336310646", + ], + normalize_raw_request_data( + q( + -----------------------------69343412719991675451336310646 + Content-Disposition: form-data; name="a" + + 1 + -----------------------------69343412719991675451336310646-- + -----------------------------69343412719991675451336310646 + Content-Disposition: form-data; name="b" + + 2 + -----------------------------69343412719991675451336310646-- + ), + ), + ), +}, +# Single quoted data is invalid +{ + type => "misc", + comment => "multipart parser (C-D uses single quotes)", + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + #SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny,status:403" + SecRule MULTIPART_INVALID_QUOTING "\@eq 1" "chain,phase:2,deny,status:403" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny,status:403" + ), + match_log => { + debug => [ qr/name: a.*variable: 1.*Duplicate Content-Disposition name/s, 1 ], + -debug => [ qr/Adding request argument \(BODY\): name "b/s, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => "multipart/form-data; boundary=---------------------------69343412719991675451336310646", + ], + normalize_raw_request_data( + q( + -----------------------------69343412719991675451336310646 + Content-Disposition: form-data; name="a" + + 1 + -----------------------------69343412719991675451336310646 + Content-Disposition: form-data; name=';filename="dummy';name=b;" + + 2 + -----------------------------69343412719991675451336310646-- + ), + ), + ), +}, diff --git a/doc/modsecurity2-apache-reference.xml b/doc/modsecurity2-apache-reference.xml index 5906b7c5..6ff8d334 100644 --- a/doc/modsecurity2-apache-reference.xml +++ b/doc/modsecurity2-apache-reference.xml @@ -6,7 +6,7 @@ Manual - Version 2.5.10 (Sep 18, 2009) + Version 2.5.11 (Nov 4, 2009) 2004-2009 @@ -308,6 +308,12 @@ http://curl.haxx.se/libcurl/ + + Many have had issues with libcurl linked with the GnuTLS + library for SSL/TLS support. It is recommended that the + openssl library be used for SSL/TLS support in libcurl. + + @@ -3111,7 +3117,8 @@ SecRule ARGS "@pm some key words" id:12345,deny,status:500 MULTIPART_DATA_AFTER, MULTIPART_HEADER_FOLDING, MULTIPART_LF_LINE, - MULTIPART_SEMICOLON_MISSING. Each of these variables + MULTIPART_SEMICOLON_MISSING + MULTIPART_INVALID_QUOTING. Each of these variables covers one unusual (although sometimes legal) aspect of the request body in multipart/form-data format. Your policies should always contain a rule to check either this variable @@ -3133,7 +3140,8 @@ DB %{MULTIPART_DATA_BEFORE}, \ DA %{MULTIPART_DATA_AFTER}, \ HF %{MULTIPART_HEADER_FOLDING}, \ LF %{MULTIPART_LF_LINE}, \ -SM %{MULTIPART_SEMICOLON_MISSING}'" +SM %{MULTIPART_SEMICOLON_MISSING}, \ +IQ %{MULTIPART_INVALID_QUOTING}'" The multipart/form-data parser was upgraded in ModSecurity v2.1.3 to actively look for signs of evasion. Many variables diff --git a/modsecurity.conf-minimal b/modsecurity.conf-minimal index ab9935f2..bc78d2f3 100644 --- a/modsecurity.conf-minimal +++ b/modsecurity.conf-minimal @@ -52,7 +52,8 @@ DB %{MULTIPART_DATA_BEFORE}, \ DA %{MULTIPART_DATA_AFTER}, \ HF %{MULTIPART_HEADER_FOLDING}, \ LF %{MULTIPART_LF_LINE}, \ -SM %{MULTIPART_SEMICOLON_MISSING}'" +SM %{MULTIPART_SEMICOLON_MISSING}, \ +IQ %{MULTIPART_INVALID_QUOTING}'" # Did we see anything that might be a boundary? SecRule MULTIPART_UNMATCHED_BOUNDARY "!@eq 0" \