From c25071b832a95aaf6006815921abd2037460cccf Mon Sep 17 00:00:00 2001 From: ivanr Date: Mon, 3 Dec 2007 14:04:53 +0000 Subject: [PATCH] Initial experimental implementation of SecRequestEncoding. See #390 for more details. --- apache2/apache2_config.c | 27 ++++++++++++++ apache2/modsecurity.h | 3 ++ apache2/msc_multipart.c | 4 ++- apache2/msc_parsers.c | 77 +++++++++++++++++++++++++++++++++------- apache2/msc_parsers.h | 2 ++ 5 files changed, 100 insertions(+), 13 deletions(-) diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index 8a8b205c..b99d0f1b 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -101,6 +101,8 @@ void *create_directory_config(apr_pool_t *mp, char *path) { dcfg->component_signatures = apr_array_make(mp, 16, sizeof(char *)); + dcfg->request_encoding = NOT_SET_P; + return dcfg; } @@ -438,6 +440,9 @@ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child) { merged->component_signatures = apr_array_append(mp, parent->component_signatures, child->component_signatures); + merged->request_encoding = (child->request_encoding == NOT_SET_P + ? parent->request_encoding : child->request_encoding); + return merged; } @@ -517,6 +522,8 @@ void init_directory_config(directory_config *dcfg) { if (dcfg->cache_trans == NOT_SET) dcfg->cache_trans = MODSEC_CACHE_ENABLED; if (dcfg->cache_trans_min == (apr_size_t)NOT_SET) dcfg->cache_trans_min = 15; if (dcfg->cache_trans_max == (apr_size_t)NOT_SET) dcfg->cache_trans_max = 0; + + if (dcfg->request_encoding == NOT_SET_P) dcfg->request_encoding = NULL; } /** @@ -1055,6 +1062,18 @@ static const char *cmd_request_body_access(cmd_parms *cmd, void *_dcfg, const ch return NULL; } +static const char *cmd_request_encoding(cmd_parms *cmd, void *_dcfg, const char *p1) { + directory_config *dcfg = (directory_config *)_dcfg; + if (dcfg == NULL) return NULL; + + // TODO Validate encoding + // return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecRequestBodyAccess: %s", p1); + + dcfg->request_encoding = p1; + + return NULL; +} + static const char *cmd_response_body_access(cmd_parms *cmd, void *_dcfg, const char *p1) { directory_config *dcfg = (directory_config *)_dcfg; if (dcfg == NULL) return NULL; @@ -1699,6 +1718,14 @@ const command_rec module_directives[] = { "maximum request body size ModSecurity will accept, but excluding the size of uploaded files." ), + AP_INIT_TAKE1 ( + "SecRequestEncoding", + cmd_request_encoding, + NULL, + CMD_SCOPE_ANY, + "character encoding used in request." + ), + AP_INIT_TAKE1 ( "SecResponseBodyAccess", cmd_response_body_access, diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h index 6e36652b..715172ba 100644 --- a/apache2/modsecurity.h +++ b/apache2/modsecurity.h @@ -466,6 +466,9 @@ struct directory_config { * appear in the ModSecurity signature in the audit log. */ apr_array_header_t *component_signatures; + + /* Request character encoding. */ + const char *request_encoding; }; struct error_message { diff --git a/apache2/msc_multipart.c b/apache2/msc_multipart.c index 41a6475b..4a9708db 100644 --- a/apache2/msc_multipart.c +++ b/apache2/msc_multipart.c @@ -12,6 +12,7 @@ #include "msc_multipart.h" #include "msc_util.h" +#include "msc_parsers.h" #if 0 @@ -1169,7 +1170,8 @@ int multipart_get_arguments(modsec_rec *msr, char *origin, apr_table_t *argument arg->value_origin_len = parts[i]->length; arg->origin = origin; - apr_table_addn(arguments, arg->name, (void *)arg); + // apr_table_addn(arguments, arg->name, (void *)arg); + add_argument(msr, arguments, arg); } } diff --git a/apache2/msc_parsers.c b/apache2/msc_parsers.c index 69be93e4..e00f1bfd 100644 --- a/apache2/msc_parsers.c +++ b/apache2/msc_parsers.c @@ -9,7 +9,9 @@ * */ #include "msc_parsers.h" +#include "iconv.h" #include +#include /** * @@ -255,10 +257,8 @@ int parse_arguments(modsec_rec *msr, const char *s, apr_size_t inputlength, arg->value_len = 0; arg->value = ""; - apr_table_addn(arguments, arg->name, (void *)arg); - msr_log(msr, 5, "Adding request argument (%s): name \"%s\", value \"%s\"", - arg->origin, log_escape_ex(msr->mp, arg->name, arg->name_len), - log_escape_ex(msr->mp, arg->value, arg->value_len)); + // apr_table_addn(arguments, arg->name, (void *)arg); + add_argument(msr, arguments, arg); arg = (msc_arg *)apr_pcalloc(msr->mp, sizeof(msc_arg)); arg->origin = origin; @@ -274,10 +274,8 @@ int parse_arguments(modsec_rec *msr, const char *s, apr_size_t inputlength, arg->value_len = urldecode_nonstrict_inplace_ex((unsigned char *)value, arg->value_origin_len, invalid_count); arg->value = apr_pstrmemdup(msr->mp, value, arg->value_len); - apr_table_addn(arguments, arg->name, (void *)arg); - msr_log(msr, 5, "Adding request argument (%s): name \"%s\", value \"%s\"", - arg->origin, log_escape_ex(msr->mp, arg->name, arg->name_len), - log_escape_ex(msr->mp, arg->value, arg->value_len)); + // apr_table_addn(arguments, arg->name, (void *)arg); + add_argument(msr, arguments, arg); arg = (msc_arg *)apr_pcalloc(msr->mp, sizeof(msc_arg)); arg->origin = origin; @@ -294,12 +292,67 @@ int parse_arguments(modsec_rec *msr, const char *s, apr_size_t inputlength, arg->value_len = 0; arg->value = ""; - apr_table_addn(arguments, arg->name, (void *)arg); - msr_log(msr, 5, "Adding request argument (%s): name \"%s\", value \"%s\"", - arg->origin, log_escape_ex(msr->mp, arg->name, arg->name_len), - log_escape_ex(msr->mp, arg->value, arg->value_len)); + // apr_table_addn(arguments, arg->name, (void *)arg); + add_argument(msr, arguments, arg); } free(buf); + return 1; } + +/** + * + */ +void add_argument(modsec_rec *msr, apr_table_t *arguments, msc_arg *arg) { + msr_log(msr, 5, "Adding request argument (%s): name \"%s\", value \"%s\"", + arg->origin, log_escape_ex(msr->mp, arg->name, arg->name_len), + log_escape_ex(msr->mp, arg->value, arg->value_len)); + + #ifdef WITH_ICONV + if (msr->txcfg->request_encoding != NULL) { + iconv_t convset; + + // TODO Convert parameter names too. + + /* Initialise iconv. */ + convset = iconv_open("ISO-8859-1", msr->txcfg->request_encoding); + if (convset == (iconv_t)(-1)) { + msr_log(msr, 1, "Iconv init to %s failed: %s", + msr->txcfg->request_encoding, strerror(errno)); + } else { + int ctlparam = 1; + size_t input_bytes = arg->value_len; + size_t output_bytes = arg->value_len; + char *o, *outbuf; + + // TODO Can output be longer than input? + o = outbuf = apr_palloc(msr->mp, output_bytes); + + /* Tell iconv to reject invalid character sequences. */ + iconvctl(convset, ICONV_SET_DISCARD_ILSEQ, &ctlparam); + + /* Convert input character sequence. */ + if (iconv(convset, (const char **)&arg->value, + (size_t *)&input_bytes, + (char **)&outbuf, + (size_t *)&output_bytes) == (size_t)(-1)) + { + msr_log(msr, 1, "Error converting to %s: %s", + msr->txcfg->request_encoding, strerror(errno)); + } else { + arg->value = o; + arg->value_len = arg->value_len - output_bytes; + + msr_log(msr, 5, "Parameter value after conversion from %s: %s", + msr->txcfg->request_encoding, + log_escape_nq_ex(msr->mp, arg->value, arg->value_len)); + } + + iconv_close(convset); + } + } + #endif + + apr_table_addn(arguments, arg->name, (void *)arg); +} diff --git a/apache2/msc_parsers.h b/apache2/msc_parsers.h index d4370c02..fe8afbc7 100644 --- a/apache2/msc_parsers.h +++ b/apache2/msc_parsers.h @@ -20,4 +20,6 @@ int DSOLOCAL parse_cookies_v1(modsec_rec *msr, char *_cookie_header, apr_table_t int DSOLOCAL parse_arguments(modsec_rec *msr, const char *s, apr_size_t inputlength, int argument_separator, const char *origin, apr_table_t *arguments, int *invalid_count); +void DSOLOCAL add_argument(modsec_rec *msr, apr_table_t *arguments, msc_arg *arg); + #endif