github_mirrors/ModSecurity
3
0
Fork 1
mirror of https://github.com/owasp-modsecurity/ModSecurity.git synced 2026-01-13 15:07:10 +03:00
Code Issues Packages Projects Releases Wiki Activity

Fixed parsing quoted strings in multipart Content-Disposition headers.

Browse Source
This commit is contained in:
b1v1r
2009-11-05 19:36:32 +00:00
parent 92cff5c58e
commit d33f656b93
14 changed files with 207 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
CHANGES
View File

@@ -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

2
apache2/apache2_config.c
View File

@@ -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;

2
apache2/autogen.sh
View File

@@ -1,4 +1,6 @@
#!/bin/sh
rm -rf autom4te.cache
#automake --add-missing --copy
autoreconf --install

2
apache2/modsecurity.c
View File

@@ -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;
}

3
apache2/modsecurity.h
View File

@@ -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"

43
apache2/msc_multipart.c
View File

@@ -97,8 +97,17 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value)
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;
}
@@ -151,7 +159,11 @@ static int multipart_parse_content_disposition(modsec_rec *msr, char *c_d_value)
/* 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) {
@@ -872,7 +888,8 @@ int multipart_complete(modsec_rec *msr, char **error_msg) {
*/
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) == '-') )
@@ -962,7 +979,8 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf,
/* Do we have something that looks like a boundary? */
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)
@@ -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;
@@ -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 */

1
apache2/msc_multipart.h
View File

@@ -117,6 +117,7 @@ struct multipart_data {
int flag_unmatched_boundary;
int flag_boundary_whitespace;
int flag_missing_semicolon;
int flag_invalid_quoting;
};

2
apache2/msc_release.h
View File

@@ -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 ""

4
apache2/msc_reqbody.c
View File

@@ -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);
}

22
apache2/re_actions.c
View File

@@ -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) {
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);
}
return NULL;
} 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

24
apache2/re_variables.c
View File

@@ -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",

81
apache2/t/regression/misc/00-multipart-parser.t
View File

@@ -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--
),
),
),
},

14
doc/modsecurity2-apache-reference.xml
View File

@@ -6,7 +6,7 @@
Manual</title>
<articleinfo>
<releaseinfo>Version 2.5.10 (Sep 18, 2009)</releaseinfo>
<releaseinfo>Version 2.5.11 (Nov 4, 2009)</releaseinfo>
<copyright>
<year>2004-2009</year>
@@ -308,6 +308,12 @@
<para><ulink type=""
url="http://curl.haxx.se/libcurl/">http://curl.haxx.se/libcurl/</ulink></para>
<note>
<para>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.</para>
</note>
</listitem>
</orderedlist>
@@ -3111,7 +3117,8 @@ SecRule ARGS "@pm some key words" id:12345,deny,status:500</programlisting>
<literal>MULTIPART_DATA_AFTER</literal>,
<literal>MULTIPART_HEADER_FOLDING</literal>,
<literal>MULTIPART_LF_LINE</literal>,
<literal>MULTIPART_SEMICOLON_MISSING</literal>. Each of these variables
<literal>MULTIPART_SEMICOLON_MISSING</literal>
<literal>MULTIPART_INVALID_QUOTING</literal>. Each of these variables
covers one unusual (although sometimes legal) aspect of the request body
in <literal>multipart/form-data format</literal>. Your policies should
<emphasis>always</emphasis> 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}'"</programlisting>
SM %{MULTIPART_SEMICOLON_MISSING}, \
IQ %{MULTIPART_INVALID_QUOTING}'"</programlisting>
<para>The <literal>multipart/form-data</literal> parser was upgraded in
ModSecurity v2.1.3 to actively look for signs of evasion. Many variables

3
modsecurity.conf-minimal
View File

@@ -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" \