Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Step:15
apache2-mod_security2
fix-CVE-2022-48279.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File fix-CVE-2022-48279.patch of Package apache2-mod_security2
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-- + ), + ), + ), +}, +
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
