406 lines
17 KiB
Diff
406 lines
17 KiB
Diff
From 7a489bd07c66d3df19a320b4306e00c49716dbb0 Mon Sep 17 00:00:00 2001
|
|
From: Martin Vierula <martin.vierula@trustwave.com>
|
|
Date: Wed, 7 Sep 2022 11:09:47 -0700
|
|
Subject: [PATCH] Multipart parsing fixes and new MULTIPART_PART_HEADERS
|
|
collection
|
|
|
|
---
|
|
CHANGES | 2 +
|
|
apache2/msc_multipart.c | 148 ++++++++++++++------
|
|
apache2/msc_multipart.h | 19 +++
|
|
apache2/re_variables.c | 57 ++++++++
|
|
tests/regression/misc/00-multipart-parser.t | 45 ++++++
|
|
5 files changed, 230 insertions(+), 41 deletions(-)
|
|
|
|
diff --git a/apache2/msc_multipart.c b/apache2/msc_multipart.c
|
|
index d087c863e..4128ab17e 100644
|
|
--- a/apache2/msc_multipart.c
|
|
+++ b/apache2/msc_multipart.c
|
|
@@ -325,7 +325,14 @@ static int multipart_process_part_header(modsec_rec *msr, char **error_msg) {
|
|
}
|
|
|
|
msr->mpd->mpp_state = 1;
|
|
+ msr->mpd->mpp_substate_part_data_read = 0;
|
|
msr->mpd->mpp->last_header_name = NULL;
|
|
+
|
|
+ /* Record the last part header line in the collection */
|
|
+ if (msr->mpd->mpp->last_header_line != NULL) {
|
|
+ *(char **)apr_array_push(msr->mpd->mpp->header_lines) = msr->mpd->mpp->last_header_line;
|
|
+ msr_log(msr, 9, "Multipart: Added part header line \"%s\"", msr->mpd->mpp->last_header_line);
|
|
+ }
|
|
} else {
|
|
/* Header line. */
|
|
|
|
@@ -379,12 +386,28 @@ static int multipart_process_part_header(modsec_rec *msr, char **error_msg) {
|
|
*error_msg = apr_psprintf(msr->mp, "Multipart: Part header too long.");
|
|
return -1;
|
|
}
|
|
+ if ((msr->mpd->mpp->last_header_line != NULL) && (msr->mpd->mpp->last_header_name != NULL)
|
|
+ && (new_value != NULL)) {
|
|
+ msr->mpd->mpp->last_header_line = apr_psprintf(msr->mp,
|
|
+ "%s: %s", msr->mpd->mpp->last_header_name, new_value);
|
|
+ }
|
|
+
|
|
} else {
|
|
char *header_name, *header_value, *data;
|
|
|
|
/* new header */
|
|
|
|
+ /* Record the most recently-seen part header line in the collection */
|
|
+ if (msr->mpd->mpp->last_header_line != NULL) {
|
|
+ *(char **)apr_array_push(msr->mpd->mpp->header_lines) = msr->mpd->mpp->last_header_line;
|
|
+ msr_log(msr, 9, "Multipart: Added part header line \"%s\"", msr->mpd->mpp->last_header_line);
|
|
+ }
|
|
+
|
|
data = msr->mpd->buf;
|
|
+
|
|
+ msr->mpd->mpp->last_header_line = apr_pstrdup(msr->mp, data);
|
|
+ remove_lf_crlf_inplace(msr->mpd->mpp->last_header_line);
|
|
+
|
|
while((*data != ':') && (*data != '\0')) data++;
|
|
if (*data == '\0') {
|
|
*error_msg = apr_psprintf(msr->mp, "Multipart: Invalid part header (colon missing): %s.",
|
|
@@ -438,6 +461,8 @@ static int multipart_process_part_data(modsec_rec *msr, char **error_msg) {
|
|
if (error_msg == NULL) return -1;
|
|
*error_msg = NULL;
|
|
|
|
+ msr->mpd->mpp_substate_part_data_read = 1;
|
|
+
|
|
/* Preserve some bytes for later. */
|
|
if ( ((MULTIPART_BUF_SIZE - msr->mpd->bufleft) >= 1)
|
|
&& (*(p - 1) == '\n') )
|
|
@@ -680,10 +705,14 @@ static int multipart_process_boundary(modsec_rec *msr, int last_part, char **err
|
|
if (msr->mpd->mpp == NULL) return -1;
|
|
msr->mpd->mpp->type = MULTIPART_FORMDATA;
|
|
msr->mpd->mpp_state = 0;
|
|
+ msr->mpd->mpp_substate_part_data_read = 0;
|
|
|
|
msr->mpd->mpp->headers = apr_table_make(msr->mp, 10);
|
|
if (msr->mpd->mpp->headers == NULL) return -1;
|
|
msr->mpd->mpp->last_header_name = NULL;
|
|
+ msr->mpd->mpp->last_header_line = NULL;
|
|
+ msr->mpd->mpp->header_lines = apr_array_make(msr->mp, 2, sizeof(char *));
|
|
+ if (msr->mpd->mpp->header_lines == NULL) return -1;
|
|
|
|
msr->mpd->reserve[0] = 0;
|
|
msr->mpd->reserve[1] = 0;
|
|
@@ -983,6 +1012,19 @@ int multipart_complete(modsec_rec *msr, char **error_msg) {
|
|
&& (*(msr->mpd->buf + 2 + strlen(msr->mpd->boundary)) == '-')
|
|
&& (*(msr->mpd->buf + 2 + strlen(msr->mpd->boundary) + 1) == '-') )
|
|
{
|
|
+ if ((msr->mpd->crlf_state_buf_end == 2) && (msr->mpd->flag_lf_line != 1)) {
|
|
+ msr->mpd->flag_lf_line = 1;
|
|
+ if (msr->mpd->flag_crlf_line) {
|
|
+ msr_log(msr, 4, "Multipart: Warning: mixed line endings used (CRLF/LF).");
|
|
+ } else {
|
|
+ msr_log(msr, 4, "Multipart: Warning: incorrect line endings used (LF).");
|
|
+ }
|
|
+ }
|
|
+ if (msr->mpd->mpp_substate_part_data_read == 0) {
|
|
+ /* it looks like the final boundary, but it's where part data should begin */
|
|
+ msr->mpd->flag_invalid_part = 1;
|
|
+ msr_log(msr, 4, "Multipart: Warning: Invalid part (data contains final boundary)");
|
|
+ }
|
|
/* Looks like the final boundary - process it. */
|
|
if (multipart_process_boundary(msr, 1 /* final */, error_msg) < 0) {
|
|
msr->mpd->flag_error = 1;
|
|
@@ -1075,54 +1117,63 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf,
|
|
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;
|
|
+ if (msr->mpd->crlf_state_buf_end == 2) {
|
|
+ msr->mpd->flag_lf_line = 1;
|
|
+ }
|
|
+ if ((msr->mpd->mpp_substate_part_data_read == 0) && (msr->mpd->boundary_count > 0)) {
|
|
+ /* string matches our boundary, but it's where part data should begin */
|
|
+ msr->mpd->flag_invalid_part = 1;
|
|
+ msr_log(msr, 4, "Multipart: Warning: Invalid part (data contains boundary)");
|
|
+ } else {
|
|
+ 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)== '-')) {
|
|
+ is_final = 1;
|
|
+ boundary_end += 2;
|
|
+
|
|
+ if (msr->mpd->is_complete != 0) {
|
|
+ msr->mpd->flag_error = 1;
|
|
+ *error_msg = apr_psprintf(msr->mp,
|
|
+ "Multipart: Invalid boundary (final duplicate).");
|
|
+ return -1;
|
|
+ }
|
|
+ }
|
|
|
|
- /* Is this the final boundary? */
|
|
- if ((*boundary_end == '-') && (*(boundary_end + 1)== '-')) {
|
|
- is_final = 1;
|
|
- boundary_end += 2;
|
|
+ /* Allow for CRLF and LF line endings. */
|
|
+ if ( ( (*boundary_end == '\r')
|
|
+ && (*(boundary_end + 1) == '\n')
|
|
+ && (*(boundary_end + 2) == '\0') )
|
|
+ || ( (*boundary_end == '\n')
|
|
+ && (*(boundary_end + 1) == '\0') ) )
|
|
+ {
|
|
+ if (*boundary_end == '\n') {
|
|
+ msr->mpd->flag_lf_line = 1;
|
|
+ } else {
|
|
+ msr->mpd->flag_crlf_line = 1;
|
|
+ }
|
|
|
|
- if (msr->mpd->is_complete != 0) {
|
|
- msr->mpd->flag_error = 1;
|
|
- *error_msg = apr_psprintf(msr->mp,
|
|
- "Multipart: Invalid boundary (final duplicate).");
|
|
- return -1;
|
|
- }
|
|
- }
|
|
+ if (multipart_process_boundary(msr, (is_final ? 1 : 0), error_msg) < 0) {
|
|
+ msr->mpd->flag_error = 1;
|
|
+ return -1;
|
|
+ }
|
|
|
|
- /* Allow for CRLF and LF line endings. */
|
|
- if ( ( (*boundary_end == '\r')
|
|
- && (*(boundary_end + 1) == '\n')
|
|
- && (*(boundary_end + 2) == '\0') )
|
|
- || ( (*boundary_end == '\n')
|
|
- && (*(boundary_end + 1) == '\0') ) )
|
|
- {
|
|
- if (*boundary_end == '\n') {
|
|
- msr->mpd->flag_lf_line = 1;
|
|
- } else {
|
|
- msr->mpd->flag_crlf_line = 1;
|
|
- }
|
|
+ if (is_final) {
|
|
+ msr->mpd->is_complete = 1;
|
|
+ }
|
|
|
|
- if (multipart_process_boundary(msr, (is_final ? 1 : 0), error_msg) < 0) {
|
|
+ processed_as_boundary = 1;
|
|
+ msr->mpd->boundary_count++;
|
|
+ }
|
|
+ else {
|
|
+ /* error */
|
|
msr->mpd->flag_error = 1;
|
|
+ *error_msg = apr_psprintf(msr->mp,
|
|
+ "Multipart: Invalid boundary: %s",
|
|
+ log_escape_nq(msr->mp, msr->mpd->buf));
|
|
return -1;
|
|
}
|
|
-
|
|
- if (is_final) {
|
|
- msr->mpd->is_complete = 1;
|
|
- }
|
|
-
|
|
- processed_as_boundary = 1;
|
|
- msr->mpd->boundary_count++;
|
|
- }
|
|
- else {
|
|
- /* error */
|
|
- msr->mpd->flag_error = 1;
|
|
- *error_msg = apr_psprintf(msr->mp,
|
|
- "Multipart: Invalid boundary: %s",
|
|
- log_escape_nq(msr->mp, msr->mpd->buf));
|
|
- return -1;
|
|
}
|
|
} else { /* It looks like a boundary but we couldn't match it. */
|
|
char *p = NULL;
|
|
@@ -1221,6 +1272,21 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf,
|
|
msr->mpd->bufptr = msr->mpd->buf;
|
|
msr->mpd->bufleft = MULTIPART_BUF_SIZE;
|
|
msr->mpd->buf_contains_line = (c == 0x0a) ? 1 : 0;
|
|
+
|
|
+ if (c == 0x0a) {
|
|
+ if (msr->mpd->crlf_state == 1) {
|
|
+ msr->mpd->crlf_state = 3;
|
|
+ } else {
|
|
+ msr->mpd->crlf_state = 2;
|
|
+ }
|
|
+ }
|
|
+ msr->mpd->crlf_state_buf_end = msr->mpd->crlf_state;
|
|
+ }
|
|
+
|
|
+ if (c == 0x0d) {
|
|
+ msr->mpd->crlf_state = 1;
|
|
+ } else if (c != 0x0a) {
|
|
+ msr->mpd->crlf_state = 0;
|
|
}
|
|
|
|
if ((msr->mpd->is_complete) && (inleft != 0)) {
|
|
diff --git a/apache2/msc_multipart.h b/apache2/msc_multipart.h
|
|
index a0f6a08dd..13db0658f 100644
|
|
--- a/apache2/msc_multipart.h
|
|
+++ b/apache2/msc_multipart.h
|
|
@@ -55,6 +55,8 @@ struct multipart_part {
|
|
|
|
char *last_header_name;
|
|
apr_table_t *headers;
|
|
+ char *last_header_line;
|
|
+ apr_array_header_t *header_lines;
|
|
|
|
unsigned int offset;
|
|
unsigned int length;
|
|
@@ -81,6 +83,15 @@ struct multipart_data {
|
|
char *bufptr;
|
|
int bufleft;
|
|
|
|
+ /* line ending status seen immediately before current position.
|
|
+ * 0 = neither LF nor CR; 1 = prev char CR; 2 = prev char LF alone;
|
|
+ * 3 = prev two chars were CRLF
|
|
+ */
|
|
+ int crlf_state;
|
|
+
|
|
+ /* crlf_state at end of previous buffer */
|
|
+ int crlf_state_buf_end;
|
|
+
|
|
unsigned int buf_offset;
|
|
|
|
/* pointer that keeps track of a part while
|
|
@@ -94,6 +105,14 @@ struct multipart_data {
|
|
*/
|
|
int mpp_state;
|
|
|
|
+ /* part parsing substate; if mpp_state is 1 (collecting
|
|
+ * data), then for this variable:
|
|
+ * 0 means we have not yet read any data between the
|
|
+ * post-headers blank line and the next boundary
|
|
+ * 1 means we have read at some data after that blank line
|
|
+ */
|
|
+ int mpp_substate_part_data_read;
|
|
+
|
|
/* because of the way this parsing algorithm
|
|
* works we hold back the last two bytes of
|
|
* each data chunk so that we can discard it
|
|
diff --git a/apache2/re_variables.c b/apache2/re_variables.c
|
|
index 400738615..f3015acd9 100644
|
|
--- a/apache2/re_variables.c
|
|
+++ b/apache2/re_variables.c
|
|
@@ -1394,6 +1394,52 @@ static int var_files_combined_size_generate(modsec_rec *msr, msre_var *var, msre
|
|
return 1;
|
|
}
|
|
|
|
+/* MULTIPART_PART_HEADERS */
|
|
+
|
|
+static int var_multipart_part_headers_generate(modsec_rec *msr, msre_var *var, msre_rule *rule,
|
|
+ apr_table_t *vartab, apr_pool_t *mptmp)
|
|
+{
|
|
+ multipart_part **parts = NULL;
|
|
+ int i, j, count = 0;
|
|
+
|
|
+ if (msr->mpd == NULL) return 0;
|
|
+
|
|
+ parts = (multipart_part **)msr->mpd->parts->elts;
|
|
+ for(i = 0; i < msr->mpd->parts->nelts; i++) {
|
|
+ int match = 0;
|
|
+
|
|
+ /* Figure out if we want to include this variable. */
|
|
+ if (var->param == NULL) match = 1;
|
|
+ else {
|
|
+ if (var->param_data != NULL) { /* Regex. */
|
|
+ char *my_error_msg = NULL;
|
|
+ if (!(msc_regexec((msc_regex_t *)var->param_data, parts[i]->name,
|
|
+ strlen(parts[i]->name), &my_error_msg) == PCRE_ERROR_NOMATCH)) match = 1;
|
|
+ } else { /* Simple comparison. */
|
|
+ if (strcasecmp(parts[i]->name, var->param) == 0) match = 1;
|
|
+ }
|
|
+ }
|
|
+
|
|
+ /* If we had a match add this argument to the collection. */
|
|
+ if (match) {
|
|
+ for (j = 0; j < parts[i]->header_lines->nelts; j++) {
|
|
+ char *header_line = ((char **)parts[i]->header_lines->elts)[j];
|
|
+ msre_var *rvar = apr_pmemdup(mptmp, var, sizeof(msre_var));
|
|
+
|
|
+ rvar->value = header_line;
|
|
+ rvar->value_len = strlen(rvar->value);
|
|
+ rvar->name = apr_psprintf(mptmp, "MULTIPART_PART_HEADERS:%s",
|
|
+ log_escape_nq(mptmp, parts[i]->name));
|
|
+ apr_table_addn(vartab, rvar->name, (void *)rvar);
|
|
+
|
|
+ count++;
|
|
+ }
|
|
+ }
|
|
+ }
|
|
+
|
|
+ return count;
|
|
+}
|
|
+
|
|
/* MODSEC_BUILD */
|
|
|
|
static int var_modsec_build_generate(modsec_rec *msr, msre_var *var, msre_rule *rule,
|
|
@@ -2966,6 +3012,17 @@ void msre_engine_register_default_variables(msre_engine *engine) {
|
|
PHASE_REQUEST_BODY
|
|
);
|
|
|
|
+ /* MULTIPART_PART_HEADERS */
|
|
+ msre_engine_variable_register(engine,
|
|
+ "MULTIPART_PART_HEADERS",
|
|
+ VAR_LIST,
|
|
+ 0, 1,
|
|
+ var_generic_list_validate,
|
|
+ var_multipart_part_headers_generate,
|
|
+ VAR_CACHE,
|
|
+ PHASE_REQUEST_BODY
|
|
+ );
|
|
+
|
|
/* GEO */
|
|
msre_engine_variable_register(engine,
|
|
"GEO",
|
|
diff --git a/tests/regression/misc/00-multipart-parser.t b/tests/regression/misc/00-multipart-parser.t
|
|
index 3c1f41b7d..e5ee4c13c 100644
|
|
--- a/tests/regression/misc/00-multipart-parser.t
|
|
+++ b/tests/regression/misc/00-multipart-parser.t
|
|
@@ -1849,3 +1849,48 @@
|
|
),
|
|
},
|
|
|
|
+# part headers
|
|
+{
|
|
+ type => "misc",
|
|
+ comment => "multipart parser (part headers)",
|
|
+ conf => qq(
|
|
+ SecRuleEngine On
|
|
+ SecDebugLog $ENV{DEBUG_LOG}
|
|
+ SecDebugLogLevel 9
|
|
+ SecRequestBodyAccess On
|
|
+ SecRule MULTIPART_STRICT_ERROR "\@eq 1" "phase:2,deny,status:400,id:500168"
|
|
+ SecRule REQBODY_PROCESSOR_ERROR "\@eq 1" "phase:2,deny,status:400,id:500169"
|
|
+ SecRule MULTIPART_PART_HEADERS:image "\@rx content-type:.*jpeg" "phase:2,deny,status:403,id:500170,t:lowercase"
|
|
+ ),
|
|
+ match_log => {
|
|
+ debug => [ qr/500170.*against MULTIPART_PART_HEADERS:image.*Rule returned 1./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="username"
|
|
+
|
|
+ Bill
|
|
+ --0000
|
|
+ Content-Disposition: form-data; name="email"
|
|
+
|
|
+ bill@fakesite.com
|
|
+ --0000
|
|
+ Content-Disposition: form-data; name="image"; filename="image.jpg"
|
|
+ Content-Type: image/jpeg
|
|
+
|
|
+ BINARYDATA
|
|
+ --0000--
|
|
+ ),
|
|
+ ),
|
|
+ ),
|
|
+},
|
|
+
|