From 1fa9655aa580033efb983e5482ffa98899cc5313 Mon Sep 17 00:00:00 2001 From: Ervin Hegedus Date: Sun, 27 Oct 2019 18:32:37 +0000 Subject: [PATCH] Fix MULTIPART_UNMATCHED_BOUNDARY FP errors --- apache2/msc_multipart.c | 11 ++++++++++ apache2/re_variables.c | 7 ++++++- modsecurity.conf-recommended | 40 ++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/apache2/msc_multipart.c b/apache2/msc_multipart.c index 880b13c579..3b0f987f75 100644 --- a/apache2/msc_multipart.c +++ b/apache2/msc_multipart.c @@ -1070,6 +1070,17 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf, { char *boundary_end = msr->mpd->buf + 2 + strlen(msr->mpd->boundary); int is_final = 0; + /* if it match, AND there was a matched boundary at least, + set the flag_unmatched_boundary to 2 + this indicates that there were an opened boundary, which + matches the reference, and here is the final boundary. + The flag will differ from 0, so the previous rules ("!@eq 0") + will catch all "errors", without any modification, but we can + use the new, permission mode with "@eq 1" + */ + if (msr->mpd->boundary_count > 0) { + msr->mpd->flag_unmatched_boundary = 2; + } /* Is this the final boundary? */ if ((*boundary_end == '-') && (*(boundary_end + 1)== '-')) { diff --git a/apache2/re_variables.c b/apache2/re_variables.c index 4007386151..4f1a54677e 100644 --- a/apache2/re_variables.c +++ b/apache2/re_variables.c @@ -1607,7 +1607,12 @@ static int var_multipart_unmatched_boundary_generate(modsec_rec *msr, msre_var * apr_table_t *vartab, apr_pool_t *mptmp) { if ((msr->mpd != NULL)&&(msr->mpd->flag_unmatched_boundary != 0)) { - return var_simple_generate(var, vartab, mptmp, "1"); + if (msr->mpd->flag_unmatched_boundary > 0) { + return var_simple_generate(var, vartab, mptmp, apr_itoa(mptmp, msr->mpd->flag_unmatched_boundary)); + } + else { + return var_simple_generate(var, vartab, mptmp, "1"); + } } else { return var_simple_generate(var, vartab, mptmp, "0"); } diff --git a/modsecurity.conf-recommended b/modsecurity.conf-recommended index 60317acb74..d551153b74 100644 --- a/modsecurity.conf-recommended +++ b/modsecurity.conf-recommended @@ -82,8 +82,48 @@ FL %{MULTIPART_FILE_LIMIT_EXCEEDED}'" # Did we see anything that might be a boundary? # +# Here is a short description about the ModSecurity Multipart parser: the +# parser returns with value 0, if all "boundary-like" line matches with +# the boundary string which given in MIME header. In any other cases it returns +# with different value, eg. 1 or 2. +# +# The RFC 1341 descript the multipart content-type and its syntax must contains +# only three mandatory lines (above the content): +# * Content-Type: multipart/mixed; boundary=BOUNDARY_STRING +# * --BOUNDARY_STRING +# * --BOUNDARY_STRING-- +# +# First line indicates, that this is a multipart content, second shows that +# here starts a part of the multipart content, third shows the end of content. +# +# If there are any other lines, which starts with "--", then it should be +# another boundary id - or not. +# +# After 2.9.3, there are two kinds of types of boundary errors: strict and permissive. +# +# If multipart content contains the three necessary lines with correct order, but +# there are one or more lines with "--", then parser returns with value 2 (non-zero). +# +# If some of the necessary lines (usually the start or end) misses, or the order +# is wrong, then parser returns with value 1 (also a non-zero). +# +# You can choose, which one is what you need. The example below contains the +# 'strict' mode, which means if there are any lines with start of "--", then +# ModSecurity blocked the content. But the next, commented example contains +# the 'permissive' mode, then you check only if the necessary lines exists in +# correct order. Whit this, you can enable to upload PEM files (eg "----BEGIN.."), +# or other text files, which contains eg. HTTP headers. +# +# The difference is only the operator - in strict mode (first) the content blocked +# in case of any non-zero value. In permissive mode (second, commented) the +# content blocked only if the value is explicit 1. If it 0 or 2, the content will +# allowed. +# + SecRule MULTIPART_UNMATCHED_BOUNDARY "!@eq 0" \ "id:'200004',phase:2,t:none,log,deny,msg:'Multipart parser detected a possible unmatched boundary.'" +#SecRule MULTIPART_UNMATCHED_BOUNDARY "@eq 1" \ +#"id:'200004',phase:2,t:none,log,deny,msg:'Multipart parser detected a possible unmatched boundary.'" # PCRE Tuning # We want to avoid a potential RegEx DoS condition