diff --git a/apache2/msc_multipart.c b/apache2/msc_multipart.c index 3acbd519..bd395cf6 100644 --- a/apache2/msc_multipart.c +++ b/apache2/msc_multipart.c @@ -535,6 +535,73 @@ static int multipart_process_boundary(modsec_rec *msr, int last_part, char **err return 1; } +static int multipart_boundary_characters_valid(char *boundary) { + unsigned char *p = (unsigned char *)boundary; + unsigned char c; + + if (p == NULL) return -1; + while((c = *p) != '\0') { + /* Control characters and space not allowed. */ + if (c < 32) { + return 0; + } + + /* Non-ASCII characters not allowed. */ + if (c > 126) { + return 0; + } + + switch(c) { + /* Special characters not allowed. */ + case '(' : + case ')' : + case '<' : + case '>' : + case '@' : + case ',' : + case ';' : + case ':' : + case '\\' : + case '"' : + case '/' : + case '[' : + case ']' : + case '?' : + case '=' : + return 0; + break; + + default : + /* Do nothing. */ + break; + } + + p++; + } + + return 0; +} + +static int multipart_count_boundary_params(apr_pool_t *mp, const char *header_value) { + char *duplicate = NULL; + char *s = NULL; + int count = 0; + + if (header_value == NULL) return -1; + duplicate = apr_pstrdup(mp, header_value); + if (duplicate == NULL) return -1; + + /* Performing a case-insensitive search. */ + strtolower_inplace(duplicate); + + s = duplicate; + while((s = strstr(s, "boundary")) != NULL) { + count++; + s += 8; + } + + return count; +} /** * @@ -553,23 +620,60 @@ int multipart_init(modsec_rec *msr, char **error_msg) { msr->mpd->mpp = NULL; if (msr->request_content_type == NULL) { + msr->mpd->flag_error = 1; *error_msg = apr_psprintf(msr->mp, "Multipart: Content-Type header not available."); return -1; } - msr->mpd->boundary = strstr(msr->request_content_type, "boundary="); - if ((msr->mpd->boundary != NULL)&&(*(msr->mpd->boundary + 9) != 0)) { - char *b = msr->mpd->boundary + 9; - int len = strlen(b); + /* Count how many times the word "boundary" appears in the C-T header. */ + if (multipart_count_boundary_params(msr->mp, msr->request_content_type) > 1) { + msr->mpd->flag_error = 1; + *error_msg = apr_psprintf(msr->mp, "Multipart: Multiple boundary parameters in C-T."); + return -1; + } + + msr->mpd->boundary = strstr(msr->request_content_type, "boundary"); + if (msr->mpd->boundary != NULL) { + char *b = NULL; + int len = 0; + + b = strchr(msr->mpd->boundary + 8, '='); + if (b == NULL) { + msr->mpd->flag_error = 1; + *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid boundary in C-T (malformed)."); + return -1; + } + + /* Check parameter name ends well. */ + if (b != (msr->mpd->boundary + 8)) { + if (isspace(*(msr->mpd->boundary + 8))) { + /* Flag for whitespace after parameter name. */ + msr->mpd->flag_boundary_whitespace = 1; + } else { + msr->mpd->flag_error = 1; + *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid boundary in C-T (parameter name)."); + return -1; + } + } + + b++; /* Go over the = character. */ + len = strlen(b); + + /* Flag for whitespace before parameter value. */ + if (isspace(*b)) { + msr->mpd->flag_boundary_whitespace = 1; + } /* Is the boundary quoted? */ if ((len >= 2)&&(*b == '"')&&(*(b + len - 1) == '"')) { /* Quoted. */ msr->mpd->boundary = apr_pstrndup(msr->mp, b + 1, len - 2); + if (msr->mpd->boundary == NULL) return -1; msr->mpd->flag_boundary_quoted = 1; if (strstr(msr->mpd->boundary, "\"") != NULL) { - *error_msg = apr_psprintf(msr->mp, "Invalid boundary in C-T (quote)."); + msr->mpd->flag_error = 1; + *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid boundary in C-T (quote)."); return -1; } } else { @@ -579,25 +683,49 @@ int multipart_init(modsec_rec *msr, char **error_msg) { if ( (*b == '"') || ((len >= 2)&&(*(b + len - 1) == '"')) ) { - *error_msg = apr_psprintf(msr->mp, "Invalid boundary in C-T (quote)."); + msr->mpd->flag_error = 1; + *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid boundary in C-T (quote)."); return -1; } msr->mpd->boundary = apr_pstrdup(msr->mp, b); + if (msr->mpd->boundary == NULL) return -1; msr->mpd->flag_boundary_quoted = 0; } + /* Case-insensitive test for the string "boundary" in the boundary. */ + if (multipart_count_boundary_params(msr->mp, msr->mpd->boundary) != 0) { + msr->mpd->flag_error = 1; + *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid boundary in C-T (content)."); + return -1; + } + + /* Validate the characters used in the boundary. */ + if (multipart_boundary_characters_valid(msr->mpd->boundary) != 1) { + msr->mpd->flag_error = 1; + *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid boundary in C-T (characters)."); + return -1; + } + msr_log(msr, 9, "Multipart: Boundary%s: %s", (msr->mpd->flag_boundary_quoted ? " (quoted)" : ""), log_escape_nq(msr->mp, msr->mpd->boundary)); if (strlen(msr->mpd->boundary) == 0) { - *error_msg = apr_psprintf(msr->mp, "Multipart boundary in C-T empty."); + msr->mpd->flag_error = 1; + *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid boundary in C-T (empty)."); return -1; } } else { - *error_msg = apr_psprintf(msr->mp, "Multipart boundary in C-T not found or invalid."); + /* Test for case-insensitive boundary. Allowed by the RFC but highly unusual. */ + if (multipart_count_boundary_params(msr->mp, msr->request_content_type) > 0) { + msr->mpd->flag_error = 1; + *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid boundary in C-T (case sensitivity)."); + return -1; + } + + *error_msg = apr_psprintf(msr->mp, "Multipart: Boundary not found in C-T."); return -1; } @@ -611,7 +739,11 @@ int multipart_complete(modsec_rec *msr, char **error_log) { if (msr->mpd == NULL) return 1; if ((msr->mpd->seen_data != 0)&&(msr->mpd->is_complete == 0)) { - *error_log = apr_psprintf(msr->mp, "Multipart: Final boundary missing."); + if (msr->mpd->boundary_count > 0) { + *error_log = apr_psprintf(msr->mp, "Multipart: Final boundary missing."); + } else { + *error_log = apr_psprintf(msr->mp, "Multipart: No boundaries found in payload."); + } return -1; } @@ -718,6 +850,7 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf, } processed_as_boundary = 1; + msr->mpd->boundary_count++; } else { /* error */ @@ -728,9 +861,11 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf, return -1; } } else { + char *p = NULL; + + /* Check if an attempt to use quotes around the boundary was made. */ if ( (msr->mpd->flag_boundary_quoted) && (strlen(msr->mpd->buf) >= strlen(msr->mpd->boundary) + 3) - && (((*(msr->mpd->buf) == '-'))&&(*(msr->mpd->buf + 1) == '-')) && (*(msr->mpd->buf + 2) == '"') && (strncmp(msr->mpd->buf + 3, msr->mpd->boundary, strlen(msr->mpd->boundary)) == 0) ) { @@ -739,6 +874,21 @@ int multipart_process_chunk(modsec_rec *msr, const char *buf, return -1; } + /* Check the beginning of the boundary for whitespace. */ + p = msr->mpd->buf + 2; + while(isspace(*p)) { + p++; + } + + if ( (p != msr->mpd->buf) + && (strncmp(p, msr->mpd->boundary, strlen(msr->mpd->boundary))) + ) { + /* Found whitespace at the beginning of the boundary. */ + msr->mpd->flag_error = 1; + *error_msg = apr_psprintf(msr->mp, "Multipart: Invalid boundary (whitespace)."); + return -1; + } + msr->mpd->flag_unmatched_boundary = 1; } } diff --git a/apache2/msc_multipart.h b/apache2/msc_multipart.h index cfef460f..ff9488dc 100644 --- a/apache2/msc_multipart.h +++ b/apache2/msc_multipart.h @@ -61,9 +61,10 @@ struct multipart_data { apr_array_header_t *parts; /* mime boundary used to detect when - * parts end and new begin + * parts end and begin */ char *boundary; + int boundary_count; /* internal buffer and other variables * used while parsing @@ -84,7 +85,7 @@ struct multipart_data { /* part parsing state; 0 means we are reading * headers, 1 means we are collecting data */ - int mpp_state; + int mpp_state; /* because of the way this parsing algorithm * works we hold back the last two bytes of @@ -93,18 +94,19 @@ struct multipart_data { * a boundary; the first byte is an indicator * 0 - no content, 1 - two data bytes available */ - char reserve[4]; - - int seen_data; - int is_complete; - - int flag_error; - int flag_data_before; - int flag_data_after; - int flag_header_folding; - int flag_boundary_quoted; - int flag_lf_line; - int flag_unmatched_boundary; + char reserve[4]; + + int seen_data; + int is_complete; + + int flag_error; + int flag_data_before; + int flag_data_after; + int flag_header_folding; + int flag_boundary_quoted; + int flag_lf_line; + int flag_unmatched_boundary; + int flag_boundary_whitespace; }; diff --git a/apache2/re_variables.c b/apache2/re_variables.c index fba59070..93a65b83 100644 --- a/apache2/re_variables.c +++ b/apache2/re_variables.c @@ -1240,6 +1240,18 @@ static int var_multipart_boundary_quoted_generate(modsec_rec *msr, msre_var *var } } +/* MULTIPART_BOUNDARY_WHITESPACE */ + +static int var_multipart_boundary_whitespace_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_boundary_whitespace != 0)) { + return var_simple_generate(var, vartab, mptmp, "1"); + } else { + return var_simple_generate(var, vartab, mptmp, "0"); + } +} + /* MULTIPART_DATA_AFTER */ static int var_multipart_data_after_generate(modsec_rec *msr, msre_var *var, msre_rule *rule, @@ -1297,6 +1309,7 @@ static int var_multipart_strict_error_generate(modsec_rec *msr, msre_var *var, m /* Respond positive if at least one of the multipart flags is raised. */ if ( (msr->mpd->flag_error) ||(msr->mpd->flag_boundary_quoted != 0) + ||(msr->mpd->flag_boundary_whitespace != 0) ||(msr->mpd->flag_data_before != 0) ||(msr->mpd->flag_data_after != 0) ||(msr->mpd->flag_header_folding != 0) @@ -2264,6 +2277,17 @@ void msre_engine_register_default_variables(msre_engine *engine) { PHASE_REQUEST_BODY ); + /* MULTIPART_BOUNDARY_WHITESPACE */ + msre_engine_variable_register(engine, + "MULTIPART_BOUNDARY_WHITESPACE", + VAR_SIMPLE, + 0, 0, + NULL, + var_multipart_boundary_whitespace_generate, + VAR_CACHE, + PHASE_REQUEST_BODY + ); + /* MULTIPART_DATA_AFTER */ msre_engine_variable_register(engine, "MULTIPART_DATA_AFTER",