From d7c77a73f55fad1a7d856d9e2f92f71b1f14e61d Mon Sep 17 00:00:00 2001 From: Liu DongMiao Date: Sun, 24 Apr 2022 19:45:57 +0800 Subject: [PATCH 1/2] fix memory leak in rules --- headers/modsecurity/rule.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/headers/modsecurity/rule.h b/headers/modsecurity/rule.h index b10e0556e..1d5570a8d 100644 --- a/headers/modsecurity/rule.h +++ b/headers/modsecurity/rule.h @@ -86,6 +86,8 @@ class Rule { return *this; } + virtual ~Rule() {} + virtual bool evaluate(Transaction *transaction) = 0; virtual bool evaluate(Transaction *transaction, From 0e8290aee3529efe4bfa48fd35cb370d73958522 Mon Sep 17 00:00:00 2001 From: Liu Dongmiao Date: Sun, 24 Apr 2022 19:52:36 +0800 Subject: [PATCH 2/2] fix memory patch, part 2 --- src/parser/driver.cc | 4 ++-- src/parser/location.hh | 4 ++-- src/parser/seclang-parser.cc | 14 +++++++------- src/parser/seclang-parser.yy | 14 +++++++------- src/parser/seclang-scanner.cc | 6 +++--- src/parser/seclang-scanner.ll | 6 +++--- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/parser/driver.cc b/src/parser/driver.cc index c8d15b48a..8505a4992 100644 --- a/src/parser/driver.cc +++ b/src/parser/driver.cc @@ -129,9 +129,9 @@ int Driver::parse(const std::string &f, const std::string &ref) { m_lastRule = nullptr; loc.push_back(new yy::location()); if (ref.empty()) { - loc.back()->begin.filename = loc.back()->end.filename = new std::string("<>"); + loc.back()->begin.filename = loc.back()->end.filename = std::make_shared("<>"); } else { - loc.back()->begin.filename = loc.back()->end.filename = new std::string(ref); + loc.back()->begin.filename = loc.back()->end.filename = std::make_shared(ref); } if (f.empty()) { diff --git a/src/parser/location.hh b/src/parser/location.hh index 89e9472c6..c33ec1f96 100644 --- a/src/parser/location.hh +++ b/src/parser/location.hh @@ -80,7 +80,7 @@ namespace yy { counter_type l = 1, counter_type c = 1) { - filename = fn; + filename = std::shared_ptr(fn); line = l; column = c; } @@ -105,7 +105,7 @@ namespace yy { /** \} */ /// File name to which this position refers. - filename_type* filename; + std::shared_ptr filename; /// Current line number. counter_type line; /// Current column number. diff --git a/src/parser/seclang-parser.cc b/src/parser/seclang-parser.cc index 7f797bd0f..ca7aa68d7 100644 --- a/src/parser/seclang-parser.cc +++ b/src/parser/seclang-parser.cc @@ -1321,7 +1321,7 @@ namespace yy { #line 320 "seclang-parser.yy" { // Initialize the initial location. - yyla.location.begin.filename = yyla.location.end.filename = new std::string(driver.file); + yyla.location.begin.filename = yyla.location.end.filename = std::make_shared(driver.file); } #line 1328 "seclang-parser.cc" @@ -2287,7 +2287,7 @@ namespace yy { #line 1078 "seclang-parser.yy" { std::vector *a = new std::vector(); - std::vector *t = new std::vector(); + auto t = std::make_shared>(); for (auto &i : *yystack_[0].value.as < std::unique_ptr > > > ().get()) { if (dynamic_cast(i.get())) { t->push_back(dynamic_cast(i.release())); @@ -2305,7 +2305,7 @@ namespace yy { /* op */ op, /* variables */ v, /* actions */ a, - /* transformations */ t, + /* transformations */ t.get(), /* file name */ std::unique_ptr(new std::string(*yystack_[3].location.end.filename)), /* line number */ yystack_[3].location.end.line )); @@ -2344,7 +2344,7 @@ namespace yy { #line 1127 "seclang-parser.yy" { std::vector *a = new std::vector(); - std::vector *t = new std::vector(); + auto t = std::make_shared>(); for (auto &i : *yystack_[0].value.as < std::unique_ptr > > > ().get()) { if (dynamic_cast(i.get())) { t->push_back(dynamic_cast(i.release())); @@ -2354,7 +2354,7 @@ namespace yy { } std::unique_ptr rule(new RuleUnconditional( /* actions */ a, - /* transformations */ t, + /* transformations */ t.get(), /* file name */ std::unique_ptr(new std::string(*yystack_[1].location.end.filename)), /* line number */ yystack_[1].location.end.line )); @@ -2368,7 +2368,7 @@ namespace yy { { std::string err; std::vector *a = new std::vector(); - std::vector *t = new std::vector(); + auto t = std::make_shared>(); for (auto &i : *yystack_[0].value.as < std::unique_ptr > > > ().get()) { if (dynamic_cast(i.get())) { t->push_back(dynamic_cast(i.release())); @@ -2379,7 +2379,7 @@ namespace yy { std::unique_ptr r(new RuleScript( /* path to script */ yystack_[1].value.as < std::string > (), /* actions */ a, - /* transformations */ t, + /* transformations */ t.get(), /* file name */ std::unique_ptr(new std::string(*yystack_[1].location.end.filename)), /* line number */ yystack_[1].location.end.line )); diff --git a/src/parser/seclang-parser.yy b/src/parser/seclang-parser.yy index 332a30d16..ef8c64281 100644 --- a/src/parser/seclang-parser.yy +++ b/src/parser/seclang-parser.yy @@ -319,7 +319,7 @@ using namespace modsecurity::operators; %initial-action { // Initialize the initial location. - @$.begin.filename = @$.end.filename = new std::string(driver.file); + @$.begin.filename = @$.end.filename = std::make_shared(driver.file); }; %define parse.trace %define parse.error verbose @@ -1077,7 +1077,7 @@ expression: | DIRECTIVE variables op actions { std::vector *a = new std::vector(); - std::vector *t = new std::vector(); + auto t = std::make_shared>(); for (auto &i : *$4.get()) { if (dynamic_cast(i.get())) { t->push_back(dynamic_cast(i.release())); @@ -1095,7 +1095,7 @@ expression: /* op */ op, /* variables */ v, /* actions */ a, - /* transformations */ t, + /* transformations */ t.get(), /* file name */ std::unique_ptr(new std::string(*@1.end.filename)), /* line number */ @1.end.line )); @@ -1126,7 +1126,7 @@ expression: | CONFIG_DIR_SEC_ACTION actions { std::vector *a = new std::vector(); - std::vector *t = new std::vector(); + auto t = std::make_shared>(); for (auto &i : *$2.get()) { if (dynamic_cast(i.get())) { t->push_back(dynamic_cast(i.release())); @@ -1136,7 +1136,7 @@ expression: } std::unique_ptr rule(new RuleUnconditional( /* actions */ a, - /* transformations */ t, + /* transformations */ t.get(), /* file name */ std::unique_ptr(new std::string(*@1.end.filename)), /* line number */ @1.end.line )); @@ -1146,7 +1146,7 @@ expression: { std::string err; std::vector *a = new std::vector(); - std::vector *t = new std::vector(); + auto t = std::make_shared>(); for (auto &i : *$2.get()) { if (dynamic_cast(i.get())) { t->push_back(dynamic_cast(i.release())); @@ -1157,7 +1157,7 @@ expression: std::unique_ptr r(new RuleScript( /* path to script */ $1, /* actions */ a, - /* transformations */ t, + /* transformations */ t.get(), /* file name */ std::unique_ptr(new std::string(*@1.end.filename)), /* line number */ @1.end.line )); diff --git a/src/parser/seclang-scanner.cc b/src/parser/seclang-scanner.cc index af23ab336..6349702d5 100644 --- a/src/parser/seclang-scanner.cc +++ b/src/parser/seclang-scanner.cc @@ -8544,7 +8544,7 @@ YY_RULE_SETUP std::string err; std::string f = modsecurity::utils::find_resource(s, *driver.loc.back()->end.filename, &err); driver.loc.push_back(new yy::location()); - driver.loc.back()->begin.filename = driver.loc.back()->end.filename = new std::string(f); + driver.loc.back()->begin.filename = driver.loc.back()->end.filename = std::make_shared(f); yyin = fopen(f.c_str(), "r" ); if (!yyin) { BEGIN(INITIAL); @@ -8575,7 +8575,7 @@ YY_RULE_SETUP for (auto& s: files) { std::string f = modsecurity::utils::find_resource(s, *driver.loc.back()->end.filename, &err); driver.loc.push_back(new yy::location()); - driver.loc.back()->begin.filename = driver.loc.back()->end.filename = new std::string(f); + driver.loc.back()->begin.filename = driver.loc.back()->end.filename = std::make_shared(f); yyin = fopen(f.c_str(), "r" ); if (!yyin) { @@ -8608,7 +8608,7 @@ YY_RULE_SETUP c.setKey(key); driver.loc.push_back(new yy::location()); - driver.loc.back()->begin.filename = driver.loc.back()->end.filename = new std::string(url); + driver.loc.back()->begin.filename = driver.loc.back()->end.filename = std::make_shared(url); YY_BUFFER_STATE temp = YY_CURRENT_BUFFER; yypush_buffer_state(temp); diff --git a/src/parser/seclang-scanner.ll b/src/parser/seclang-scanner.ll index 2e32fea04..bcf4d67bb 100755 --- a/src/parser/seclang-scanner.ll +++ b/src/parser/seclang-scanner.ll @@ -1257,7 +1257,7 @@ EQUALS_MINUS (?i:=\-) std::string err; std::string f = modsecurity::utils::find_resource(s, *driver.loc.back()->end.filename, &err); driver.loc.push_back(new yy::location()); - driver.loc.back()->begin.filename = driver.loc.back()->end.filename = new std::string(f); + driver.loc.back()->begin.filename = driver.loc.back()->end.filename = std::make_shared(f); yyin = fopen(f.c_str(), "r" ); if (!yyin) { BEGIN(INITIAL); @@ -1285,7 +1285,7 @@ EQUALS_MINUS (?i:=\-) for (auto& s: files) { std::string f = modsecurity::utils::find_resource(s, *driver.loc.back()->end.filename, &err); driver.loc.push_back(new yy::location()); - driver.loc.back()->begin.filename = driver.loc.back()->end.filename = new std::string(f); + driver.loc.back()->begin.filename = driver.loc.back()->end.filename = std::make_shared(f); yyin = fopen(f.c_str(), "r" ); if (!yyin) { @@ -1314,7 +1314,7 @@ EQUALS_MINUS (?i:=\-) c.setKey(key); driver.loc.push_back(new yy::location()); - driver.loc.back()->begin.filename = driver.loc.back()->end.filename = new std::string(url); + driver.loc.back()->begin.filename = driver.loc.back()->end.filename = std::make_shared(url); YY_BUFFER_STATE temp = YY_CURRENT_BUFFER; yypush_buffer_state(temp);