From ab55a8716e03dbb8ecc2e7987d969b34899fcf2b Mon Sep 17 00:00:00 2001 From: brectanus Date: Thu, 8 Mar 2007 16:15:45 +0000 Subject: [PATCH] Fix potential memory corruption in msre_create_var_ex allocating per-request data out of global pool. --- CHANGES | 4 +++- apache2/re.c | 11 +++++++---- apache2/re.h | 2 +- apache2/re_actions.c | 2 +- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/CHANGES b/CHANGES index 89248f73..7c101104 100644 --- a/CHANGES +++ b/CHANGES @@ -1,7 +1,9 @@ -01 Mar 2007 - 2.1.1-dev1 +01 Mar 2007 - 2.1.1-dev3 ------------------------ +* Fixed potential memory corruption when expanding macros. + * Fixed error when a collection var was fetched in the same second as creation by setting the rate to zero. diff --git a/apache2/re.c b/apache2/re.c index 8b5b7cc0..0dc5ada3 100644 --- a/apache2/re.c +++ b/apache2/re.c @@ -121,10 +121,10 @@ msre_action_metadata *msre_resolve_action(msre_engine *engine, const char *name) * Creates a new variable instance given the variable name * and an (optional) parameter. */ -msre_var *msre_create_var_ex(msre_engine *engine, const char *name, const char *param, +msre_var *msre_create_var_ex(apr_pool_t *pool, msre_engine *engine, const char *name, const char *param, modsec_rec *msr, char **error_msg) { - msre_var *var = apr_pcalloc(engine->mp, sizeof(msre_var)); + msre_var *var = apr_pcalloc(pool, sizeof(msre_var)); if (var == NULL) return NULL; if (error_msg == NULL) return NULL; @@ -147,7 +147,7 @@ msre_var *msre_create_var_ex(msre_engine *engine, const char *name, const char * /* CGI HTTP variables emulation. */ if (strncasecmp(var->name, "HTTP_", 5) == 0) { if (var->param != NULL) { - *error_msg = apr_psprintf(engine->mp, "Variable %s does not support parameters.", + *error_msg = apr_psprintf(pool, "Variable %s does not support parameters.", var->name); return NULL; } @@ -196,11 +196,14 @@ msre_var *msre_create_var_ex(msre_engine *engine, const char *name, const char * /** * Create a new variable object from the provided name and value. + * + * NOTE: this allocates out of the global pool and should not be used + * per-request */ msre_var *msre_create_var(msre_ruleset *ruleset, const char *name, const char *param, modsec_rec *msr, char **error_msg) { - msre_var *var = msre_create_var_ex(ruleset->engine, name, param, msr, error_msg); + msre_var *var = msre_create_var_ex(ruleset->engine->mp, ruleset->engine, name, param, msr, error_msg); if (var == NULL) return NULL; /* Validate & initialise variable */ diff --git a/apache2/re.h b/apache2/re.h index 9a9ac660..896e2a11 100644 --- a/apache2/re.h +++ b/apache2/re.h @@ -54,7 +54,7 @@ msre_action_metadata *msre_resolve_action(msre_engine *engine, const char *name) msre_var *msre_create_var(msre_ruleset *ruleset, const char *name, const char *param, modsec_rec *msr, char **error_msg); -msre_var *msre_create_var_ex(msre_engine *engine, const char *name, const char *param, +msre_var *msre_create_var_ex(apr_pool_t *pool, msre_engine *engine, const char *name, const char *param, modsec_rec *msr, char **error_msg); msre_action *msre_create_action(msre_engine *engine, const char *name, diff --git a/apache2/re_actions.c b/apache2/re_actions.c index ed71a0c7..58c47b5b 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -131,7 +131,7 @@ int DSOLOCAL expand_macros(modsec_rec *msr, msc_string *var, msre_rule *rule, ap *(msc_string **)apr_array_push(arr) = part; /* Resolve the macro and add that to the array. */ - var_resolved = msre_create_var_ex(msr->modsecurity->msre, var_name, var_value, + var_resolved = msre_create_var_ex(mptmp, msr->modsecurity->msre, var_name, var_value, msr, &my_error_msg); if (var_resolved != NULL) { var_generated = generate_single_var(msr, var_resolved, rule, mptmp);