From 97e328fbc72ebfa14ce70f112bd12b983d20c474 Mon Sep 17 00:00:00 2001 From: Ervin Hegedus Date: Sun, 1 Nov 2026 21:49:26 +0100 Subject: [PATCH 1/5] fix: expand %{RULE.*} in SecAction and SecRuleScript rules RunTimeString::evaluate() cast the rule to RuleWithOperator before resolving the variables in a macro. SecAction (RuleUnconditional) and SecRuleScript (RuleScript) derive from RuleWithActions, not from RuleWithOperator, so the cast returned NULL, and %{RULE.id} and the other RULE fields expanded to an empty string in their actions. Cast to RuleWithActions, which is what Variable::evaluate() takes. In SetVar::evaluate(), the rule already is a RuleWithActions, so pass it on without a cast. Add regression cases for %{RULE.id} and %{RULE.msg} in a SecAction, %{RULE.id} in a SecRuleScript rule, and an =+ setvar with %{RULE.id} in a SecAction. Fixes #3733 --- src/actions/set_var.cc | 3 +- src/run_time_string.cc | 3 +- test/test-cases/regression/variable-RULE.json | 200 +++++++++++++++++- 3 files changed, 195 insertions(+), 11 deletions(-) diff --git a/src/actions/set_var.cc b/src/actions/set_var.cc index 70c07d065d..ad1eb22725 100644 --- a/src/actions/set_var.cc +++ b/src/actions/set_var.cc @@ -101,8 +101,7 @@ bool SetVar::evaluate(RuleWithActions *rule, Transaction *t) { try { std::vector l; - RuleWithOperator *rr = dynamic_cast(rule); - m_variable->evaluate(t, rr, &l); + m_variable->evaluate(t, rule, &l); if (l.size() == 0) { value = 0; } else { diff --git a/src/run_time_string.cc b/src/run_time_string.cc index 45f298bc3d..9dac8a92a8 100644 --- a/src/run_time_string.cc +++ b/src/run_time_string.cc @@ -58,8 +58,7 @@ std::string RunTimeString::evaluate(Transaction *t, Rule *r) { s.append(z->m_string); } else if (z->m_var != NULL && t != NULL) { std::vector l; - // FIXME: This cast should be removed. - RuleWithOperator *rr = dynamic_cast(r); + RuleWithActions *rr = dynamic_cast(r); z->m_var->evaluate(t, rr, &l); if (l.size() > 0) { s.append(l[0]->getValue()); diff --git a/test/test-cases/regression/variable-RULE.json b/test/test-cases/regression/variable-RULE.json index f622a7cecb..559fe9083e 100644 --- a/test/test-cases/regression/variable-RULE.json +++ b/test/test-cases/regression/variable-RULE.json @@ -2,7 +2,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (1/7)", + "title": "Testing Variables :: RULE (1/11)", "client": { "ip": "200.249.12.31", "port": 123 @@ -48,7 +48,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (2/7)", + "title": "Testing Variables :: RULE (2/11)", "client": { "ip": "200.249.12.31", "port": 123 @@ -94,7 +94,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (3/7)", + "title": "Testing Variables :: RULE (3/11)", "client": { "ip": "200.249.12.31", "port": 123 @@ -140,7 +140,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (4/7)", + "title": "Testing Variables :: RULE (4/11)", "client": { "ip": "200.249.12.31", "port": 123 @@ -186,7 +186,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (5/7)", + "title": "Testing Variables :: RULE (5/11)", "client": { "ip": "200.249.12.31", "port": 123 @@ -232,7 +232,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (6/7)", + "title": "Testing Variables :: RULE (6/11)", "client": { "ip": "200.249.12.31", "port": 123 @@ -278,7 +278,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (7/7)", + "title": "Testing Variables :: RULE (7/11)", "client": { "ip": "200.249.12.31", "port": 123 @@ -320,5 +320,191 @@ "SecRuleEngine On", "SecRule rule:msg \"@contains message\" \"id:1,msg:'message123',setvar:'ip.block_reason=%{RULE.msg}%',phase:3,pass,t:trim\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Variables :: RULE (8/11)", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "27", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=value1¶m2=value2" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Saving variable: TX:rule_id with value: 108", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:108,phase:3,pass,setvar:tx.rule_id=%{RULE.id}\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Variables :: RULE (9/11)", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "27", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=value1¶m2=value2" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Saving variable: TX:rule_msg with value: message123", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:109,msg:'message123',phase:3,pass,setvar:tx.rule_msg=%{RULE.msg}\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Variables :: RULE (10/11)", + "resource": "lua", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "27", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=value1¶m2=value2" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Saving variable: TX:rule_id with value: 110.*Called from rule 110\\.", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRuleScript test-cases/data/lua-getvar-tx-rule-id.lua \"id:110,phase:3,pass,setvar:tx.rule_id=%{RULE.id}\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Variables :: RULE (11/11)", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "27", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=value1¶m2=value2" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Saving variable: TX:score with value: 117", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:111,phase:3,pass,setvar:tx.score=5\"", + "SecAction \"id:112,phase:3,pass,setvar:tx.score=+%{RULE.id}\"" + ] } ] From 2f7b59de4408922a4bc59951214952e4896c1190 Mon Sep 17 00:00:00 2001 From: Ervin Hegedus Date: Sun, 1 Nov 2026 22:02:22 +0100 Subject: [PATCH 2/5] fix: expand %{RULE.*} in dynamic collection keys when reading them The dynamic collection elements (TX, IP, GLOBAL, RESOURCE, SESSION, USER) expanded their key with m_string->evaluate(t), without the rule, so a %{RULE.*} macro in the key expanded to an empty string when the variable was read. SetVar::evaluate() expands the key with the rule when it writes, so an =+ or =- setvar on a key such as tx.score_%{RULE.id} read tx.score_, got 0, and overwrote tx.score_ with the increment alone. This happened in SecRule rules before the previous commit, and in SecAction and SecRuleScript rules after it. Pass the rule to m_string->evaluate() in all six. Add regression cases for an =+ setvar on a tx.score_%{RULE.id} key in a SecAction and in a SecRule. Refs #3733 --- src/variables/global.h | 2 +- src/variables/ip.h | 2 +- src/variables/resource.h | 2 +- src/variables/session.h | 2 +- src/variables/tx.h | 2 +- src/variables/user.h | 2 +- test/test-cases/regression/variable-RULE.json | 114 ++++++++++++++++-- 7 files changed, 109 insertions(+), 17 deletions(-) diff --git a/src/variables/global.h b/src/variables/global.h index 8c7d393605..81246577c2 100644 --- a/src/variables/global.h +++ b/src/variables/global.h @@ -92,7 +92,7 @@ class Global_DynamicElement : public Variable { void evaluate(Transaction *t, RuleWithActions *rule, std::vector *l) override { - std::string string = m_string->evaluate(t); + std::string string = m_string->evaluate(t, rule); t->m_collections.m_global_collection->resolveMultiMatches( string, t->m_collections.m_global_collection_key, diff --git a/src/variables/ip.h b/src/variables/ip.h index a32dbecd93..93fdda4222 100644 --- a/src/variables/ip.h +++ b/src/variables/ip.h @@ -91,7 +91,7 @@ class Ip_DynamicElement : public Variable { void evaluate(Transaction *t, RuleWithActions *rule, std::vector *l) override { - std::string string = m_string->evaluate(t); + std::string string = m_string->evaluate(t, rule); t->m_collections.m_ip_collection->resolveMultiMatches( string, t->m_collections.m_ip_collection_key, diff --git a/src/variables/resource.h b/src/variables/resource.h index a198ce7624..033807d319 100644 --- a/src/variables/resource.h +++ b/src/variables/resource.h @@ -91,7 +91,7 @@ class Resource_DynamicElement : public Variable { void evaluate(Transaction *t, RuleWithActions *rule, std::vector *l) override { - std::string string = m_string->evaluate(t); + std::string string = m_string->evaluate(t, rule); t->m_collections.m_resource_collection->resolveMultiMatches( string, t->m_collections.m_resource_collection_key, diff --git a/src/variables/session.h b/src/variables/session.h index 02efbe66db..6dc0a82aae 100644 --- a/src/variables/session.h +++ b/src/variables/session.h @@ -91,7 +91,7 @@ class Session_DynamicElement : public Variable { void evaluate(Transaction *t, RuleWithActions *rule, std::vector *l) override { - std::string string = m_string->evaluate(t); + std::string string = m_string->evaluate(t, rule); t->m_collections.m_session_collection->resolveMultiMatches( string, t->m_collections.m_session_collection_key, diff --git a/src/variables/tx.h b/src/variables/tx.h index 1fae827f13..40d02e6849 100644 --- a/src/variables/tx.h +++ b/src/variables/tx.h @@ -89,7 +89,7 @@ class Tx_DynamicElement : public Variable { void evaluate(Transaction *t, RuleWithActions *rule, std::vector *l) override { - std::string string = m_string->evaluate(t); + std::string string = m_string->evaluate(t, rule); t->m_collections.m_tx_collection->resolveMultiMatches(string, l, m_keyExclusion); } diff --git a/src/variables/user.h b/src/variables/user.h index a0a46533eb..1edcbc9dc0 100644 --- a/src/variables/user.h +++ b/src/variables/user.h @@ -91,7 +91,7 @@ class User_DynamicElement : public Variable { void evaluate(Transaction *t, RuleWithActions *rule, std::vector *l) override { - std::string string = m_string->evaluate(t); + std::string string = m_string->evaluate(t, rule); t->m_collections.m_user_collection->resolveMultiMatches( string, t->m_collections.m_user_collection_key, diff --git a/test/test-cases/regression/variable-RULE.json b/test/test-cases/regression/variable-RULE.json index 559fe9083e..047a8f4026 100644 --- a/test/test-cases/regression/variable-RULE.json +++ b/test/test-cases/regression/variable-RULE.json @@ -2,7 +2,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (1/11)", + "title": "Testing Variables :: RULE (1/13)", "client": { "ip": "200.249.12.31", "port": 123 @@ -48,7 +48,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (2/11)", + "title": "Testing Variables :: RULE (2/13)", "client": { "ip": "200.249.12.31", "port": 123 @@ -94,7 +94,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (3/11)", + "title": "Testing Variables :: RULE (3/13)", "client": { "ip": "200.249.12.31", "port": 123 @@ -140,7 +140,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (4/11)", + "title": "Testing Variables :: RULE (4/13)", "client": { "ip": "200.249.12.31", "port": 123 @@ -186,7 +186,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (5/11)", + "title": "Testing Variables :: RULE (5/13)", "client": { "ip": "200.249.12.31", "port": 123 @@ -232,7 +232,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (6/11)", + "title": "Testing Variables :: RULE (6/13)", "client": { "ip": "200.249.12.31", "port": 123 @@ -278,7 +278,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (7/11)", + "title": "Testing Variables :: RULE (7/13)", "client": { "ip": "200.249.12.31", "port": 123 @@ -324,7 +324,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (8/11)", + "title": "Testing Variables :: RULE (8/13)", "client": { "ip": "200.249.12.31", "port": 123 @@ -370,7 +370,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (9/11)", + "title": "Testing Variables :: RULE (9/13)", "client": { "ip": "200.249.12.31", "port": 123 @@ -416,7 +416,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (10/11)", + "title": "Testing Variables :: RULE (10/13)", "resource": "lua", "client": { "ip": "200.249.12.31", @@ -463,7 +463,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (11/11)", + "title": "Testing Variables :: RULE (11/13)", "client": { "ip": "200.249.12.31", "port": 123 @@ -506,5 +506,97 @@ "SecAction \"id:111,phase:3,pass,setvar:tx.score=5\"", "SecAction \"id:112,phase:3,pass,setvar:tx.score=+%{RULE.id}\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Variables :: RULE (12/13)", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "27", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=value1¶m2=value2" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Saving variable: TX:score_113 with value: 6", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:113,phase:3,pass,setvar:tx.score_%{RULE.id}=5,setvar:tx.score_%{RULE.id}=+1\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Variables :: RULE (13/13)", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "27", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=value1¶m2=value2" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Saving variable: TX:score_114 with value: 6", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_METHOD \"@streq POST\" \"id:114,phase:3,pass,setvar:tx.score_%{RULE.id}=5,setvar:tx.score_%{RULE.id}=+1\"" + ] } ] From a0451bdec09c9492304df78b5e21c4e26a2ca5b1 Mon Sep 17 00:00:00 2001 From: Ervin Hegedus Date: Sun, 1 Nov 2026 22:07:56 +0100 Subject: [PATCH 3/5] Apply SonarCloud changes and make variable type to auto. --- src/run_time_string.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/run_time_string.cc b/src/run_time_string.cc index 9dac8a92a8..d66cc7b405 100644 --- a/src/run_time_string.cc +++ b/src/run_time_string.cc @@ -58,7 +58,7 @@ std::string RunTimeString::evaluate(Transaction *t, Rule *r) { s.append(z->m_string); } else if (z->m_var != NULL && t != NULL) { std::vector l; - RuleWithActions *rr = dynamic_cast(r); + auto *rr = dynamic_cast(r); z->m_var->evaluate(t, rr, &l); if (l.size() > 0) { s.append(l[0]->getValue()); From 6ce25b82c8b69d6afd1afa6d71325027b35dd048 Mon Sep 17 00:00:00 2001 From: Ervin Hegedus Date: Sun, 1 Nov 2026 22:21:26 +0100 Subject: [PATCH 4/5] fix: expand %{RULE.*} in msg and logdata Msg::evaluate() and LogData::evaluate() get the rule, but expanded their string with data(), which doesn't pass it on, so %{RULE.id} and the other RULE fields expanded to an empty string in msg and logdata, in every kind of rule. Expand the string with the rule instead. data() stays as it is. RuleWithActions::msg() and logData() use it to resolve %{RULE.msg} and %{RULE.logdata}, and passing the rule there too would make a msg that contains %{RULE.msg} recurse. Add regression cases for %{RULE.id} in the msg of a SecRule and in the logdata of a SecAction. Refs #3733 --- src/actions/log_data.cc | 3 +- src/actions/msg.cc | 3 +- test/test-cases/regression/variable-RULE.json | 118 ++++++++++++++++-- 3 files changed, 109 insertions(+), 15 deletions(-) diff --git a/src/actions/log_data.cc b/src/actions/log_data.cc index 596505868f..81cce50ec5 100644 --- a/src/actions/log_data.cc +++ b/src/actions/log_data.cc @@ -22,6 +22,7 @@ #include "modsecurity/actions/action.h" #include "modsecurity/transaction.h" #include "modsecurity/rule.h" +#include "modsecurity/rule_with_actions.h" #include "modsecurity/rule_message.h" @@ -30,7 +31,7 @@ namespace actions { bool LogData::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) { - ruleMessage.m_data = data(transaction); + ruleMessage.m_data = m_string->evaluate(transaction, rule); return true; } diff --git a/src/actions/msg.cc b/src/actions/msg.cc index fecab53858..467b38f15a 100644 --- a/src/actions/msg.cc +++ b/src/actions/msg.cc @@ -22,6 +22,7 @@ #include "modsecurity/actions/action.h" #include "modsecurity/transaction.h" #include "modsecurity/rule.h" +#include "modsecurity/rule_with_actions.h" #include "modsecurity/rule_message.h" /* @@ -47,7 +48,7 @@ namespace actions { bool Msg::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) { - const auto msg = data(transaction); + const auto msg = m_string->evaluate(transaction, rule); ruleMessage.m_message = msg; ms_dbg_a(transaction, 9, "Saving msg: " + msg); diff --git a/test/test-cases/regression/variable-RULE.json b/test/test-cases/regression/variable-RULE.json index 047a8f4026..c0fea64850 100644 --- a/test/test-cases/regression/variable-RULE.json +++ b/test/test-cases/regression/variable-RULE.json @@ -2,7 +2,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (1/13)", + "title": "Testing Variables :: RULE (1/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -48,7 +48,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (2/13)", + "title": "Testing Variables :: RULE (2/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -94,7 +94,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (3/13)", + "title": "Testing Variables :: RULE (3/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -140,7 +140,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (4/13)", + "title": "Testing Variables :: RULE (4/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -186,7 +186,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (5/13)", + "title": "Testing Variables :: RULE (5/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -232,7 +232,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (6/13)", + "title": "Testing Variables :: RULE (6/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -278,7 +278,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (7/13)", + "title": "Testing Variables :: RULE (7/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -324,7 +324,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (8/13)", + "title": "Testing Variables :: RULE (8/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -370,7 +370,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (9/13)", + "title": "Testing Variables :: RULE (9/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -416,7 +416,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (10/13)", + "title": "Testing Variables :: RULE (10/15)", "resource": "lua", "client": { "ip": "200.249.12.31", @@ -463,7 +463,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (11/13)", + "title": "Testing Variables :: RULE (11/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -510,7 +510,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (12/13)", + "title": "Testing Variables :: RULE (12/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -556,7 +556,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (13/13)", + "title": "Testing Variables :: RULE (13/15)", "client": { "ip": "200.249.12.31", "port": 123 @@ -598,5 +598,97 @@ "SecRuleEngine On", "SecRule REQUEST_METHOD \"@streq POST\" \"id:114,phase:3,pass,setvar:tx.score_%{RULE.id}=5,setvar:tx.score_%{RULE.id}=+1\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Variables :: RULE (14/15)", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "27", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=value1¶m2=value2" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Saving msg: id 115", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_METHOD \"@streq POST\" \"id:115,phase:3,pass,msg:'id %{RULE.id}'\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Variables :: RULE (15/15)", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "27", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=value1¶m2=value2" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "error_log": "data \"id 116\"", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:116,phase:3,pass,log,msg:'logdata test',logdata:'id %{RULE.id}'\"" + ] } ] From 6d305994f02ef8539bd56af58aaeec575d729b08 Mon Sep 17 00:00:00 2001 From: Ervin Hegedus Date: Mon, 2 Nov 2026 22:16:49 +0100 Subject: [PATCH 5/5] ifix: expand %{RULE.*} in expirevar with the rule ExpireVar::evaluate() expanded its expression with m_string->evaluate(t), without the rule, while setvar expands with the rule. So in SecAction "id:200,...,setvar:ip.score_%{RULE.id}=5,expirevar:ip.score_%{RULE.id}=60" setvar wrote IP:score_200, but expirevar set the expiry on an empty 'score_' entry, and score_200 never expired. In a SecRule the mismatch already existed; expanding %{RULE.*} in SecAction rules made it reachable there too. Add case 16 to variable-RULE.json for the example above. --- src/actions/expire_var.cc | 2 +- test/test-cases/regression/variable-RULE.json | 72 +++++++++++++++---- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/src/actions/expire_var.cc b/src/actions/expire_var.cc index ffc89f4779..2ae38d4637 100644 --- a/src/actions/expire_var.cc +++ b/src/actions/expire_var.cc @@ -34,7 +34,7 @@ namespace actions { bool ExpireVar::evaluate(RuleWithActions *rule, Transaction *t) { - std::string expireExpressionExpanded(m_string->evaluate(t)); + std::string expireExpressionExpanded(m_string->evaluate(t, rule)); std::string fully_qualified_var; int expirySeconds = 0; diff --git a/test/test-cases/regression/variable-RULE.json b/test/test-cases/regression/variable-RULE.json index c0fea64850..ffac51d510 100644 --- a/test/test-cases/regression/variable-RULE.json +++ b/test/test-cases/regression/variable-RULE.json @@ -2,7 +2,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (1/15)", + "title": "Testing Variables :: RULE (1/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -48,7 +48,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (2/15)", + "title": "Testing Variables :: RULE (2/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -94,7 +94,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (3/15)", + "title": "Testing Variables :: RULE (3/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -140,7 +140,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (4/15)", + "title": "Testing Variables :: RULE (4/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -186,7 +186,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (5/15)", + "title": "Testing Variables :: RULE (5/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -232,7 +232,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (6/15)", + "title": "Testing Variables :: RULE (6/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -278,7 +278,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (7/15)", + "title": "Testing Variables :: RULE (7/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -324,7 +324,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (8/15)", + "title": "Testing Variables :: RULE (8/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -370,7 +370,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (9/15)", + "title": "Testing Variables :: RULE (9/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -416,7 +416,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (10/15)", + "title": "Testing Variables :: RULE (10/16)", "resource": "lua", "client": { "ip": "200.249.12.31", @@ -463,7 +463,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (11/15)", + "title": "Testing Variables :: RULE (11/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -510,7 +510,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (12/15)", + "title": "Testing Variables :: RULE (12/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -556,7 +556,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (13/15)", + "title": "Testing Variables :: RULE (13/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -602,7 +602,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (14/15)", + "title": "Testing Variables :: RULE (14/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -648,7 +648,7 @@ { "enabled": 1, "version_min": 300000, - "title": "Testing Variables :: RULE (15/15)", + "title": "Testing Variables :: RULE (15/16)", "client": { "ip": "200.249.12.31", "port": 123 @@ -690,5 +690,47 @@ "SecRuleEngine On", "SecAction \"id:116,phase:3,pass,log,msg:'logdata test',logdata:'id %{RULE.id}'\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Variables :: RULE (16/16)", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*" + }, + "uri": "/", + "method": "GET" + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Setting variable `score_200' to expire in 60 seconds", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecAction \"id:1,phase:1,pass,nolog,initcol:ip=%{REMOTE_ADDR}\"", + "SecAction \"id:200,phase:1,pass,nolog,setvar:ip.score_%{RULE.id}=5,expirevar:ip.score_%{RULE.id}=60\"" + ] } ]