Skip to content

Commit c34ec48

Browse files
feat: ctl:ruleRemoveTargetById regex support (maintainer's approach)
- Compile regex at config load, not per-request - RuleRemoveTargetByIdEntry struct: literal or shared_ptr<Regex> - Test 2 (ARGS:/mixpanel$/) passes; Test 1 blocked by parser owasp-modsecurity#2927 Made-with: Cursor
1 parent bc95a9b commit c34ec48

File tree

7 files changed

+112
-13
lines changed

7 files changed

+112
-13
lines changed
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* ModSecurity, http://www.modsecurity.org/
3+
* Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
4+
*
5+
* You may not use this file except in compliance with
6+
* the License. You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* If any of the files related to licensing are missing or if you have any
11+
* other questions related to licensing please contact Trustwave Holdings, Inc.
12+
* directly using the email address security@modsecurity.org.
13+
*
14+
*/
15+
16+
#ifndef HEADERS_MODSECURITY_RULE_REMOVE_TARGET_ENTRY_H_
17+
#define HEADERS_MODSECURITY_RULE_REMOVE_TARGET_ENTRY_H_
18+
19+
#include <memory>
20+
#include <string>
21+
22+
#include "src/utils/regex.h"
23+
24+
namespace modsecurity {
25+
26+
/**
27+
* Entry for ctl:ruleRemoveTargetById exclusion.
28+
* Supports literal target (e.g. ARGS:pwd) or regex (e.g. ARGS:/^json\.\d+\.JobDescription$/).
29+
* Regex is compiled at config load (maintainer's approach).
30+
*/
31+
struct RuleRemoveTargetByIdEntry {
32+
int id;
33+
std::string literal;
34+
std::shared_ptr<Utils::Regex> regex; // shared: same compiled regex reused per request
35+
36+
/**
37+
* Match VariableValue. For regex: match against key (dict element).
38+
* For literal: match against keyWithCollection (e.g. ARGS:mixpanel).
39+
*/
40+
bool matches(const std::string &key, const std::string &keyWithCollection) const {
41+
if (regex) {
42+
return regex->searchAll(key).size() > 0;
43+
}
44+
return literal == keyWithCollection;
45+
}
46+
47+
/**
48+
* Match Variable (for getFinalVars). Uses case-insensitive literal match.
49+
* Regex uses key from variable's fullName (extract part after colon).
50+
*/
51+
bool matchesVariable(const std::string &fullName) const {
52+
if (regex) {
53+
size_t colon = fullName.find(':');
54+
std::string keyPart = (colon != std::string::npos && colon + 1 < fullName.size())
55+
? fullName.substr(colon + 1) : fullName;
56+
return regex->searchAll(keyPart).size() > 0;
57+
}
58+
if (literal.size() != fullName.size()) {
59+
return false;
60+
}
61+
return std::equal(literal.begin(), literal.end(), fullName.begin(),
62+
[](char a, char b) {
63+
return std::tolower(static_cast<unsigned char>(a)) ==
64+
std::tolower(static_cast<unsigned char>(b));
65+
});
66+
}
67+
};
68+
69+
} // namespace modsecurity
70+
71+
#endif // HEADERS_MODSECURITY_RULE_REMOVE_TARGET_ENTRY_H_

headers/modsecurity/transaction.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ typedef struct Rules_t RulesSet;
5151
#include "modsecurity/variable_origin.h"
5252
#include "modsecurity/anchored_set_variable_translation_proxy.h"
5353
#include "modsecurity/audit_log.h"
54+
#ifdef __cplusplus
55+
#include "modsecurity/rule_remove_target_entry.h"
56+
#endif
5457

5558

5659
#ifndef NO_LOGS
@@ -525,7 +528,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
525528
/**
526529
*
527530
*/
528-
std::list< std::pair<int, std::string> > m_ruleRemoveTargetById;
531+
std::list<RuleRemoveTargetByIdEntry> m_ruleRemoveTargetById;
529532

530533
/**
531534
*

src/actions/ctl/rule_remove_target_by_id.cc

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@
1919
#include <string>
2020
#include <vector>
2121
#include <utility>
22+
#include <memory>
2223

2324
#include "modsecurity/transaction.h"
25+
#include "modsecurity/rule_remove_target_entry.h"
2426
#include "src/utils/string.h"
27+
#include "src/utils/regex.h"
2528

2629

2730
namespace modsecurity {
@@ -48,12 +51,35 @@ bool RuleRemoveTargetById::init(std::string *error) {
4851

4952
m_target = param[1];
5053

54+
// Detect regex format: COLLECTION:/pattern/ (e.g. ARGS:/mixpanel$/)
55+
if (m_target.size() >= 4) {
56+
size_t colon = m_target.find(':');
57+
if (colon != std::string::npos && colon + 2 < m_target.size() &&
58+
m_target[colon + 1] == '/' && m_target[m_target.size() - 1] == '/') {
59+
size_t pattern_start = colon + 2;
60+
size_t pattern_end = m_target.size() - 1;
61+
if (pattern_end > pattern_start) {
62+
std::string pattern = m_target.substr(pattern_start,
63+
pattern_end - pattern_start);
64+
m_regex = std::make_unique<Utils::Regex>(pattern, true);
65+
if (m_regex->hasError()) {
66+
error->assign("Invalid regex in ctl:ruleRemoveTargetById: " +
67+
m_target);
68+
return false;
69+
}
70+
}
71+
}
72+
}
73+
5174
return true;
5275
}
5376

5477
bool RuleRemoveTargetById::evaluate(RuleWithActions *rule, Transaction *transaction) {
55-
transaction->m_ruleRemoveTargetById.push_back(
56-
std::make_pair(m_id, m_target));
78+
RuleRemoveTargetByIdEntry entry;
79+
entry.id = m_id;
80+
entry.literal = m_target;
81+
entry.regex = m_regex; // shared_ptr: reuse pre-compiled regex
82+
transaction->m_ruleRemoveTargetById.push_back(std::move(entry));
5783
return true;
5884
}
5985

src/actions/ctl/rule_remove_target_by_id.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@
1313
*
1414
*/
1515

16+
#include <memory>
1617
#include <string>
1718

1819
#include "modsecurity/actions/action.h"
1920
#include "modsecurity/transaction.h"
21+
#include "src/utils/regex.h"
2022

2123

2224
#ifndef SRC_ACTIONS_CTL_RULE_REMOVE_TARGET_BY_ID_H_
@@ -39,6 +41,7 @@ class RuleRemoveTargetById : public Action {
3941

4042
int m_id;
4143
std::string m_target;
44+
std::shared_ptr<Utils::Regex> m_regex; // pre-compiled at config load
4245
};
4346

4447

src/rule_with_operator.cc

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -166,15 +166,8 @@ inline void RuleWithOperator::getFinalVars(variables::Variables *vars,
166166
if (std::find_if(trans->m_ruleRemoveTargetById.begin(),
167167
trans->m_ruleRemoveTargetById.end(),
168168
[&, variable, this](const auto &m) -> bool {
169-
const auto& str1 = m.second;
170-
const auto& str2 = *variable->m_fullName.get();
171-
return m.first == m_ruleId &&
172-
str1.size() == str2.size() &&
173-
std::equal(str1.begin(), str1.end(), str2.begin(),
174-
[](char a, char b) {
175-
return std::tolower(static_cast<unsigned char>(a)) ==
176-
std::tolower(static_cast<unsigned char>(b));
177-
}); // end-of std::equal
169+
return m.id == m_ruleId &&
170+
m.matchesVariable(*variable->m_fullName.get());
178171
}) != trans->m_ruleRemoveTargetById.end()) {
179172
continue;
180173
}
@@ -266,7 +259,8 @@ bool RuleWithOperator::evaluate(Transaction *trans,
266259
std::find_if(trans->m_ruleRemoveTargetById.begin(),
267260
trans->m_ruleRemoveTargetById.end(),
268261
[&, v, this](const auto &m) -> bool {
269-
return m.first == m_ruleId && m.second == v->getKeyWithCollection();
262+
return m.id == m_ruleId &&
263+
m.matches(v->getKey(), v->getKeyWithCollection());
270264
}) != trans->m_ruleRemoveTargetById.end()
271265
) {
272266
delete v;

test/benchmark/Makefile.am

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ benchmark_LDFLAGS = \
3333
$(LUA_LDFLAGS)
3434

3535
benchmark_CPPFLAGS = \
36+
-I$(top_srcdir) \
3637
-I$(top_builddir)/headers \
3738
$(GLOBAL_CPPFLAGS) \
3839
$(PCRE_CFLAGS) \

tools/rules-check/Makefile.am

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ modsec_rules_check_LDFLAGS = \
3030
$(LIBXML2_LDFLAGS)
3131

3232
modsec_rules_check_CPPFLAGS = \
33+
-I$(top_srcdir) \
3334
-I$(top_builddir)/headers \
3435
$(GLOBAL_CPPFLAGS) \
3536
$(PCRE_CFLAGS) \

0 commit comments

Comments
 (0)