Skip to content

Multipart parsing fixes and new MULTIPART_PART_HEADERS collection #2797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 7, 2022
Merged

Multipart parsing fixes and new MULTIPART_PART_HEADERS collection #2797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
DD mmm YYYY - 2.9.x (to be released)
-------------------

* Multipart parsing fixes and new MULTIPART_PART_HEADERS collection
[Issue #2797 - @terjanq, @martinhsv]
* Limit rsub null termination to where necessary
[Issue #2794 - @marcstern, @martinhsv]
* IIS: Update dependencies for next planned release
Expand Down
148 changes: 107 additions & 41 deletions apache2/msc_multipart.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. */

Expand Down Expand Up @@ -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.",
Expand Down Expand Up @@ -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') )
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down
19 changes: 19 additions & 0 deletions apache2/msc_multipart.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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
Expand Down
57 changes: 57 additions & 0 deletions apache2/re_variables.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down
45 changes: 45 additions & 0 deletions tests/regression/misc/00-multipart-parser.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

[email protected]
--0000
Content-Disposition: form-data; name="image"; filename="image.jpg"
Content-Type: image/jpeg

BINARYDATA
--0000--
),
),
),
},