From e6699ca7bfd825b8b8ea96a6d8e4fe155f4df173 Mon Sep 17 00:00:00 2001 From: b1v1r Date: Fri, 5 Feb 2010 18:11:36 +0000 Subject: [PATCH] Allow for more robust parsing for multipart header folding. Reported by Sogeti/ESEC R&D (MODSEC-118). Added additional multipart regression tests. --- CHANGES | 5 +- apache2/msc_multipart.c | 28 +- apache2/msc_multipart.h | 1 + apache2/re_variables.c | 24 + .../t/regression/misc/00-multipart-parser.t | 1254 +++++++++++++++++ doc/modsecurity2-apache-reference.xml | 6 +- modsecurity.conf-minimal | 3 +- 7 files changed, 1315 insertions(+), 6 deletions(-) diff --git a/CHANGES b/CHANGES index 5cff7302..2c4bd037 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,9 @@ -14 Jan 2010 - 2.5.12 +18 Jan 2010 - 2.5.12 -------------------- + * Allow for more robust parsing for multipart header folding. Reported + by Sogeti/ESEC R&D. + * Fixed failure to match internally set TX variables with regex (TX:/.../) syntax. diff --git a/apache2/msc_multipart.c b/apache2/msc_multipart.c index bcbba526..89b152dd 100644 --- a/apache2/msc_multipart.c +++ b/apache2/msc_multipart.c @@ -279,12 +279,20 @@ 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 (isspace(msr->mpd->buf[0])) { char *header_value, *new_value, *data; /* header folding, add data to the header we are building */ msr->mpd->flag_header_folding = 1; + /* RFC-2557 states header folding is SP / HTAB, but PHP and + * perhaps others will take any whitespace. So, we accept, + * but with a flag set. + */ + if ((msr->mpd->buf[0] != '\t') && (msr->mpd->buf[0] != ' ')) { + msr->mpd->flag_invalid_header_folding = 1; + } + if (msr->mpd->mpp->last_header_name == NULL) { /* we are not building a header at this moment */ *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid part header (folding error)."); @@ -293,7 +301,15 @@ 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(isspace(*data)) { + /* Flag invalid header folding if an invalid RFC-2557 character is used anywhere + * in the folding prefix. + */ + if ((*data != '\t') && (*data != ' ')) { + msr->mpd->flag_invalid_header_folding = 1; + } + data++; + } new_value = apr_pstrdup(msr->mp, data); remove_lf_crlf_inplace(new_value); @@ -879,6 +895,14 @@ int multipart_complete(modsec_rec *msr, char **error_msg) { if (msr->mpd->flag_missing_semicolon) { msr_log(msr, 4, "Multipart: Warning: missing semicolon in C-T header."); } + + if (msr->mpd->flag_invalid_quoting) { + msr_log(msr, 4, "Multipart: Warning: invalid quoting used."); + } + + if (msr->mpd->flag_invalid_header_folding) { + msr_log(msr, 4, "Multipart: Warning: invalid header folding used."); + } } if ((msr->mpd->seen_data != 0) && (msr->mpd->is_complete == 0)) { diff --git a/apache2/msc_multipart.h b/apache2/msc_multipart.h index f83ff0d4..1ad9d2f9 100644 --- a/apache2/msc_multipart.h +++ b/apache2/msc_multipart.h @@ -118,6 +118,7 @@ struct multipart_data { int flag_boundary_whitespace; int flag_missing_semicolon; int flag_invalid_quoting; + int flag_invalid_header_folding; }; diff --git a/apache2/re_variables.c b/apache2/re_variables.c index c77553bb..2868a376 100644 --- a/apache2/re_variables.c +++ b/apache2/re_variables.c @@ -1378,6 +1378,18 @@ static int var_multipart_invalid_quoting_generate(modsec_rec *msr, msre_var *var } } +/* MULTIPART_INVALID_HEADER_FOLDING */ + +static int var_multipart_invalid_header_folding_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_header_folding != 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, @@ -1394,6 +1406,7 @@ static int var_multipart_strict_error_generate(modsec_rec *msr, msre_var *var, m ||(msr->mpd->flag_lf_line != 0) ||(msr->mpd->flag_missing_semicolon != 0) ||(msr->mpd->flag_invalid_quoting != 0) + ||(msr->mpd->flag_invalid_header_folding != 0) ) { return var_simple_generate(var, vartab, mptmp, "1"); } @@ -2478,6 +2491,17 @@ void msre_engine_register_default_variables(msre_engine *engine) { PHASE_REQUEST_BODY ); + /* MULTIPART_INVALID_HEADER_FOLDING */ + msre_engine_variable_register(engine, + "MULTIPART_INVALID_HEADER_FOLDING", + VAR_SIMPLE, + 0, 0, + NULL, + var_multipart_invalid_header_folding_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 5d156e15..da1a3eee 100644 --- a/apache2/t/regression/misc/00-multipart-parser.t +++ b/apache2/t/regression/misc/00-multipart-parser.t @@ -1,5 +1,51 @@ ### Multipart parser tests +# Normal +{ + type => "misc", + comment => "multipart parser (normal)", + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/Added file part [0-9a-h]+ to the list: name "image" file name "image.jpg" \(offset 258, length 10\).*Adding request argument \(BODY\): name "name", value "Brian Rectanus".*Adding request argument \(BODY\): name "email", value "brian.rectanus\@breach.com"/s, 1 ], + -debug => [ qr/Multipart.*(?i:error|warning)/, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary=0000), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, + # Final CRLF or not, we should still work { type => "misc", @@ -41,6 +87,8 @@ ), ), }, + +# No final CRLF { type => "misc", comment => "multipart parser (no final CRLF)", @@ -362,6 +410,7 @@ ), ), }, + # Zero length part name should not crash { type => "misc", @@ -404,6 +453,211 @@ ), ), }, + +# Part header folding +{ + type => "misc", + comment => "multipart parser (part header folding - space)", + 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_HEADER_FOLDING "!\@eq 1" "phase:2,deny,status:403" + SecRule MULTIPART_INVALID_HEADER_FOLDING "!\@eq 0" "phase:2,deny,status:403" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny,status:403" + ), + match_log => { + debug => [ qr/name: a.*variable: 1.*name: b.*variable: 2/s, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + 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="b" + + 2 + -----------------------------69343412719991675451336310646-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (part header folding - tab)", + 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_HEADER_FOLDING "!\@eq 1" "phase:2,deny,status:403" + SecRule MULTIPART_INVALID_HEADER_FOLDING "!\@eq 0" "phase:2,deny,status:403" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny,status:403" + ), + match_log => { + debug => [ qr/name: a.*variable: 1.*name: b.*variable: 2/s, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + 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="b" + + 2 + -----------------------------69343412719991675451336310646-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (part header folding - mixed)", + 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_HEADER_FOLDING "!\@eq 1" "phase:2,deny,status:403" + SecRule MULTIPART_INVALID_HEADER_FOLDING "!\@eq 0" "phase:2,deny,status:403" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny,status:403" + ), + match_log => { + debug => [ qr/name: a.*variable: 1.*name: b.*variable: 2/s, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + 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="b" + + 2 + -----------------------------69343412719991675451336310646-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (part header folding - invalid)", + 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_HEADER_FOLDING "!\@eq 1" "phase:2,deny,status:403" + SecRule MULTIPART_INVALID_HEADER_FOLDING "!\@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.*name: b.*variable: 2/s, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + 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="b" + + 2 + -----------------------------69343412719991675451336310646-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (part header folding - mixed invalid)", + 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_HEADER_FOLDING "!\@eq 1" "phase:2,deny,status:403" + SecRule MULTIPART_INVALID_HEADER_FOLDING "!\@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.*name: b.*variable: 2/s, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + 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="b" + + 2 + -----------------------------69343412719991675451336310646-- + ), + ), + ), +}, + # Data following final boundary should set flag { type => "misc", @@ -443,6 +697,7 @@ ), ), }, + # Single quoted data is invalid { type => "misc", @@ -483,3 +738,1002 @@ ), ), }, + +# Invalid boundary separators +{ + type => "misc", + comment => "multipart parser (invalid C-T boundary separator - comma)", + note => q( + PHP 5.2.3 - no effect. + ), + conf => qq( + SecRuleEngine On + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + error => [ qr/Invalid boundary in C-T \(malformed\)/, 1 ], + + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data, boundary=0000), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (invalid C-T boundary separator - space)", + note => q( + PHP 5.2.3 - no effect. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data boundary=0000), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, + +# Invalid boundary parameter names +{ + type => "misc", + comment => "multipart parser (invalid C-T boundary parameter name - case)", + note => q( + PHP 5.2.3 - does not recognise boundary. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + error => [ qr/Invalid boundary in C-T \(case sensitivity\)/, 1 ], + + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; bOundAry=0000), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (invalid C-T boundary parameter name - trailing chars)", + note => q( + PHP 5.2.3 - does not recognise boundary. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + error => [ qr/Invalid boundary in C-T \(parameter name\)/, 1 ], + + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary123=0000), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, + +# Invalid boundaries +{ + type => "misc", + comment => "multipart parser (multiple C-T boundaries - first quoted)", + note => q( + PHP 5.2.3 - uses first boundary. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/Multiple boundary parameters in C-T/, 1 ], + + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary="0000"; boundary=1111), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (multiple C-T boundaries - comma separated)", + note => q( + PHP 5.2.3 - uses first boundary. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/Multiple boundary parameters in C-T/, 1 ], + + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary=0000, boundary=1111), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (boundary whitespace in C-T - after name)", + note => q( + PHP 5.2.3 - no effect. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary =0000), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (boundary whitespace in C-T - before value)", + note => q( + PHP 5.2.3 - uses " 0000" as boundary. + We should probably interpret it as "0000" and just flag a strict error. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/boundary whitespace in C-T header/, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary= 0000), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (boundary whitespace in C-T - after value)", + note => q( + PHP 5.2.3 - no effect. + Apache just silently removes the trailing whitespace for us. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/Added file part [0-9a-h]+ to the list: name "image" file name "image.jpg" \(offset 258, length 10\).*Adding request argument \(BODY\): name "name", value "Brian Rectanus".*Adding request argument \(BODY\): name "email", value "brian.rectanus\@breach.com"/s, 1 ], + -debug => [ qr/Multipart.*(?i:error|warning)/, 1 ], + }, + match_response => { + status => qr/^200$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary=0000 ), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, + +# Special chars +{ + type => "misc", + comment => "multipart parser (boundary special char - trailing whitespace+token)", + note => q( + PHP 5.2.3 - uses "0000 1111" as boundary. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/No boundaries found in payload/, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary=0000 1111), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (boundary special char - trailing comma+token)", + note => q( + PHP 5.2.3 - uses "0000" as boundary. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/Invalid boundary in C-T \(characters\)/, 1 ], + + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary=0000,1111), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, + +# Quoting +{ + type => "misc", + comment => "multipart parser (quoted boundary - normal)", + note => q( + PHP 5.2.3 - no effect. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/boundary was quoted/, 1 ], + + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary="0000"), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (quoted boundary value - whitespace before)", + note => q( + PHP 5.2.3 - uses " 0000" as boundary. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/boundary was quoted.*No boundaries found in payload/s, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary=" 0000"), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (quoted boundary value - whitespace after)", + note => q( + PHP 5.2.3 - uses "0000 " as boundary. + Trailing whitespace violates RFC as well. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/boundary was quoted.*No boundaries found in payload/s, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary="0000 "), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (quoted boundary value - whitespace between)", + note => q( + PHP 5.2.3 - uses "0000 " as boundary. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/boundary was quoted/s, 1 ], + -debug => [ qr/No boundaries found in payload/s, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary="0000 1111"), + ], + normalize_raw_request_data( + q( + --0000 1111 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 1111 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 1111 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000 1111-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (quoted boundary value - contained quote)", + note => q( + PHP 5.2.3 - uses "0000 " as boundary. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/Invalid boundary in C-T \(characters\)/, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary="00\"00"), + ], + normalize_raw_request_data( + q( + --00"00 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --00"00 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --00"00 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --00"00-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (quoted boundary value - two quoted values)", + note => q( + PHP 5.2.3 - does not work, uses "00" as boundary. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/Invalid boundary in C-T \(characters\)/, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary="00""00"), + ], + normalize_raw_request_data( + q( + --00"00 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --00"00 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --00"00 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --00"00-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (partial quoted boundary value - only start quote)", + note => q( + PHP 5.2.3 - breaks parsing. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/Invalid boundary in C-T \(quote\)/, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary="0000), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (partial quoted boundary value - only end quote)", + note => q( + PHP 5.2.3 - breaks parsing. + ), + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/Invalid boundary in C-T \(quote\)/, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary=0000"), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: form-data; name="image"; filename="image.jpg" + Content-Type: image/jpeg + + BINARYDATA + --0000-- + ), + ), + ), +}, + +# Multipart mixed +{ + type => "misc", + comment => "multipart parser (multipart mixed - normal)", + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/Invalid Content-Disposition header/, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary=0000), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Disposition: attachment + Content-Type: multipart/mixed; boundary=BbC04y + + --BbC04y + Content-Disposition: file; filename="file1.txt" + Content-Type: text/plain + + ... contents of file1.txt ... + --BbC04y + Content-Disposition: file; filename="file2.gif + Content-Type: image/jpeg + Content-Transfer-Encoding: binary + + ...contents of file2.gif... + --0000-- + ), + ), + ), +}, +{ + type => "misc", + comment => "multipart parser (multipart mixed - missing disposition)", + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 9 + SecRequestBodyAccess On + SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny" + SecRule MULTIPART_UNMATCHED_BOUNDARY "\@eq 1" "phase:2,deny" + SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny" + ), + match_log => { + debug => [ qr/Part missing Content-Disposition header/, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", + [ + "Content-Type" => q(multipart/form-data; boundary=0000), + ], + normalize_raw_request_data( + q( + --0000 + Content-Disposition: form-data; name="name" + + Brian Rectanus + --0000 + Content-Disposition: form-data; name="email" + + brian.rectanus@breach.com + --0000 + Content-Type: multipart/mixed; boundary=BbC04y + + --BbC04y + Content-Disposition: file; filename="file1.txt" + Content-Type: text/plain + + ... contents of file1.txt ... + --BbC04y + Content-Disposition: file; filename="file2.gif + Content-Type: image/jpeg + Content-Transfer-Encoding: binary + + ...contents of file2.gif... + --0000-- + ), + ), + ), +}, diff --git a/doc/modsecurity2-apache-reference.xml b/doc/modsecurity2-apache-reference.xml index fc280a19..e9087117 100644 --- a/doc/modsecurity2-apache-reference.xml +++ b/doc/modsecurity2-apache-reference.xml @@ -3173,7 +3173,8 @@ SecRule ARGS "@pm some key words" id:12345,deny,status:500 MULTIPART_HEADER_FOLDING, MULTIPART_LF_LINE, MULTIPART_SEMICOLON_MISSING - MULTIPART_INVALID_QUOTING. Each of these variables + MULTIPART_INVALID_QUOTING + MULTIPART_INVALID_HEADER_FOLDING. 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 @@ -3196,7 +3197,8 @@ DA %{MULTIPART_DATA_AFTER}, \ HF %{MULTIPART_HEADER_FOLDING}, \ LF %{MULTIPART_LF_LINE}, \ SM %{MULTIPART_SEMICOLON_MISSING}, \ -IQ %{MULTIPART_INVALID_QUOTING}'" +IQ %{MULTIPART_INVALID_QUOTING}, \ +IQ %{MULTIPART_INVALID_HEADER_FOLDING}'" 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 a02b00e1..584cac19 100644 --- a/modsecurity.conf-minimal +++ b/modsecurity.conf-minimal @@ -57,7 +57,8 @@ DA %{MULTIPART_DATA_AFTER}, \ HF %{MULTIPART_HEADER_FOLDING}, \ LF %{MULTIPART_LF_LINE}, \ SM %{MULTIPART_SEMICOLON_MISSING}, \ -IQ %{MULTIPART_INVALID_QUOTING}'" +IQ %{MULTIPART_INVALID_QUOTING}, \ +IH %{MULTIPART_INVALID_HEADER_FOLDING}'" # Did we see anything that might be a boundary? SecRule MULTIPART_UNMATCHED_BOUNDARY "!@eq 0" \