Crashes in v3/dev/3.1-experimental branch

[WGH-]

Hello, @zimmerle! I debugged crashes found when testing #2374 (comment) a bit.

I found two trivial bugs, here are the patches for them:

diff --git a/src/rule_with_actions.cc b/src/rule_with_actions.cc
index 738b3d72..82e1bd6a 100644
--- a/src/rule_with_actions.cc
+++ b/src/rule_with_actions.cc
@@ -250,7 +250,7 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans) const {
         }
     }
     for (auto &a : getMatchActionsPtr()) {
-        if (!dynamic_cast<ActionDisruptive *>(a) != NULL
+        if (dynamic_cast<ActionDisruptive *>(a) != NULL
                 && !(disruptiveAlreadyExecuted
                 && dynamic_cast<actions::Block *>(a))) {
             executeAction(trans, a, false);

diff --git a/src/rules_exceptions.cc b/src/rules_exceptions.cc
index aee9e8c0..8f0c6ef1 100644
--- a/src/rules_exceptions.cc
+++ b/src/rules_exceptions.cc
@@ -46,7 +46,7 @@ bool RulesExceptions::loadUpdateActionById(double id,
         }

         if (dynamic_cast<actions::transformations::Transformation *>(a.get())) {
-            actions::transformations::Transformation *t = dynamic_cast<actions::transformations::Transformation *>(a.get());
+            actions::transformations::Transformation *t = dynamic_cast<actions::transformations::Transformation *>(a.release());
             m_action_transformation_update_target_by_id.emplace(
                 std::pair<double,
                 std::unique_ptr<actions::transformations::Transformation>>(id, std::unique_ptr<actions::transformations::Transformation>(t))

And not-so-trivial UAFs which I was unable to decipher. "freed by" and "allocated by" look really wrong here.

=================================================================
==1494020==ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000528f0 at pc 0x7ffff6fdcf37 bp 0x7fffffff79b0 sp 0x7fffffff79a0
READ of size 8 at 0x6160000528f0 thread T0
    #0 0x7ffff6fdcf36 in modsecurity::variables::Rule_DictElement::msg(modsecurity::Transaction*, modsecurity::RuleWithActions const*, std::vector<modsecurity::VariableValue const*, std::allocator<modsecurity::VariableValue const*> >*) (/usr/local/modsecurity/lib/libmodsecurity.so.3+0x285f36)
    #1 0x7ffff6fdd63b in non-virtual thunk to modsecurity::variables::Rule_DictElement::evaluate(modsecurity::Transaction*, std::vector<modsecurity::VariableValue const*, std::allocator<modsecurity::VariableValue const*> >*) (/usr/local/modsecurity/lib/libmodsecurity.so.3+0x28663b)
    #2 0x7ffff7140c82 in modsecurity::RunTimeString::evaluate[abi:cxx11](modsecurity::Transaction*) /home/wgh/ModSecurity-upstream/src/run_time_string.cc:58
    #3 0x7ffff721258d in modsecurity::actions::ActionWithRunTimeString::getEvaluatedRunTimeString[abi:cxx11](modsecurity::Transaction*) const ../src/actions/action_with_run_time_string.h:53
    #4 0x7ffff721258d in modsecurity::actions::SetVar::execute(modsecurity::Transaction*) const actions/set_var.cc:50
    #5 0x7ffff714f216 in modsecurity::RuleWithActions::executeActionsIndependentOfChainedRuleResult(modsecurity::Transaction*) const /home/wgh/ModSecurity-upstream/src/rule_with_actions.cc:207
    #6 0x7ffff717769c in modsecurity::RuleWithOperator::evaluate(modsecurity::Transaction*) const /home/wgh/ModSecurity-upstream/src/rule_with_operator.cc:341
    #7 0x7ffff711de3d in modsecurity::RulesSet::evaluate(int, modsecurity::Transaction*) /home/wgh/ModSecurity-upstream/src/rules_set.cc:300
    #8 0x7ffff70cfa30 in modsecurity::Transaction::processRequestBody() /home/wgh/ModSecurity-upstream/src/transaction.cc:985
    #9 0x55555555e209 in process_transaction /home/wgh/modsecurity-benchmark/benchmark.cc:103
    #10 0x55555555e209 in main /home/wgh/modsecurity-benchmark/benchmark.cc:194
    #11 0x7ffff69900b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #12 0x555555560a9d in _start (/home/wgh/modsecurity-benchmark/benchmark+0xca9d)

0x6160000528f0 is located 112 bytes inside of 601-byte region [0x616000052880,0x616000052ad9)
freed by thread T0 here:
    #0 0x7ffff769c8df in operator delete(void*) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x1108df)
    #1 0x7ffff7172c6a in __gnu_cxx::new_allocator<char>::deallocate(char*, unsigned long) /usr/include/c++/9/ext/new_allocator.h:128
    #2 0x7ffff7172c6a in std::allocator_traits<std::allocator<char> >::deallocate(std::allocator<char>&, char*, unsigned long) /usr/include/c++/9/bits/alloc_traits.h:470
    #3 0x7ffff7172c6a in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_destroy(unsigned long) /usr/include/c++/9/bits/basic_string.h:237
    #4 0x7ffff7172c6a in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_dispose() /usr/include/c++/9/bits/basic_string.h:232
    #5 0x7ffff7172c6a in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() /usr/include/c++/9/bits/basic_string.h:658
    #6 0x7ffff7172c6a in modsecurity::RuleWithOperator::evaluate(modsecurity::Transaction*) const /home/wgh/ModSecurity-upstream/src/rule_with_operator.cc:251
    #7 0x7ffff711de3d in modsecurity::RulesSet::evaluate(int, modsecurity::Transaction*) /home/wgh/ModSecurity-upstream/src/rules_set.cc:300
    #8 0x7ffff70cfa30 in modsecurity::Transaction::processRequestBody() /home/wgh/ModSecurity-upstream/src/transaction.cc:985
    #9 0x55555555e209 in process_transaction /home/wgh/modsecurity-benchmark/benchmark.cc:103
    #10 0x55555555e209 in main /home/wgh/modsecurity-benchmark/benchmark.cc:194
    #11 0x7ffff69900b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

previously allocated by thread T0 here:
    #0 0x7ffff769b947 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10f947)
    #1 0x7ffff6cb8bbd in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_mutate(unsigned long, unsigned long, char const*, unsigned long) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x142bbd)

SUMMARY: AddressSanitizer: heap-use-after-free (/usr/local/modsecurity/lib/libmodsecurity.so.3+0x285f36) in modsecurity::variables::Rule_DictElement::msg(modsecurity::Transaction*, modsecurity::RuleWithActions const*, std::vector<modsecurity::VariableValue const*, std::allocator<modsecurity::VariableValue const*> >*)
Shadow bytes around the buggy address:
  0x0c2c800024c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c800024d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c800024e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c800024f0: fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa
  0x0c2c80002500: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c2c80002510: fd fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd
  0x0c2c80002520: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c80002530: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c80002540: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2c80002550: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa
  0x0c2c80002560: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==1494020==ABORTING

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50      ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff698e859 in __GI_abort () at abort.c:79
#2  0x00007ffff76b76a2 in ?? () from /usr/lib/x86_64-linux-gnu/libasan.so.5
#3  0x00007ffff76c224c in ?? () from /usr/lib/x86_64-linux-gnu/libasan.so.5
#4  0x00007ffff76a38ec in ?? () from /usr/lib/x86_64-linux-gnu/libasan.so.5
#5  0x00007ffff76a3363 in ?? () from /usr/lib/x86_64-linux-gnu/libasan.so.5
#6  0x00007ffff76a41ab in __asan_report_load8 () from /usr/lib/x86_64-linux-gnu/libasan.so.5
#7  0x00007ffff6fdcf37 in std::__shared_ptr<modsecurity::actions::Msg, (__gnu_cxx::_Lock_policy)2>::operator bool (this=0x6160000528f0) at ../src/variables/rule.h:136
#8  std::operator!=<modsecurity::actions::Msg>(std::shared_ptr<modsecurity::actions::Msg> const&, decltype(nullptr)) (__a=<error reading variable: Cannot access memory at address 0x3038253029657c40>)
    at /usr/include/c++/9/bits/shared_ptr.h:404
#9  modsecurity::RuleWithActions::hasMessageAction (this=0x616000052880) at ../src/rule_with_actions.h:426
#10 modsecurity::variables::Rule_DictElement::msg (t=<optimized out>, rule=0x616000052880, l=<optimized out>) at ../src/variables/rule.h:134
#11 0x00007ffff6fdd63c in non-virtual thunk to modsecurity::variables::Rule_DictElement::evaluate(modsecurity::Transaction*, std::vector<modsecurity::VariableValue const*, std::allocator<modsecurity::VariableValue const*> >*) ()
    at ../src/variables/rule.h:165
#12 0x00007ffff7140c83 in modsecurity::RunTimeString::evaluate[abi:cxx11](modsecurity::Transaction*) (this=<optimized out>, transaction=transaction@entry=0x6250005e8900) at /usr/include/c++/9/bits/shared_ptr_base.h:1309
#13 0x00007ffff721258e in modsecurity::actions::ActionWithRunTimeString::getEvaluatedRunTimeString[abi:cxx11](modsecurity::Transaction*) const (transaction=0x6250005e8900, this=0x60c00002eb40) at /usr/include/c++/9/bits/unique_ptr.h:721
#14 modsecurity::actions::SetVar::execute (this=0x60c00002eb40, t=<optimized out>) at actions/set_var.cc:50
#15 0x00007ffff714f217 in modsecurity::RuleWithActions::executeActionsIndependentOfChainedRuleResult (this=this@entry=0x616000105080, trans=trans@entry=0x6250005e8900) at rule_with_actions.cc:207
#16 0x00007ffff717769d in modsecurity::RuleWithOperator::evaluate (this=0x616000105080, trans=<optimized out>) at rule_with_operator.cc:341
#17 0x00007ffff711de3e in modsecurity::RulesSet::evaluate (this=<optimized out>, phase=phase@entry=3, t=t@entry=0x6250005e8900) at /usr/include/c++/9/bits/shared_ptr_base.h:1309
#18 0x00007ffff70cfa31 in modsecurity::Transaction::processRequestBody (this=<optimized out>) at transaction.cc:985
#19 0x000055555555e20a in process_transaction (it=0x7fffffffdb00, modsecTransaction=0x6250005e8900, j=...) at benchmark.cc:103
#20 main (argc=<optimized out>, argv=<optimized out>) at benchmark.cc:194

I think the problem might be due to incorrect use of smart pointers somewhere where the rules are parsed. For example, I found the following problematic fragment, but that wasn't enough to fix the problem.

void RuleWithActions::addDefaultAction(std::shared_ptr<actions::Action> a) {
// std::dynamic_pointer_cast<actions::Msg> should've been used here to avoid UAF
   } else if (dynamic_cast<actions::Msg *>(a.get())) {
        m_defaultActionMsg.reset(dynamic_cast<actions::Msg *>(a.get()));
// ...

Maybe addAction is also wrong, but it's hard to tell since the incoming argument is not a smart pointer.

[zimmerle]

Well noticed. Let me give a little bit of background on that particular change.

The idea is that 3.1 will be able to reload the rules in place, without the server to restart. For that, it was necessary to reduce the memory footprint of the RuleObject and adjacent. At a certain time, rules will be allocated twice in memory: (a) Adress the active requests; (b) Adress the new ones. That new feature also makes it necessary to clean up the memory properly after a new rule set is un-load.

Having said that, in terms of actions (cited in this issue), we have different approaches: some stuff became a rule property (logging) and some are actions to be used in run time with parameters inherited from the parser. To exemplify: Instead to be an object: nolog, log, auditlog and noauditlog became a bit properties of the class Rule:

https://github.com/SpiderLabs/ModSecurity/blob/5824470768e73d438a5db03b7a6eb24d7c795097/src/rule_with_actions.h#L592-L595

Further, those will become a two-bit property, that will be queried with a bitwise operation:

Porporty	Value
noLog	00
log	01
noAuditLog	10
auditLog	11

The thing with run-time-actions, the ones that are classes, is that they belong to a rule. Still, they can also belong to a rule in a second (or nth) virtual host (a consequence of a merging). Atop of that, there are the DefaultActions, which we are aiming to pre-compute in rules load time -- avoid any but necessary processing at runtime.

The smart pointers have an important role in that architecture as we are offloading the responsibility to disposable the action object to C++. I will have a look to understand better what is going on and let you know.

Thanks for testing and reporting this ;) appreciated!

[zimmerle]

The dynamic_cast<ActionDisruptive *>(a) != NULL issue was already addressed on v3.1-experimental.

[WGH-]

Might not be the same crash you mentioned in #2374 (comment), but it's likely related.

Commit b321829

To reproduce, use this string in benchmark.cc:

char request_uri[] = "/foobar?message=%22AAAAAAAAAAAAAAAAA%3DBBBBB%3BAAAAAAAAAAAAAAA%3DBBBBB%3BAAAAAAAAAAAA%3DBBBBB%3BAAAAAAAAAA%3DBBBBB%3BAAAAAAAAAAAA%3DBBBBB%3BAAAAAAAAAAAAA%3DBBBBB%3BAAAAAAAAAAA%3DBBBBB%3BAAAAAAAAAA%3DBBBBB%3BAAAAAAAAAAAAAA%3DBBBBB%3BAAAAAAAAAAA%3DBBBBB%3BAAAAAAAAAAAAAA%3DBBBBB%3BloadAAAAAAAA%3DBBBBB%3BAAAAAAAAAAAAAAA%22"

(gdb) bt
#0  modsecurity::RuleWithActions::getMessage[abi:cxx11](modsecurity::Transaction*) const (this=this@entry=0x555556175e00, t=t@entry=0x5555565bb3e0)
    at rule_with_actions.cc:399
#1  0x00007ffff7e49494 in modsecurity::variables::Rule_DictElement::msg (t=0x5555565bb3e0, rule=0x555556175e00, l=0x7fffffffc720)
    at /usr/include/c++/9/bits/shared_ptr_base.h:1312
#2  0x00007ffff7e4970c in non-virtual thunk to modsecurity::variables::Rule_DictElement::evaluate(modsecurity::Transaction*, std::vector<modsecurity::VariableValue const*, std::allocator<modsecurity::VariableValue const*> >*) () at ../src/variables/rule.h:165
#3  0x00007ffff7ea79ae in modsecurity::RunTimeString::evaluate[abi:cxx11](modsecurity::Transaction*) (this=0x555556177050, transaction=transaction@entry=0x5555565bb3e0)
    at /usr/include/c++/9/bits/shared_ptr_base.h:1309
#4  0x00007ffff7edae66 in modsecurity::actions::ActionWithRunTimeString::getEvaluatedRunTimeString[abi:cxx11](modsecurity::Transaction*) const (
    transaction=0x5555565bb3e0, this=0x555556171e00) at /usr/include/c++/9/bits/unique_ptr.h:721
#5  modsecurity::actions::SetVar::execute (this=0x555556171e00, t=0x5555565bb3e0) at actions/set_var.cc:50
#6  0x00007ffff7eaaafc in modsecurity::RuleWithActions::executeActionsIndependentOfChainedRuleResult (this=this@entry=0x5555565a1970, trans=trans@entry=0x5555565bb3e0)
    at rule_with_actions.cc:207
#7  0x00007ffff7eb33dd in modsecurity::RuleWithOperator::evaluate (this=0x5555565a1970, trans=0x5555565bb3e0) at rule_with_operator.cc:340
#8  0x00007ffff7e9fb7f in modsecurity::RulesSet::evaluate (this=0x5555555aac80, phase=phase@entry=3, t=t@entry=0x5555565bb3e0)
    at /usr/include/c++/9/bits/shared_ptr_base.h:1309
#9  0x00007ffff7e8bbb5 in modsecurity::Transaction::processRequestBody (this=0x5555565bb3e0) at transaction.cc:985
#10 0x0000555555556f2c in main (argc=<optimized out>, argv=<optimized out>) at benchmark.cc:136

==95724==ERROR: AddressSanitizer: heap-use-after-free on address 0x61600006f6f0 at pc 0x7f6f3a19bf27 bp 0x7fffcb419890 sp 0x7fffcb419880
READ of size 8 at 0x61600006f6f0 thread T0
    #0 0x7f6f3a19bf26 in modsecurity::variables::Rule_DictElement::msg(modsecurity::Transaction*, modsecurity::RuleWithActions const*, std::vector<modsecurity::VariableValue const*, std::allocator<modsecurity::VariableValue const*> >*) (/home/wgh/ModSecurity-upstream/src/.libs/libmodsecurity.so.3+0x285f26)
    #1 0x7f6f3a19c62b in non-virtual thunk to modsecurity::variables::Rule_DictElement::evaluate(modsecurity::Transaction*, std::vector<modsecurity::VariableValue const*, std::allocator<modsecurity::VariableValue const*> >*) (/home/wgh/ModSecurity-upstream/src/.libs/libmodsecurity.so.3+0x28662b)
    #2 0x7f6f3a3015c2 in modsecurity::RunTimeString::evaluate[abi:cxx11](modsecurity::Transaction*) /home/wgh/ModSecurity-upstream/src/run_time_string.cc:58
    #3 0x7f6f3a3d41d1 in modsecurity::actions::ActionWithRunTimeString::getEvaluatedRunTimeString[abi:cxx11](modsecurity::Transaction*) const ../src/actions/action_with_run_time_string.h:53
    #4 0x7f6f3a3d41d1 in modsecurity::actions::SetVar::execute(modsecurity::Transaction*) const actions/set_var.cc:50
    #5 0x7f6f3a30fb56 in modsecurity::RuleWithActions::executeActionsIndependentOfChainedRuleResult(modsecurity::Transaction*) const /home/wgh/ModSecurity-upstream/src/rule_with_actions.cc:207
    #6 0x7f6f3a338662 in modsecurity::RuleWithOperator::evaluate(modsecurity::Transaction*) const /home/wgh/ModSecurity-upstream/src/rule_with_operator.cc:340
    #7 0x7f6f3a2de49f in modsecurity::RulesSet::evaluate(int, modsecurity::Transaction*) /home/wgh/ModSecurity-upstream/src/rules_set.cc:300
    #8 0x7f6f3a290092 in modsecurity::Transaction::processRequestBody() /home/wgh/ModSecurity-upstream/src/transaction.cc:985
    #9 0x56129356b4d7 in main /home/wgh/ModSecurity-upstream/test/benchmark/benchmark.cc:136
    #10 0x7f6f399bf0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #11 0x56129343d67d in _start (/home/wgh/ModSecurity-upstream/test/benchmark/.libs/benchmark+0xb67d)

0x61600006f6f0 is located 112 bytes inside of 576-byte region [0x61600006f680,0x61600006f8c0)
freed by thread T0 here:
    #0 0x56129352b3df in operator delete(void*) (/home/wgh/ModSecurity-upstream/test/benchmark/.libs/benchmark+0xf93df)
    #1 0x7f6f3a26259b in std::default_delete<modsecurity::RuleWithActions>::operator()(modsecurity::RuleWithActions*) const /usr/include/c++/9/bits/unique_ptr.h:81
    #2 0x7f6f3a26259b in std::_Sp_counted_deleter<modsecurity::RuleWithActions*, std::default_delete<modsecurity::RuleWithActions>, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() /usr/include/c++/9/bits/shared_ptr_base.h:471
    #3 0x7f6f3a254204 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/include/c++/9/bits/shared_ptr_base.h:155
    #4 0x7f6f3a254204 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /usr/include/c++/9/bits/shared_ptr_base.h:730
    #5 0x7f6f3a254204 in std::__shared_ptr<modsecurity::Rule, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() /usr/include/c++/9/bits/shared_ptr_base.h:1169
    #6 0x7f6f3a254204 in std::shared_ptr<modsecurity::Rule>::~shared_ptr() /usr/include/c++/9/bits/shared_ptr.h:103
    #7 0x7f6f3a254204 in void std::_Destroy<std::shared_ptr<modsecurity::Rule> >(std::shared_ptr<modsecurity::Rule>*) /usr/include/c++/9/bits/stl_construct.h:98
    #8 0x7f6f3a254204 in void std::_Destroy_aux<false>::__destroy<std::shared_ptr<modsecurity::Rule>*>(std::shared_ptr<modsecurity::Rule>*, std::shared_ptr<modsecurity::Rule>*) /usr/include/c++/9/bits/stl_construct.h:108
    #9 0x7f6f3a254204 in void std::_Destroy<std::shared_ptr<modsecurity::Rule>*>(std::shared_ptr<modsecurity::Rule>*, std::shared_ptr<modsecurity::Rule>*) /usr/include/c++/9/bits/stl_construct.h:137
    #10 0x7f6f3a254204 in void std::_Destroy<std::shared_ptr<modsecurity::Rule>*, std::shared_ptr<modsecurity::Rule> >(std::shared_ptr<modsecurity::Rule>*, std::shared_ptr<modsecurity::Rule>*, std::allocator<std::shared_ptr<modsecurity::Rule> >&) /usr/include/c++/9/bits/stl_construct.h:206
    #11 0x7f6f3a254204 in std::vector<std::shared_ptr<modsecurity::Rule>, std::allocator<std::shared_ptr<modsecurity::Rule> > >::~vector() /usr/include/c++/9/bits/stl_vector.h:677
    #12 0x7f6f3a254204 in modsecurity::Rules::~Rules() ../headers/modsecurity/rules.h:46
    #13 0x7f6f3a254204 in std::array<modsecurity::Rules, 7ul>::~array() /usr/include/c++/9/array:94
    #14 0x7f6f3a254204 in modsecurity::RulesSetPhases::~RulesSetPhases() ../headers/modsecurity/rules_set_phases.h:44
    #15 0x7f6f3a254204 in modsecurity::Parser::Driver::~Driver() parser/driver.cc:36
    #16 0x7f6f3a254ebe in modsecurity::Parser::Driver::~Driver() parser/driver.cc:42
    #17 0x7f6f3a2e8c0e in modsecurity::RulesSet::loadFromUri(char const*) /home/wgh/ModSecurity-upstream/src/rules_set.cc:102
    #18 0x56129356a41c in main /home/wgh/ModSecurity-upstream/test/benchmark/benchmark.cc:82
    #19 0x7f6f399bf0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

previously allocated by thread T0 here:
    #0 0x56129352a447 in operator new(unsigned long) (/home/wgh/ModSecurity-upstream/test/benchmark/.libs/benchmark+0xf8447)
    #1 0x7f6f3a0d8637 in yy::seclang_parser::parse() /home/wgh/ModSecurity-upstream/src/seclang-parser.yy:1096
    #2 0x7f6f3a26094b in modsecurity::Parser::Driver::parse(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) parser/driver.cc:210
    #3 0x7f6f3a261abb in modsecurity::Parser::Driver::parseFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) parser/driver.cc:254
    #4 0x7f6f3a2e8b49 in modsecurity::RulesSet::loadFromUri(char const*) /home/wgh/ModSecurity-upstream/src/rules_set.cc:95
    #5 0x56129356a41c in main /home/wgh/ModSecurity-upstream/test/benchmark/benchmark.cc:82
    #6 0x7f6f399bf0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

SUMMARY: AddressSanitizer: heap-use-after-free (/home/wgh/ModSecurity-upstream/src/.libs/libmodsecurity.so.3+0x285f26) in modsecurity::variables::Rule_DictElement::msg(modsecurity::Transaction*, modsecurity::RuleWithActions const*, std::vector<modsecurity::VariableValue const*, std::allocator<modsecurity::VariableValue const*> >*)

[zimmerle]

Perfect! I am going to investigate.

RunTimeStrings are strings that accept variables as macro expansions. Those variables sometimes need to have the knowledge of the Rule that it belongs (XML or RULE). In this case, a new variable instance needs to be created for every rule. That scenario became especially confused when applied to rule merging. I will try to create a new set of tests to stress the rule merging focusing on reproduce this kind of error. As collateral, this test suit will highlight if such errors appear as regression.

[zimmerle]

@WGH- ping. Can you assist me with some testing on that matter? I think we have already mitigated that issue + a couple of changes for the benefit of performance && usability. It would be wonderful if I can count on you to test this again.

[zimmerle]

I think that it is no longer an issue in v3.1-experimetal. Therefore I am closing it.

The problem was related to a broken copy constructor for an Action that wasn't updating the reference to a new rule. Leading to an invalid memory access.