Memory leak in vg_replace_malloc.c with large ruleset

[chenuduss]

I'm developer of WAF. My product can update its rules on the fly while running. I ran tests loading a large set of rules in a separately launched appication (custom developed service, which not a WAF).

Load large ruleset performed like this:

//std::string BaseRules 

auto base_ruleset = std::make_unique<modsecurity::RulesSet>();
base_ruleset->load(BaseRules.c_str());  

The application was launched using valgrind (https://valgrind.org/). The large set was loaded several times, with multiple operations to load small sets of rules between them. The leak was present even if the large set was loaded exactly once. If you remove the code that loads the large set and leave only operations with small sets, the leak disappears.

valgrind results (once cycle of tests: many small ruleset operations and one large ruleset load):

==338909== HEAP SUMMARY:
==338909==     in use at exit: 9,062 bytes in 244 blocks
==338909==   total heap usage: 51,768,316 allocs, 51,768,072 frees, 6,623,981,398 bytes allocated
==338909== 
==338909== 127 (24 direct, 103 indirect) bytes in 1 blocks are definitely lost in loss record 6 of 14
==338909==    at 0x4840F2F: operator new(unsigned long) (vg_replace_malloc.c:422)
==338909==    by 0x389E3B: allocate (new_allocator.h:137)
==338909==    by 0x389E3B: allocate (alloc_traits.h:464)
==338909==    by 0x389E3B: _M_get_node (stl_list.h:518)
==338909==    by 0x389E3B: _M_create_node<std::unique_ptr<modsecurity::RunTimeElementHolder, std::default_delete<modsecurity::RunTimeElementHolder> > > (stl_list.h:710)
==338909==    by 0x389E3B: _M_insert<std::unique_ptr<modsecurity::RunTimeElementHolder, std::default_delete<modsecurity::RunTimeElementHolder> > > (stl_list.h:2005)
==338909==    by 0x389E3B: push_back (stl_list.h:1311)
==338909==    by 0x389E3B: modsecurity::RunTimeString::appendText(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (run_time_string.cc:36)
==338909==    by 0x3AEDA2: yy::seclang_parser::parse() (seclang-parser.yy:3070)
==338909==    by 0x3885BC: 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&) (driver.cc:148)
==338909==    by 0x3211F6: modsecurity::RulesSet::load(char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (rules_set.cc:69)
==338909==    by 0x32185B: modsecurity::RulesSet::load(char const*) (rules_set.cc:100)
==338909==    by 0x2712B1: ..... (modsec_checker.cpp:45)
==338909==    by 0x23E215: .....
==338909==    by 0x258D92: .....
==338909== 
==338909== 8,543 (2,448 direct, 6,095 indirect) bytes in 34 blocks are definitely lost in loss record 14 of 14
==338909==    at 0x4840F2F: operator new(unsigned long) (vg_replace_malloc.c:422)
==338909==    by 0x3AA35E: yy::seclang_parser::parse() (seclang-parser.yy:2777)
==338909==    by 0x3885BC: 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&) (driver.cc:148)
==338909==    by 0x3211F6: modsecurity::RulesSet::load(char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (rules_set.cc:69)
==338909==    by 0x32185B: modsecurity::RulesSet::load(char const*) (rules_set.cc:100)
==338909==    by 0x2712B1: ..... (modsec_checker.cpp:45)
==338909==    by 0x23E215: .....
==338909==    by 0x2574A5: .....
==338909== 
==338909== LEAK SUMMARY:
==338909==    definitely lost: 2,472 bytes in 35 blocks
==338909==    indirectly lost: 6,198 bytes in 203 blocks
==338909==      possibly lost: 0 bytes in 0 blocks
==338909==    still reachable: 392 bytes in 6 blocks
==338909==         suppressed: 0 bytes in 0 blocks
==338909== Reachable blocks (those to which a pointer was found) are not shown.

• ModSecurity v3.0.13
• Main app: C++17 with boost ASIO
• OS Debian 12 64bit
• Valgrind-3.19.0 and LibVEX

Ruleset large_rules_raw.txt

[airween]

Hi @chenuduss,

thanks for reporting this.

Unfortunately this is a known issue in libmodsecurity3, see this list or especially the issue #2848.

The problem is not in vg_replace_malloc.c:422 as you mentioned (that's not part of libmodsecurity3), but in the generated code by Bison here.

There the generated code allocates the action by new - probably it's worth a try to change this to std::make_unique, I mean we could change new actions::Msg(...) to std::make_unique<actions::Msg>(...) (generally, everywhere) - but it's just an idea.