diff --git a/apache2/acmp.c b/apache2/acmp.c index cd8646dd..734c6666 100644 --- a/apache2/acmp.c +++ b/apache2/acmp.c @@ -9,7 +9,17 @@ * */ #include "acmp.h" + +#ifdef ACMP_USE_UTF8 +/* UTF support */ #include "utf8tables.h" +#else +/* No UTF support */ +#define acmp_utf8_char_t long +#include +#define utf8_lcase(a) apr_tolower(a) +#endif + #include #include #include @@ -58,7 +68,9 @@ struct acmp_btree_node_t { * Data related to parser, not to individual nodes */ struct ACMP { + #ifdef ACMP_USE_UTF8 int is_utf8; + #endif int is_case_sensitive; apr_pool_t *parent_pool; apr_pool_t *pool; @@ -92,6 +104,7 @@ struct ACMP { * Functions for UTF-8 support */ +#ifdef ACMP_USE_UTF8 /** * Returns length of utf-8 sequence based on its first byte */ @@ -152,6 +165,7 @@ static long utf8_lcase(acmp_utf8_char_t ucs_code) { } return ucs_code; } +#endif /* ******************************************************************************* @@ -163,7 +177,11 @@ static long utf8_lcase(acmp_utf8_char_t ucs_code) { * Returns length of given string for parser's encoding */ static size_t acmp_strlen(ACMP *parser, const char *str) { + #ifdef ACMP_USE_UTF8 return (parser->is_utf8 == 0) ? strlen(str) : utf8_strlen(str); + #else + return strlen(str); + #endif } /** @@ -176,15 +194,18 @@ static void acmp_strtoucs(ACMP *parser, const char *str, acmp_utf8_char_t *ucs_c int i; const char *c = str; - if (parser->is_utf8 == 0) { - for (i = 0; i < len; i++) { - *(ucs_chars++) = *(c++); - } - } else { + #ifdef ACMP_USE_UTF8 + if (parser->is_utf8) { for (i = 0; i < len; i++) { *(ucs_chars++) = utf8_decodechar(c); c += utf8_seq_len(c); } + } else + #endif + { + for (i = 0; i < len; i++) { + *(ucs_chars++) = *(c++); + } } } @@ -484,7 +505,9 @@ ACMP *acmp_create(int flags, apr_pool_t *pool) { /* ENH: Check alloc succeded */ parser->pool = p; parser->parent_pool = pool; + #ifdef ACMP_USE_UTF8 parser->is_utf8 = (flags & ACMP_FLAG_UTF8) == 0 ? 0 : 1; + #endif parser->is_case_sensitive = (flags & ACMP_FLAG_CASE_SENSITIVE) == 0 ? 0 : 1; parser->root_node = apr_pcalloc(p, sizeof(acmp_node_t)); /* ENH: Check alloc succeded */ @@ -520,7 +543,9 @@ ACMP *acmp_duplicate(ACMP *parser, apr_pool_t *pool) { /* ENH: Check alloc succeded */ new_parser->pool = p; new_parser->parent_pool = pool; + #ifdef ACMP_USE_UTF8 new_parser->is_utf8 = parser->is_utf8; + #endif new_parser->is_case_sensitive = parser->is_case_sensitive; new_parser->root_node = apr_pcalloc(p, sizeof(acmp_node_t)); /* ENH: Check alloc succeded */ @@ -618,7 +643,9 @@ apr_status_t acmp_add_pattern(ACMP *parser, const char *pattern, */ apr_status_t acmp_process(ACMP *parser, const char *data, apr_size_t len) { acmp_node_t *node, *go_to; + #ifdef ACMP_USE_UTF8 apr_size_t seq_length; + #endif const char *end; if (parser->is_failtree_done == 0) acmp_prepare(parser); @@ -630,6 +657,7 @@ apr_status_t acmp_process(ACMP *parser, const char *data, apr_size_t len) { acmp_utf8_char_t letter; parser->bp_buffer[parser->char_pos % parser->bp_buff_len] = parser->byte_pos; + #ifdef ACMP_USE_UTF8 if (parser->is_utf8) { if (parser->u8buff_len > 0) { /* Resuming partial utf-8 sequence */ @@ -657,7 +685,9 @@ apr_status_t acmp_process(ACMP *parser, const char *data, apr_size_t len) { parser->char_pos++; } } - } else { + } else + #endif + { letter = *data++; parser->byte_pos++; parser->char_pos++; diff --git a/apache2/acmp.h b/apache2/acmp.h index 83b6f8f8..f79a44cd 100644 --- a/apache2/acmp.h +++ b/apache2/acmp.h @@ -15,9 +15,11 @@ #include #define ACMP_FLAG_BYTE 0 -#define ACMP_FLAG_UTF8 0x100 #define ACMP_FLAG_CASE_SENSITIVE 1 #define ACMP_FLAG_CASE_INSENSITIVE 0 +#ifdef ACMP_USE_UTF8 +#define ACMP_FLAG_UTF8 0x100 +#endif /** * Opaque struct with parser data diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c index 5cc286b1..d9513766 100644 --- a/apache2/mod_security2.c +++ b/apache2/mod_security2.c @@ -348,7 +348,7 @@ static modsec_rec *create_tx_context(request_rec *r) { /* Invoke the engine to continue with initialisation */ if (modsecurity_tx_init(msr) < 0) { - msr_log(msr, 1, "Failed to initialising transaction (txid %s).", msr->txid); + msr_log(msr, 1, "Failed to initialise transaction (txid %s).", msr->txid); return NULL; } diff --git a/apache2/msc_geo.c b/apache2/msc_geo.c index b28b023e..3269ba3d 100644 --- a/apache2/msc_geo.c +++ b/apache2/msc_geo.c @@ -300,34 +300,35 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro georec->dma_code = 0; georec->area_code = 0; - msr_log(msr, 9, "GEO: Looking up \"%s\".", target); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "GEO: Looking up \"%s\".", log_escape(msr->mp, target)); + } /* NOTE: This only works with ipv4 */ if ((rc = apr_sockaddr_info_get(&addr, target, APR_INET, 0, 0, msr->mp)) != APR_SUCCESS) { - *error_msg = apr_psprintf(msr->mp, "Geo lookup of \"%s\" failed: %s", target, apr_strerror(rc, errstr, 1024)); + *error_msg = apr_psprintf(msr->mp, "Geo lookup of \"%s\" failed: %s", log_escape(msr->mp, target), apr_strerror(rc, errstr, 1024)); return 0; } if ((rc = apr_sockaddr_ip_get(&targetip, addr)) != APR_SUCCESS) { - *error_msg = apr_psprintf(msr->mp, "Geo lookup of \"%s\" failed: %s", target, apr_strerror(rc, errstr, 1024)); + *error_msg = apr_psprintf(msr->mp, "Geo lookup of \"%s\" failed: %s", log_escape(msr->mp, target), apr_strerror(rc, errstr, 1024)); return 0; }; /* Why is this in host byte order? */ ipnum = ntohl(addr->sa.sin.sin_addr.s_addr); - msr_log(msr, 9, "GEO: Using address \"%s\" (0x%08lx).", targetip, ipnum); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "GEO: Using address \"%s\" (0x%08lx).", targetip, ipnum); + } for (level = 31; level >= 0; level--) { - /* Read the record */ seekto = 2 * reclen * rec_val; apr_file_seek(geo->db, APR_SET, &seekto); /* TODO: check rc */ rc = apr_file_read_full(geo->db, &buf, (2 * reclen), &nbytes); - - /* NOTE: This is hard-coded for size 3 records */ /* Left */ if ((ipnum & (1 << level)) == 0) { @@ -352,7 +353,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro country = rec_val; country -= geo->ctry_offset; if (country <= 0) { - *error_msg = apr_psprintf(msr->mp, "No geo data for \"%s\".", target); + *error_msg = apr_psprintf(msr->mp, "No geo data for \"%s\".", log_escape(msr->mp, target)); return 0; } @@ -375,13 +376,17 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro country = cbuf[0]; if (country <= 0) { - *error_msg = apr_psprintf(msr->mp, "No geo data for \"%s\".", target); + *error_msg = apr_psprintf(msr->mp, "No geo data for \"%s\".", log_escape(msr->mp, target)); return 0; } - msr_log(msr, 9, "GEO: rec=\"%s\"", log_escape_raw(msr->mp, cbuf, sizeof(cbuf))); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "GEO: rec=\"%s\"", log_escape_raw(msr->mp, cbuf, sizeof(cbuf))); + } /* Country */ - msr_log(msr, 9, "GEO: country=\"%.*s\"", (1*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "GEO: country=\"%.*s\"", (1*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))); + } georec->country_code = geo_country_code[country]; georec->country_code3 = geo_country_code3[country]; georec->country_name = geo_country_name[country]; @@ -391,27 +396,35 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro /* Region */ field_len = field_length((const char *)cbuf+rec_offset, remaining); - msr_log(msr, 9, "GEO: region=\"%.*s\"", ((field_len+1)*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "GEO: region=\"%.*s\"", ((field_len+1)*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + } georec->region = apr_pstrmemdup(msr->mp, (const char *)cbuf+rec_offset, (remaining)); rec_offset += field_len + 1; remaining -= field_len + 1; /* City */ field_len = field_length((const char *)cbuf+rec_offset, remaining); - msr_log(msr, 9, "GEO: city=\"%.*s\"", ((field_len+1)*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "GEO: city=\"%.*s\"", ((field_len+1)*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + } georec->city = apr_pstrmemdup(msr->mp, (const char *)cbuf+rec_offset, (remaining)); rec_offset += field_len + 1; remaining -= field_len + 1; /* Postal Code */ field_len = field_length((const char *)cbuf+rec_offset, remaining); - msr_log(msr, 9, "GEO: postal_code=\"%.*s\"", ((field_len+1)*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "GEO: postal_code=\"%.*s\"", ((field_len+1)*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + } georec->postal_code = apr_pstrmemdup(msr->mp, (const char *)cbuf+rec_offset, (remaining)); rec_offset += field_len + 1; remaining -= field_len + 1; /* Latitude */ - msr_log(msr, 9, "GEO: latitude=\"%.*s\"", (3*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "GEO: latitude=\"%.*s\"", (3*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + } dtmp = cbuf[rec_offset] + (cbuf[rec_offset+1] << 8) + (cbuf[rec_offset+2] << 16); @@ -421,7 +434,9 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro /* Longitude */ - msr_log(msr, 9, "GEO: longitude=\"%.*s\"", (3*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "GEO: longitude=\"%.*s\"", (3*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + } dtmp = cbuf[rec_offset] + (cbuf[rec_offset+1] << 8) + (cbuf[rec_offset+2] << 16); @@ -430,7 +445,9 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro remaining -= 3; /* dma/area codes are in city rev1 and US only */ - msr_log(msr, 9, "GEO: dma/area=\"%.*s\"", (3*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "GEO: dma/area=\"%.*s\"", (3*4), log_escape_raw(msr->mp, cbuf, sizeof(cbuf))+(rec_offset*4)); + } if (geo->dbtype == GEO_CITY_DATABASE_1 && georec->country_code[0] == 'U' && georec->country_code[1] == 'S') @@ -447,7 +464,7 @@ int geo_lookup(modsec_rec *msr, geo_rec *georec, const char *target, char **erro } - *error_msg = apr_psprintf(msr->mp, "Geo lookup of \"%s\" succeeded.", target); + *error_msg = apr_psprintf(msr->mp, "Geo lookup of \"%s\" succeeded.", log_escape(msr->mp, target)); return 1; } diff --git a/apache2/re.c b/apache2/re.c index 186218ab..e6af7111 100644 --- a/apache2/re.c +++ b/apache2/re.c @@ -1325,7 +1325,6 @@ char *msre_format_metadata(modsec_rec *msr, msre_actionset *actionset) { log_escape_ex(msr->mp, var->value, var->value_len)); } if (actionset->logdata != NULL) { - //TODO: restrict to 512 bytes /* Expand variables in the message string. */ msc_string *var = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string)); var->value = (char *)actionset->logdata; @@ -1334,6 +1333,18 @@ char *msre_format_metadata(modsec_rec *msr, msre_actionset *actionset) { logdata = apr_psprintf(msr->mp, " [data \"%s\"]", log_escape_hex(msr->mp, (unsigned char *)var->value, var->value_len)); + + /* If it is > 512 bytes, then truncate at 512 with ellipsis. + * NOTE: 512 actual data + 9 bytes of label = 521 + */ + if (strlen(logdata) > 521) { + logdata[517] = '.'; + logdata[518] = '.'; + logdata[519] = '.'; + logdata[520] = '"'; + logdata[521] = ']'; + logdata[522] = '\0'; + } } if ((actionset->severity >= 0)&&(actionset->severity <= 7)) { severity = apr_psprintf(msr->mp, " [severity \"%s\"]", @@ -1835,18 +1846,24 @@ static apr_status_t msre_rule_process_normal(msre_rule *rule, modsec_rec *msr) { /* check the cache options */ if (var->value_len < msr->txcfg->cache_trans_min) { - msr_log(msr, 9, "CACHE: Disabled - %s value length=%u, smaller than minlen=%" APR_SIZE_T_FMT, var->name, var->value_len, msr->txcfg->cache_trans_min); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "CACHE: Disabled - %s value length=%u, smaller than minlen=%" APR_SIZE_T_FMT, var->name, var->value_len, msr->txcfg->cache_trans_min); + } usecache = 0; } if ((msr->txcfg->cache_trans_max != 0) && (var->value_len > msr->txcfg->cache_trans_max)) { - msr_log(msr, 9, "CACHE: Disabled - %s value length=%u, larger than maxlen=%" APR_SIZE_T_FMT, var->name, var->value_len, msr->txcfg->cache_trans_max); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "CACHE: Disabled - %s value length=%u, larger than maxlen=%" APR_SIZE_T_FMT, var->name, var->value_len, msr->txcfg->cache_trans_max); + } usecache = 0; } /* if cache is still enabled, check the VAR for cacheablity */ if (usecache) { if (var->metadata->is_cacheable == VAR_CACHE) { - msr_log(msr, 9, "CACHE: Enabled"); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "CACHE: Enabled"); + } /* Fetch cache table for this target */ carr = (apr_table_t **)apr_hash_get(msr->tcache, var->name, APR_HASH_KEY_STRING); @@ -1868,7 +1885,9 @@ static apr_status_t msre_rule_process_normal(msre_rule *rule, modsec_rec *msr) { } else { usecache = 0; - msr_log(msr, 9, "CACHE: %s transformations are not cacheable", var->name); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "CACHE: %s transformations are not cacheable", var->name); + } } } } @@ -1945,7 +1964,9 @@ static apr_status_t msre_rule_process_normal(msre_rule *rule, modsec_rec *msr) { var->value_len = crec->val_len; } - msr_log(msr, 9, "T (%d) %s: \"%s\" [cached hits=%d]", crec->changed, crec->path, log_escape_nq_ex(mptmp, var->value, var->value_len), crec->hits); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "T (%d) %s: \"%s\" [cached hits=%d]", crec->changed, crec->path, log_escape_nq_ex(mptmp, var->value, var->value_len), crec->hits); + } #if !defined(PERFORMANCE_MEASUREMENT) if (msr->txcfg->debuglog_level >= 4) @@ -2079,7 +2100,9 @@ static apr_status_t msre_rule_process_normal(msre_rule *rule, modsec_rec *msr) { var->value_len = crec->val_len; } - msr_log(msr, 9, "T (%d) %s: \"%s\" [cached hits=%d]", crec->changed, metadata->name, log_escape_nq_ex(mptmp, var->value, var->value_len), crec->hits); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "T (%d) %s: \"%s\" [cached hits=%d]", crec->changed, metadata->name, log_escape_nq_ex(mptmp, var->value, var->value_len), crec->hits); + } continue; } } @@ -2183,10 +2206,14 @@ static apr_status_t msre_rule_process_normal(msre_rule *rule, modsec_rec *msr) { cn++; rec = (msre_cache_rec *)ctelts[ri].val; if (rec->changed) { - msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=\"%s\"", cn, msr->phase, rec->hits, rec->num, rec->path, log_escape_nq_ex(mptmp, rec->val, rec->val_len)); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=\"%s\"", cn, msr->phase, rec->hits, rec->num, rec->path, log_escape_nq_ex(mptmp, rec->val, rec->val_len)); + } } else { - msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=", cn, msr->phase, rec->hits, rec->num, rec->path); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=", cn, msr->phase, rec->hits, rec->num, rec->path); + } } } } diff --git a/apache2/re_actions.c b/apache2/re_actions.c index 25e5c6e0..00eada8d 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -69,7 +69,7 @@ msre_var *generate_single_var(modsec_rec *msr, msre_var *var, apr_array_header_t return rvar; } - /* Copy the value so that we can transform it in piece. */ + /* Copy the value so that we can transform it in place. */ rvar->value = apr_pstrndup(mptmp, rvar->value, rvar->value_len); /* Transform rvar in a loop. */ @@ -127,7 +127,7 @@ apr_table_t *generate_multi_var(modsec_rec *msr, msre_var *var, apr_array_header for (j = 0; j < tarr->nelts; j++) { rvar = (msre_var *)telts[j].val; - /* Copy the value so that we can transform it in piece. */ + /* Copy the value so that we can transform it in place. */ rvar->value = apr_pstrndup(mptmp, rvar->value, rvar->value_len); /* Transform rvar in a loop. */ @@ -230,11 +230,13 @@ int expand_macros(modsec_rec *msr, msc_string *var, msre_rule *rule, apr_pool_t part->value_len = var_generated->value_len; part->value = (char *)var_generated->value; *(msc_string **)apr_array_push(arr) = part; - msr_log(msr, 9, "Resolved macro %%{%s%s%s} to \"%s\"", - var_name, - (var_value ? "." : ""), - (var_value ? var_value : ""), - log_escape_ex(mptmp, part->value, part->value_len)); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "Resolved macro %%{%s%s%s} to \"%s\"", + var_name, + (var_value ? "." : ""), + (var_value ? var_value : ""), + log_escape_ex(mptmp, part->value, part->value_len)); + } } } else { msr_log(msr, 4, "Failed to resolve macro %%{%s%s%s}: %s", @@ -1348,8 +1350,10 @@ static apr_status_t msre_action_deprecatevar_execute(modsec_rec *msr, apr_pool_t /* Find the current value. */ var = (msc_string *)apr_table_get(target_col, var_name); if (var == NULL) { - msr_log(msr, 9, "Asked to deprecate variable \"%s.%s\", but it does not exist.", - log_escape(msr->mp, col_name), log_escape(msr->mp, var_name)); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "Asked to deprecate variable \"%s.%s\", but it does not exist.", + log_escape(msr->mp, col_name), log_escape(msr->mp, var_name)); + } return 0; } current_value = atoi(var->value); @@ -1393,10 +1397,12 @@ static apr_status_t msre_action_deprecatevar_execute(modsec_rec *msr, apr_pool_t apr_table_set(msr->collections_dirty, col_name, "1"); } else { - msr_log(msr, 9, "Not deprecating variable \"%s.%s\" because the new value (%ld) is " - "the same as the old one (%ld) (%" APR_TIME_T_FMT " seconds since last update).", - log_escape(msr->mp, col_name), log_escape(msr->mp, var_name), current_value, - new_value, (apr_time_t)(current_time - last_update_time)); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "Not deprecating variable \"%s.%s\" because the new value (%ld) is " + "the same as the old one (%ld) (%" APR_TIME_T_FMT " seconds since last update).", + log_escape(msr->mp, col_name), log_escape(msr->mp, var_name), current_value, + new_value, (apr_time_t)(current_time - last_update_time)); + } } return 1; diff --git a/apache2/re_operators.c b/apache2/re_operators.c index e9d067ec..5eacf863 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -166,11 +166,6 @@ static int msre_op_rx_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, c } } - /* - if ( ((rc == PCRE_ERROR_NOMATCH)&&(rule->op_negated == 1)) - || ((rc != PCRE_ERROR_NOMATCH)&&(rule->op_negated == 0)) ) - { - */ if (rc != PCRE_ERROR_NOMATCH) { /* Match. */ /* We no longer escape the pattern here as it is done when logging */ char *pattern = apr_pstrdup(msr->mp, regex->pattern); @@ -207,15 +202,14 @@ static int msre_op_pm_param_init(msre_rule *rule, char **error_msg) { if (p == NULL) return 0; phrase = apr_pstrdup(rule->ruleset->mp, rule->op_param); - next = rule->op_param + strlen(rule->op_param); /* Loop through phrases */ /* ENH: Need to allow quoted phrases w/space */ for (;;) { - while((isspace(*phrase) != 0) && (*phrase != '\0')) phrase++; + while((apr_isspace(*phrase) != 0) && (*phrase != '\0')) phrase++; if (*phrase == '\0') break; next = phrase; - while((isspace(*next) == 0) && (*next != 0)) next++; + while((apr_isspace(*next) == 0) && (*next != 0)) next++; acmp_add_pattern(p, phrase, NULL, NULL, next - phrase); phrase = next; } @@ -238,7 +232,7 @@ static int msre_op_pmFromFile_param_init(msre_rule *rule, char **error_msg) { ACMP *p; if ((rule->op_param == NULL)||(strlen(rule->op_param) == 0)) { - *error_msg = apr_psprintf(rule->ruleset->mp, "Missing parameter for operator 'pm'."); + *error_msg = apr_psprintf(rule->ruleset->mp, "Missing parameter for operator 'pmFromFile'."); return 0; /* ERROR */ } @@ -246,7 +240,6 @@ static int msre_op_pmFromFile_param_init(msre_rule *rule, char **error_msg) { if (p == NULL) return 0; fn = apr_pstrdup(rule->ruleset->mp, rule->op_param); - next = fn + strlen(rule->op_param); /* Get the path of the rule filename to use as a base */ rulefile_path = apr_pstrndup(rule->ruleset->mp, rule->filename, strlen(rule->filename) - strlen(apr_filepath_name_get(rule->filename))); @@ -263,11 +256,11 @@ static int msre_op_pmFromFile_param_init(msre_rule *rule, char **error_msg) { int line = 0; /* Trim whitespace */ - while((isspace(*fn) != 0) && (*fn != '\0')) fn++; + while((apr_isspace(*fn) != 0) && (*fn != '\0')) fn++; if (*fn == '\0') break; next = fn; - while((isspace(*next) == 0) && (*next != '\0')) next++; - while((isspace(*next) != 0) && (*next != '\0')) *(next++) = '\0'; + while((apr_isspace(*next) == 0) && (*next != '\0')) next++; + while((apr_isspace(*next) != 0) && (*next != '\0')) *(next++) = '\0'; /* Add path of the rule filename for a relative phrase filename */ filepath = fn; @@ -433,7 +426,6 @@ static int msre_op_within_execute(modsec_rec *msr, msre_rule *rule, msre_var *va /* scan for first character, then compare from there until we * have a match or there is no room left in the target */ - msr_log(msr, 9, "match[%u]='%s' target[%u]='%s'", match_length, match, target_length, target); i_max = match_length - target_length; for (i = 0; i <= i_max; i++) { if (match[i] == target[0]) { @@ -507,8 +499,12 @@ static int msre_op_contains_execute(modsec_rec *msr, msre_rule *rule, msre_var * */ i_max = target_length - match_length; for (i = 0; i <= i_max; i++) { + /* First character matched - avoid func call */ if (target[i] == match[0]) { - if (memcmp((match + 1), (target + i + 1), (match_length - 1)) == 0) { + /* See if remaining matches */ + if ( (match_length == 1) + || (memcmp((match + 1), (target + i + 1), (match_length - 1)) == 0)) + { /* Match. */ *error_msg = apr_psprintf(msr->mp, "String match \"%s\" at %s.", log_escape_ex(msr->mp, match, match_length), @@ -531,6 +527,7 @@ static int msre_op_containsWord_execute(modsec_rec *msr, msre_rule *rule, msre_v unsigned int match_length; unsigned int target_length = 0; unsigned int i, i_max; + int rc = 0; str->value = (char *)rule->op_param; str->value_len = strlen(str->value); @@ -580,42 +577,36 @@ static int msre_op_containsWord_execute(modsec_rec *msr, msre_rule *rule, msre_v for (i = 0; i <= i_max; i++) { /* Previous char must have been a start or non-word */ - if ((i > 0) && (isalnum(target[i-1])||(target[i-1] == '_'))) + if ((i > 0) && (apr_isalnum(target[i-1])||(target[i-1] == '_'))) continue; - - /* First character matched */ + /* First character matched - avoid func call */ if (target[i] == match[0]) { - - /* Maybe a match. */ - *error_msg = apr_psprintf(msr->mp, "String match \"%s\" at %s.", - log_escape_ex(msr->mp, match, match_length), - var->name); - - /* only one character */ - if (match_length == 1) return 1; - - /* remaining matched */ - if (memcmp((match + 1), (target + i + 1), (match_length - 1)) == 0) { - - + /* See if remaining matches */ + if ( (match_length == 1) + || (memcmp((match + 1), (target + i + 1), (match_length - 1)) == 0)) + { /* check boundaries */ if (i == i_max) { /* exact/end word match */ - return 1; + rc = 1; } - else if (!(isalnum(target[i+match_length])||(target[i+match_length] == '_'))) { + else if (!(apr_isalnum(target[i + match_length])||(target[i + match_length] == '_'))) { /* start/mid word match */ - return 1; + rc = 1; } - - /* No word match */ - *error_msg = NULL; - return 0; } } } + if (rc == 1) { + /* Maybe a match. */ + *error_msg = apr_psprintf(msr->mp, "String match \"%s\" at %s.", + log_escape_ex(msr->mp, match, match_length), + var->name); + return 1; + } + /* No match. */ *error_msg = NULL; return 0; @@ -987,7 +978,7 @@ static int luhn_verify(const char *ccnumber, int len) { * for both odd and even CC numbers to avoid 2 passes. */ for (i = 0; i < len; i++) { - if (isdigit(ccnumber[i])) { + if (apr_isdigit(ccnumber[i])) { sum[0] += (!odd ? wtable[ccnumber[i] - '0'] : (ccnumber[i] - '0')); sum[1] += (odd ? wtable[ccnumber[i] - '0'] : (ccnumber[i] - '0')); odd = 1 - odd; /* alternate weights */ @@ -1160,7 +1151,7 @@ static int msre_op_geoLookup_execute(modsec_rec *msr, msre_rule *rule, msre_var *error_msg = NULL; if (geo == NULL) { - msr_log(msr, 1, "Geo lookup for \"%s\" attempted without a database. Set SecGeoLookupDB.", log_escape_nq(msr->mp, geo_host)); + msr_log(msr, 1, "Geo lookup for \"%s\" attempted without a database. Set SecGeoLookupDB.", log_escape(msr->mp, geo_host)); return 0; } diff --git a/apache2/t/op/contains.t b/apache2/t/op/contains.t index d8a8641d..0b406b48 100644 --- a/apache2/t/op/contains.t +++ b/apache2/t/op/contains.t @@ -50,3 +50,24 @@ input => "abcdefghi", ret => 0, }, +{ + type => "op", + name => "contains", + param => "x", + input => "x", + ret => 1, +}, +{ + type => "op", + name => "contains", + param => "y", + input => "xyz", + ret => 1, +}, +{ + type => "op", + name => "contains", + param => "hiding", + input => "hidinX<-not quite, but is later on->hiding", + ret => 1, +}, diff --git a/apache2/t/op/containsWord.t b/apache2/t/op/containsWord.t index 6621e76f..5b66bb5a 100644 --- a/apache2/t/op/containsWord.t +++ b/apache2/t/op/containsWord.t @@ -78,3 +78,31 @@ input => "abc\0def ghi", ret => 1, }, +{ + type => "op", + name => "containsWord", + param => "x", + input => "x", + ret => 1, +}, +{ + type => "op", + name => "containsWord", + param => "x", + input => " x ", + ret => 1, +}, +{ + type => "op", + name => "containsWord", + param => "y", + input => "xyz", + ret => 0, +}, +{ + type => "op", + name => "containsWord", + param => "hiding", + input => "hidingX<-not on word boundary, but is later on->hiding", + ret => 1, +},