From 608cd1d09dc68d8e20cd520206b836676be919a1 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Thu, 26 Oct 2023 14:21:32 +0200 Subject: [PATCH 1/3] Avoid last loop and storing an empty value in case nothing after last %{..} macro --- apache2/re_actions.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apache2/re_actions.c b/apache2/re_actions.c index 02ec07d2..6bd86655 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -279,7 +279,7 @@ int expand_macros(modsec_rec *msr, msc_string *var, msre_rule *rule, apr_pool_t part->value_len = strlen(part->value); *(msc_string **)apr_array_push(arr) = part; } - } while (p != NULL); + } while (p != NULL && *next_text_start); /* If there's more than one member of the array that * means there was at least one macro present. Combine From 31dae62f4145604da0bc94b2beed0af3dce04218 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Thu, 26 Oct 2023 15:00:10 +0200 Subject: [PATCH 2/3] Don't store empty string before macro and empty macro result --- apache2/re_actions.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apache2/re_actions.c b/apache2/re_actions.c index 6bd86655..d3e60c97 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -228,18 +228,20 @@ int expand_macros(modsec_rec *msr, msc_string *var, msre_rule *rule, apr_pool_t msre_var *var_resolved = NULL; /* Add the text part before the macro to the array. */ + if (p != text_start) { part = (msc_string *)apr_pcalloc(mptmp, sizeof(msc_string)); if (part == NULL) return -1; part->value_len = p - text_start; part->value = apr_pstrmemdup(mptmp, text_start, part->value_len); *(msc_string **)apr_array_push(arr) = part; + } /* Resolve the macro and add that to the array. */ 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, NULL, rule, mptmp); - if (var_generated != NULL) { + if (var_generated != NULL && var_generated->value_len) { part = (msc_string *)apr_pcalloc(mptmp, sizeof(msc_string)); if (part == NULL) return -1; part->value_len = var_generated->value_len; From 029fdedc67ecc788126381d69f2d1186f1a406b4 Mon Sep 17 00:00:00 2001 From: Marc Stern Date: Thu, 26 Oct 2023 15:55:57 +0200 Subject: [PATCH 3/3] useless (and now incorrect) check --- apache2/re_actions.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/apache2/re_actions.c b/apache2/re_actions.c index d3e60c97..8818f0b3 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -283,11 +283,9 @@ int expand_macros(modsec_rec *msr, msc_string *var, msre_rule *rule, apr_pool_t } } while (p != NULL && *next_text_start); - /* If there's more than one member of the array that - * means there was at least one macro present. Combine - * text parts into a single string now. + /* Combine text parts into a single string now. + * If no macro was present, we already returned */ - if (arr->nelts > 1) { /* Figure out the required size for the string. */ var->value_len = 0; for(i = 0; i < arr->nelts; i++) { @@ -307,7 +305,6 @@ int expand_macros(modsec_rec *msr, msc_string *var, msre_rule *rule, apr_pool_t offset += part->value_len; } var->value[offset] = '\0'; - } return 1; }