Removes memory leaks on the parse

- Parser location is now a custom class. It holds a shared pointer
  with the file name; If the parser fails, the resource is deleted.

 - To follow the parser change, the Rule class now holds the file
  name in a shared pointer instead of a unique pointer. As a shared
  pointer we avoid duplication of the file name in memory, plus,
  it frees itself when not in use anymore.

 - Operator init also accepting the filename as a shared pointer.

 - Driver is treating m_location was privative. Now it holds a
  std::list<std::shared_ptr<yy::seclang_parser::location_type>>
  instead of: std::list<yy::seclang_parser::location_type *>.

 - Fix: addSecRule on Driver() was changed from RuleWithAction to
  RuleWithOperator.

 - Minor changes on the regression and rules-check utility to force
  deletion of resources even when they fail.

 - Couple of virtual destructors were placed to force the shared
  pointer decrementing on shared variables.

 - Deleted constructors for copy were placed for the sake of
  readability.
This commit is contained in:
Felipe Zimmerle 2021-01-12 09:58:30 -03:00
parent 0bf36192a5
commit 9f47f1473c
No known key found for this signature in database
GPG Key ID: E6DFB08CE8B11277
54 changed files with 2134 additions and 2026 deletions

1
.gitignore vendored
View File

@ -23,7 +23,6 @@ depcomp
.dirstamp
src/config.h
src/config.h.in
src/location.hh
src/position.hh
src/stack.hh
src/stamp-h1

View File

@ -44,8 +44,8 @@ class Operator;
class Rule {
public:
Rule(std::unique_ptr<std::string> fileName, int lineNumber)
: m_fileName(std::move(fileName)),
Rule(std::shared_ptr<std::string> fileName, int lineNumber)
: m_fileName(fileName),
m_lineNumber(lineNumber),
m_phase(modsecurity::Phases::RequestHeadersPhase)
{ }

View File

@ -47,6 +47,17 @@ class Rules {
using iterator=typename container::iterator;
using const_iterator=typename container::const_iterator;
Rules()
: m_rules()
{ };
Rules(const Rules&) = delete;
virtual ~Rules() {
m_rules.clear();
}
int append(Rules *from);
bool insert(const std::shared_ptr<Rule> &rule);

View File

@ -60,7 +60,8 @@ class RulesSet : public RulesSetProperties {
#endif
{ }
~RulesSet() { }
~RulesSet()
{ }
int loadFromUri(const char *uri);
int loadRemote(const char *key, const char *uri);

View File

@ -47,6 +47,15 @@ class RulesSetPhases {
using iterator = typename container::iterator;
using const_iterator = typename container::const_iterator;
RulesSetPhases()
: m_rulesAtPhase()
{ }
RulesSetPhases(const RulesSetPhases&) = delete;
virtual ~RulesSetPhases()
{ };
void insert(std::shared_ptr<Rule> rule);
void append(RulesSetPhases *from);

View File

@ -23,7 +23,7 @@
namespace modsecurity {
namespace operators {
bool FuzzyHash::init(const std::string &param2, std::string *error) {
bool FuzzyHash::init(std::shared_ptr<std::string> param2, std::string *error) {
#ifdef WITH_SSDEEP
std::string digit;
std::string file;

View File

@ -49,7 +49,7 @@ class FuzzyHash : public Operator {
const bpstd::string_view &input,
RuleMessage *ruleMessage) override;
bool init(const std::string &file, std::string *error) override;
bool init(std::shared_ptr<std::string> file, std::string *error) override;
private:
int m_threshold;
struct fuzzy_hash_chunk *m_head;

View File

@ -26,7 +26,7 @@
namespace modsecurity {
namespace operators {
bool InspectFile::init(const std::string &param2, std::string *error) {
bool InspectFile::init(std::shared_ptr<std::string> param2, std::string *error) {
std::istream *iss;
std::string err;
std::string err_lua;

View File

@ -35,7 +35,7 @@ class InspectFile : public Operator {
m_file(""),
m_isScript(false) { }
bool init(const std::string &file, std::string *error) override;
bool init(std::shared_ptr<std::string> file, std::string *error) override;
bool evaluate(Transaction *transaction,
const RuleWithActions *rule,

View File

@ -25,7 +25,7 @@ namespace modsecurity {
namespace operators {
bool IpMatch::init(const std::string &file, std::string *error) {
bool IpMatch::init(std::shared_ptr<std::string> file, std::string *error) {
std::string e("");
bool res = m_tree.addFromBuffer(m_param, &e);

View File

@ -39,7 +39,7 @@ class IpMatch : public Operator {
const bpstd::string_view &input,
RuleMessage *ruleMessage) override;
bool init(const std::string &file, std::string *error) override;
bool init(std::shared_ptr<std::string> file, std::string *error) override;
protected:
Utils::IpTree m_tree;

View File

@ -26,7 +26,7 @@ namespace modsecurity {
namespace operators {
bool IpMatchFromFile::init(const std::string &file,
bool IpMatchFromFile::init(std::shared_ptr<std::string> file,
std::string *error) {
std::string e("");
bool res = false;

View File

@ -31,7 +31,7 @@ class IpMatchFromFile : public IpMatch {
: IpMatch("IpMatchFromFile", std::move(param)) { }
IpMatchFromFile(const std::string &n, std::unique_ptr<RunTimeString> param)
: IpMatch(n, std::move(param)) { }
bool init(const std::string& file, std::string *error) override;
bool init(std::shared_ptr<std::string> file, std::string *error) override;
};
} // namespace operators

View File

@ -104,7 +104,7 @@ class Operator {
virtual ~Operator() { }
static Operator *instantiate(std::string opName, std::string param);
virtual bool init(const std::string &arg, std::string *error) {
virtual bool init(std::shared_ptr<std::string> arg, std::string *error) {
return true;
}

View File

@ -117,7 +117,7 @@ bool Pm::evaluate(Transaction *transaction,
}
bool Pm::init(const std::string &file, std::string *error) {
bool Pm::init(std::shared_ptr<std::string> file, std::string *error) {
std::vector<std::string> vec;
std::istringstream *iss;
const char *err = NULL;

View File

@ -48,7 +48,7 @@ class Pm : public Operator {
RuleMessage *ruleMessage) override;
bool init(const std::string &file, std::string *error) override;
bool init(std::shared_ptr<std::string> file, std::string *error) override;
void postOrderTraversal(acmp_btree_node_t *node);
void cleanup(acmp_node_t *n);

View File

@ -43,7 +43,7 @@ bool PmFromFile::isComment(const std::string &s) {
return true;
}
bool PmFromFile::init(const std::string &config, std::string *error) {
bool PmFromFile::init(std::shared_ptr<std::string> config, std::string *error) {
std::istream *iss;
if (m_param.compare(0, 8, "https://") == 0) {
@ -56,7 +56,7 @@ bool PmFromFile::init(const std::string &config, std::string *error) {
iss = new std::stringstream(client.content);
} else {
std::string err;
std::string resource = utils::find_resource(m_param, config.c_str(), &err);
std::string resource = utils::find_resource(m_param, config.get()->c_str(), &err);
iss = new std::ifstream(resource, std::ios::in);
if (((std::ifstream *)iss)->is_open() == false) {

View File

@ -35,7 +35,7 @@ class PmFromFile : public Pm {
explicit PmFromFile(const std::string &n, std::unique_ptr<RunTimeString> param)
: Pm(n, std::move(param)) { }
bool init(const std::string &file, std::string *error) override;
bool init(std::shared_ptr<std::string> file, std::string *error) override;
private:
static bool isComment(const std::string &s);

View File

@ -29,7 +29,7 @@ namespace modsecurity {
namespace operators {
bool Rx::init(const std::string &file, std::string *error) {
bool Rx::init(std::shared_ptr<std::string> file, std::string *error) {
if (m_string->containsMacro() == false) {
m_re = new Regex(m_param);
}

View File

@ -42,7 +42,7 @@ class Rx : public Operator {
m_couldContainsMacro = true;
}
~Rx() {
virtual ~Rx() {
if (m_string->containsMacro() == false && m_re != NULL) {
delete m_re;
m_re = NULL;
@ -54,7 +54,7 @@ class Rx : public Operator {
const bpstd::string_view &input,
RuleMessage *ruleMessage) override;
bool init(const std::string &file, std::string *error) override;
bool init(std::shared_ptr<std::string> file, std::string *error) override;
private:
Regex *m_re;

View File

@ -27,7 +27,7 @@ namespace modsecurity {
namespace operators {
bool RxGlobal::init(const std::string &arg, std::string *error) {
bool RxGlobal::init(std::shared_ptr<std::string> arg, std::string *error) {
if (m_string->containsMacro() == false) {
m_re = new Regex(m_param);
}

View File

@ -54,7 +54,7 @@ class RxGlobal : public Operator {
const bpstd::string_view& input,
RuleMessage *ruleMessage) override;
bool init(const std::string &arg, std::string *error) override;
bool init(std::shared_ptr<std::string> arg, std::string *error) override;
private:
Regex *m_re;

View File

@ -85,7 +85,7 @@ bool ValidateByteRange::getRange(const bpstd::string_view &rangeRepresentation,
}
bool ValidateByteRange::init(const std::string &file,
bool ValidateByteRange::init(std::shared_ptr<std::string> file,
std::string *error) {
size_t pos = m_param.find_first_of(",");

View File

@ -43,7 +43,7 @@ class ValidateByteRange : public Operator {
RuleMessage *ruleMessage) override;
bool getRange(const bpstd::string_view &rangeRepresentation, std::string *error);
bool init(const std::string& file, std::string *error) override;
bool init(std::shared_ptr<std::string> file, std::string *error) override;
private:
std::vector<std::string> ranges;
char table[32];

View File

@ -25,7 +25,7 @@ namespace modsecurity {
namespace operators {
#ifdef WITH_LIBXML2
bool ValidateDTD::init(const std::string &file, std::string *error) {
bool ValidateDTD::init(std::shared_ptr<std::string> file, std::string *error) {
std::string err;
m_resource = utils::find_resource(m_param, file, &err);
if (m_resource == "") {

View File

@ -64,7 +64,7 @@ class ValidateDTD : public Operator {
const bpstd::string_view &input,
RuleMessage *ruleMessage) override;
bool init(const std::string &file, std::string *error) override;
bool init(std::shared_ptr<std::string> file, std::string *error) override;
static void error_runtime(void *ctx, const char *msg, ...) {

View File

@ -27,7 +27,7 @@ namespace operators {
#ifdef WITH_LIBXML2
bool ValidateSchema::init(const std::string &file, std::string *error) {
bool ValidateSchema::init(std::shared_ptr<std::string> file, std::string *error) {
std::string err;
m_resource = utils::find_resource(m_param, file, &err);
if (m_resource == "") {

View File

@ -46,7 +46,7 @@ class ValidateSchema : public Operator {
const bpstd::string_view &input,
RuleMessage *ruleMessage) override;
bool init(const std::string &file, std::string *error) override;
bool init(std::shared_ptr<std::string> file, std::string *error) override;
static void error_load(void *ctx, const char *msg, ...) {

View File

@ -91,7 +91,7 @@ int VerifyCC::luhnVerify(const char *ccnumber, int len) {
bool VerifyCC::init(const std::string &param2, std::string *error) {
bool VerifyCC::init(std::shared_ptr<std::string> param2, std::string *error) {
const char *errptr = NULL;
int erroffset = 0;

View File

@ -35,7 +35,7 @@ class VerifyCC : public Operator {
m_pce(NULL) { }
~VerifyCC();
bool init(const std::string &param, std::string *error) override;
bool init(std::shared_ptr<std::string> param, std::string *error) override;
bool evaluate(Transaction *transaction,
const RuleWithActions *rule,

View File

@ -38,7 +38,6 @@ CLEANFILES = test.cc \
seclang-scanner.cc \
seclang-parser.cc \
seclang-parser.hh \
location.hh \
position.hh \
stack.hh

View File

@ -34,18 +34,16 @@ Driver::Driver()
Driver::~Driver() {
while (loc.empty() == false) {
yy::location *a = loc.back();
loc.pop_back();
delete a;
while (!m_location.empty()) {
m_location.pop_back();
}
}
int Driver::addSecMarker(std::string marker, std::unique_ptr<std::string> fileName, int lineNumber) {
int Driver::addSecMarker(std::string marker, std::shared_ptr<std::string> fileName, int lineNumber) {
// FIXME: we might move this to the parser.
for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
RuleMarker *r = new RuleMarker(marker, std::move(fileName), lineNumber);
RuleMarker *r = new RuleMarker(marker, fileName, lineNumber);
std::unique_ptr<RuleMarker> rule(std::move(r));
rule->setPhase(i);
m_rulesSetPhases.insert(std::move(rule));
@ -73,7 +71,7 @@ int Driver::addSecRuleScript(std::unique_ptr<RuleScript> rule) {
}
int Driver::addSecRule(std::unique_ptr<RuleWithActions> r) {
int Driver::addSecRule(std::unique_ptr<RuleWithOperator> r) {
if (r->getPhase() >= modsecurity::Phases::NUMBER_OF_PHASES) {
m_parserError << "Unknown phase: " << std::to_string(r->getPhase());
m_parserError << std::endl;
@ -173,7 +171,7 @@ int Driver::addSecRule(std::unique_ptr<RuleWithActions> r) {
return true;
}
std::shared_ptr<RuleWithActions> rule(std::move(r));
std::shared_ptr<RuleWithOperator> rule(std::move(r));
/*
* Checking if the rule has an ID and also checking if this ID is not used
* by other rule
@ -195,11 +193,10 @@ int Driver::addSecRule(std::unique_ptr<RuleWithActions> r) {
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("<<reference missing or not informed>>");
newLocation("<<reference missing or not informed>>");
} else {
loc.back()->begin.filename = loc.back()->end.filename = new std::string(ref);
newLocation(ref);
}
if (f.empty()) {
@ -258,16 +255,16 @@ int Driver::parseFile(const std::string &f) {
}
void Driver::error(const yy::location& l, const std::string& m) {
void Driver::error(const yy::seclang_parser::location_type& l, const std::string& m) {
error(l, m, "");
}
void Driver::error(const yy::location& l, const std::string& m,
void Driver::error(const yy::seclang_parser::location_type& l, const std::string& m,
const std::string& c) {
if (m_parserError.tellp() == 0) {
m_parserError << "Rules error. ";
m_parserError << "File: " << *l.end.filename << ". ";
m_parserError << "File: " << *l.end.filename->get() << ". ";
m_parserError << "Line: " << l.end.line << ". ";
m_parserError << "Column: " << l.end.column - 1 << ". ";
}

View File

@ -64,11 +64,13 @@ typedef struct Driver_t Driver;
class Driver : public RulesSetProperties {
public:
Driver();
Driver(const Driver &d) = delete;
Driver &operator=(const Driver& other) = delete;
virtual ~Driver();
int addSecRule(std::unique_ptr<RuleWithActions> rule);
int addSecRule(std::unique_ptr<RuleWithOperator> rule);
int addSecAction(std::unique_ptr<RuleWithActions> rule);
int addSecMarker(std::string marker, std::unique_ptr<std::string> fileName, int lineNumber);
int addSecMarker(std::string marker, std::shared_ptr<std::string> fileName, int lineNumber);
int addSecRuleScript(std::unique_ptr<RuleScript> rule);
bool scan_begin();
@ -82,16 +84,29 @@ class Driver : public RulesSetProperties {
bool trace_parsing;
void error(const yy::location& l, const std::string& m);
void error(const yy::location& l, const std::string& m,
void error(const yy::seclang_parser::location_type& l, const std::string& m);
void error(const yy::seclang_parser::location_type& l, const std::string& m,
const std::string& c);
std::list<yy::location *> loc;
void newLocation(std::string file) {
m_location.push_back(std::make_shared<yy::seclang_parser::location_type>());
m_location.back()->setFileName(file);
}
yy::seclang_parser::location_type *currentLocation() {
return m_location.back().get();
}
void popLocation() {
m_location.pop_back();
}
std::string buffer;
RuleWithActions *m_lastRule;
RulesSetPhases m_rulesSetPhases;
private:
std::list<std::shared_ptr<yy::seclang_parser::location_type>> m_location;
};

View File

@ -40,6 +40,7 @@
# include <iostream>
# include <string>
# include <memory>
# ifndef YY_NULLPTR
# if defined __cplusplus
@ -61,7 +62,7 @@ namespace yy {
{
public:
/// Type for file name.
typedef const std::string filename_type;
typedef const std::shared_ptr<std::string> filename_type;
/// Type for line and column numbers.
typedef int counter_type;
@ -174,12 +175,14 @@ namespace yy {
location (const position& b, const position& e)
: begin (b)
, end (e)
, m_fileName()
{}
/// Construct a 0-width location in \a p.
explicit location (const position& p = position ())
: begin (p)
, end (p)
, m_fileName()
{}
/// Construct a 0-width location in \a f, \a l, \a c.
@ -188,6 +191,7 @@ namespace yy {
counter_type c = 1)
: begin (f, l, c)
, end (f, l, c)
, m_fileName()
{}
@ -222,12 +226,28 @@ namespace yy {
}
/** \} */
void setFileName(std::string fileName) {
m_fileName = std::make_shared<std::string>(fileName);
begin.filename = &m_fileName;
end.filename = &m_fileName;
}
std::shared_ptr<std::string> getFileName() {
if (!m_fileName) {
if (end.filename) {
m_fileName = *end.filename;
}
}
return m_fileName;
}
public:
/// Beginning of the located region.
position begin;
/// End of the located region.
position end;
private:
std::shared_ptr<std::string> m_fileName;
};
/// Join two locations, in place.

File diff suppressed because it is too large Load Diff

View File

@ -45,17 +45,22 @@
#ifndef YY_YY_SECLANG_PARSER_HH_INCLUDED
# define YY_YY_SECLANG_PARSER_HH_INCLUDED
// "%code requires" blocks.
#line 10 "seclang-parser.yy"
#line 13 "seclang-parser.yy"
#include <string>
#include <iterator>
#include <memory>
#include "location.hh"
namespace ModSecurity {
namespace Parser {
class Driver;
}
}
#include "src/rule_unconditional.h"
#include "src/rule_with_operator.h"
#include "src/rule_with_actions.h"
@ -200,6 +205,7 @@ class Driver;
#include "src/utils/geo_lookup.h"
#include "src/utils/string.h"
#include "src/utils/system.h"
#include "src/variables/variable.h"
#include "src/variables/args_combined_size.h"
#include "src/variables/args_get.h"
#include "src/variables/args_get_names.h"
@ -354,7 +360,7 @@ using namespace modsecurity::operators;
a = std::move(c);
#line 358 "seclang-parser.hh"
#line 364 "seclang-parser.hh"
# include <cassert>
# include <cstdlib> // std::abort
@ -399,7 +405,7 @@ using namespace modsecurity::operators;
#else
# define YY_CONSTEXPR
#endif
# include "location.hh"
#include <typeinfo>
#ifndef YY_ASSERT
# include <cassert>
@ -488,7 +494,7 @@ using namespace modsecurity::operators;
#endif
namespace yy {
#line 492 "seclang-parser.hh"
#line 498 "seclang-parser.hh"
@ -946,7 +952,7 @@ namespace yy {
typedef YYSTYPE semantic_type;
#endif
/// Symbol locations.
typedef location location_type;
typedef yy::location location_type;
/// Syntax errors thrown from user actions.
struct syntax_error : std::runtime_error
@ -8629,7 +8635,7 @@ switch (yykind)
}
} // yy
#line 8633 "seclang-parser.hh"
#line 8639 "seclang-parser.hh"

View File

@ -4,19 +4,27 @@
%define api.parser.class {seclang_parser}
%define api.token.constructor
%define api.value.type variant
//%define api.namespace {modsecurity::yy}
%define parse.assert
//%define api.filename.type { std::shared_ptr<std::string> }
%define api.location.type { yy::location }
%code requires
{
#include <string>
#include <iterator>
#include <memory>
#include "location.hh"
namespace ModSecurity {
namespace Parser {
class Driver;
}
}
#include "src/rule_unconditional.h"
#include "src/rule_with_operator.h"
#include "src/rule_with_actions.h"
@ -161,6 +169,7 @@ class Driver;
#include "src/utils/geo_lookup.h"
#include "src/utils/string.h"
#include "src/utils/system.h"
#include "src/variables/variable.h"
#include "src/variables/args_combined_size.h"
#include "src/variables/args_get.h"
#include "src/variables/args_get_names.h"
@ -321,8 +330,9 @@ using namespace modsecurity::operators;
%initial-action
{
// Initialize the initial location.
@$.begin.filename = @$.end.filename = new std::string(driver.file);
@$.setFileName(driver.file);
};
%define parse.trace
%define parse.error verbose
%code
@ -876,7 +886,7 @@ op:
{
$$ = std::move($1);
std::string error;
if ($$->init(*@1.end.filename, &error) == false) {
if ($$->init(@1.getFileName(), &error) == false) {
driver.error(@0, error);
YYERROR;
}
@ -886,7 +896,7 @@ op:
$$ = std::move($2);
$$->m_negation = true;
std::string error;
if ($$->init(*@1.end.filename, &error) == false) {
if ($$->init(@1.getFileName(), &error) == false) {
driver.error(@0, error);
YYERROR;
}
@ -895,7 +905,7 @@ op:
{
OPERATOR_CONTAINER($$, new operators::Rx(std::move($1)));
std::string error;
if ($$->init(*@1.end.filename, &error) == false) {
if ($$->init(@1.getFileName(), &error) == false) {
driver.error(@0, error);
YYERROR;
}
@ -905,7 +915,7 @@ op:
OPERATOR_CONTAINER($$, new operators::Rx(std::move($2)));
$$->m_negation = true;
std::string error;
if ($$->init(*@1.end.filename, &error) == false) {
if ($$->init(@1.getFileName(), &error) == false) {
driver.error(@0, error);
YYERROR;
}
@ -952,7 +962,7 @@ op_before_init:
| OPERATOR_VALIDATE_HASH run_time_string
{
/* $$ = new operators::ValidateHash($1); */
OPERATOR_NOT_SUPPORTED("ValidateHash", @0);
OPERATOR_NOT_SUPPORTED("ValidateHash", @1);
}
| OPERATOR_VALIDATE_SCHEMA run_time_string
{
@ -977,12 +987,12 @@ op_before_init:
| OPERATOR_GSB_LOOKUP run_time_string
{
/* $$ = new operators::GsbLookup($1); */
OPERATOR_NOT_SUPPORTED("GsbLookup", @0);
OPERATOR_NOT_SUPPORTED("GsbLookup", @1);
}
| OPERATOR_RSUB run_time_string
{
/* $$ = new operators::Rsub($1); */
OPERATOR_NOT_SUPPORTED("Rsub", @0);
OPERATOR_NOT_SUPPORTED("Rsub", @1);
}
| OPERATOR_WITHIN run_time_string
{
@ -1099,7 +1109,7 @@ expression:
/* variables */ v,
/* actions */ a,
/* transformations */ t,
/* file name */ std::unique_ptr<std::string>(new std::string(*@1.end.filename)),
/* file name */ @1.getFileName(),
/* line number */ @1.end.line
));
// TODO: filename should be a shared_ptr.
@ -1119,7 +1129,7 @@ expression:
/* variables */ v,
/* actions */ NULL,
/* transformations */ NULL,
/* file name */ std::unique_ptr<std::string>(new std::string(*@1.end.filename)),
/* file name */ @1.getFileName(),
/* line number */ @1.end.line
));
if (driver.addSecRule(std::move(rule)) == false) {
@ -1142,7 +1152,7 @@ expression:
std::unique_ptr<RuleUnconditional> rule(new RuleUnconditional(
/* actions */ a,
/* transformations */ t,
/* file name */ std::unique_ptr<std::string>(new std::string(*@1.end.filename)),
/* file name */ @1.getFileName(),
/* line number */ @1.end.line
));
driver.addSecAction(std::move(rule));
@ -1165,7 +1175,7 @@ expression:
/* path to script */ $1,
/* actions */ a,
/* transformations */ t,
/* file name */ std::unique_ptr<std::string>(new std::string(*@1.end.filename)),
/* file name */ @1.getFileName(),
/* line number */ @1.end.line
));
@ -1196,12 +1206,12 @@ expression:
if (phase != NULL) {
definedPhase = phase->getPhase();
secRuleDefinedPhase = phase->getSecRulePhase();
delete phase;
} else if (dynamic_cast<actions::ActionAllowedAsSecDefaultAction *>(a.get())
&& !dynamic_cast<actions::transformations::None *>(a.get())) {
checkedActions.push_back(a);
} else {
driver.error(@0, "The action '" + *a->getName() + "' is not suitable to be part of the SecDefaultActions");
delete actions;
YYERROR;
}
}
@ -1210,6 +1220,7 @@ expression:
}
if (hasDisruptive == false) {
driver.error(@0, "SecDefaultAction must specify a disruptive action.");
delete actions;
YYERROR;
}
if (!driver.m_rulesSetPhases[definedPhase]->m_defaultActions.empty()) {
@ -1218,6 +1229,7 @@ expression:
ss << secRuleDefinedPhase;
ss << " was informed already.";
driver.error(@0, ss.str());
delete actions;
YYERROR;
}
for (auto &a : checkedActions) {
@ -1228,12 +1240,12 @@ expression:
driver.m_rulesSetPhases[definedPhase]->m_defaultActions.push_back(a);
}
}
//delete actions;
delete actions;
}
| CONFIG_DIR_SEC_MARKER
{
driver.addSecMarker(modsecurity::utils::string::removeBracketsIfNeeded($1),
/* file name */ std::unique_ptr<std::string>(new std::string(*@1.end.filename)),
/* file name */ @1.getFileName(),
/* line number */ @1.end.line
);
}
@ -1562,7 +1574,7 @@ expression:
#if defined(WITH_GEOIP) or defined(WITH_MAXMIND)
std::string err;
std::string file = modsecurity::utils::find_resource($1,
*@1.end.filename, &err);
@1.getFileName(), &err);
if (file.empty()) {
std::stringstream ss;
ss << "Failed to load locate the GeoDB file from: " << $1 << " ";
@ -1749,7 +1761,7 @@ expression:
param.pop_back();
}
file = modsecurity::utils::find_resource(f, *@1.end.filename, &err);
file = modsecurity::utils::find_resource(f, @1.getFileName(), &err);
if (file.empty()) {
std::stringstream ss;
ss << "Failed to locate the unicode map file from: " << f << " ";
@ -2606,7 +2618,7 @@ act:
}
| ACTION_APPEND
{
ACTION_NOT_SUPPORTED("Append", @0);
ACTION_NOT_SUPPORTED("Append", @1);
}
| ACTION_AUDIT_LOG
{
@ -2626,18 +2638,15 @@ act:
}
| ACTION_CTL_AUDIT_ENGINE CONFIG_VALUE_ON
{
ACTION_NOT_SUPPORTED("CtlAuditEngine", @0);
//ACTION_CONTAINER($$, new actions::Action($1));
ACTION_NOT_SUPPORTED("ctl:auditEngine", @1);
}
| ACTION_CTL_AUDIT_ENGINE CONFIG_VALUE_OFF
{
ACTION_NOT_SUPPORTED("CtlAuditEngine", @0);
//ACTION_CONTAINER($$, new actions::Action($1));
ACTION_NOT_SUPPORTED("ctl:auditEngine", @1);
}
| ACTION_CTL_AUDIT_ENGINE CONFIG_VALUE_RELEVANT_ONLY
{
ACTION_NOT_SUPPORTED("CtlAuditEngine", @0);
//ACTION_CONTAINER($$, new actions::Action($1));
ACTION_NOT_SUPPORTED("ctl:auditEngine", @1);
}
| ACTION_CTL_AUDIT_LOG_PARTS
{
@ -2657,13 +2666,11 @@ act:
}
| ACTION_CTL_FORCE_REQ_BODY_VAR CONFIG_VALUE_ON
{
ACTION_NOT_SUPPORTED("CtlForceRequestBodyVariable", @0);
//ACTION_CONTAINER($$, new actions::Action($1));
ACTION_NOT_SUPPORTED("ctl:forceRequestBodyVariable", @1);
}
| ACTION_CTL_FORCE_REQ_BODY_VAR CONFIG_VALUE_OFF
{
ACTION_NOT_SUPPORTED("CtlForceRequestBodyVariable", @0);
//ACTION_CONTAINER($$, new actions::Action($1));
ACTION_NOT_SUPPORTED("ctl:forceRequestBodyVariable", @1);
}
| ACTION_CTL_REQUEST_BODY_ACCESS CONFIG_VALUE_ON
{
@ -2707,7 +2714,7 @@ act:
}
| ACTION_DEPRECATE_VAR
{
ACTION_NOT_SUPPORTED("DeprecateVar", @0);
ACTION_NOT_SUPPORTED("DeprecateVar", @1);
}
| ACTION_DROP
{
@ -2763,7 +2770,7 @@ act:
}
| ACTION_PAUSE
{
ACTION_NOT_SUPPORTED("Pause", @0);
ACTION_NOT_SUPPORTED("Pause", @1);
}
| ACTION_PHASE
{
@ -2771,11 +2778,11 @@ act:
}
| ACTION_PREPEND
{
ACTION_NOT_SUPPORTED("Prepend", @0);
ACTION_NOT_SUPPORTED("Prepend", @1);
}
| ACTION_PROXY
{
ACTION_NOT_SUPPORTED("Proxy", @0);
ACTION_NOT_SUPPORTED("Proxy", @1);
}
| ACTION_REDIRECT run_time_string
{
@ -2787,23 +2794,23 @@ act:
}
| ACTION_SANITISE_ARG
{
ACTION_NOT_SUPPORTED("SanitiseArg", @0);
ACTION_NOT_SUPPORTED("SanitiseArg", @1);
}
| ACTION_SANITISE_MATCHED
{
ACTION_NOT_SUPPORTED("SanitiseMatched", @0);
ACTION_NOT_SUPPORTED("SanitiseMatched", @1);
}
| ACTION_SANITISE_MATCHED_BYTES
{
ACTION_NOT_SUPPORTED("SanitiseMatchedBytes", @0);
ACTION_NOT_SUPPORTED("SanitiseMatchedBytes", @1);
}
| ACTION_SANITISE_REQUEST_HEADER
{
ACTION_NOT_SUPPORTED("SanitiseRequestHeader", @0);
ACTION_NOT_SUPPORTED("SanitiseRequestHeader", @1);
}
| ACTION_SANITISE_RESPONSE_HEADER
{
ACTION_NOT_SUPPORTED("SanitiseResponseHeader", @0);
ACTION_NOT_SUPPORTED("SanitiseResponseHeader", @1);
}
| ACTION_SETENV run_time_string
{

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@ -39,19 +39,19 @@ class RuleMarker : public Rule {
public:
RuleMarker(
const std::string &name,
std::unique_ptr<std::string> fileName,
std::shared_ptr<std::string> fileName,
int lineNumber)
: Rule(std::move(fileName), lineNumber),
: Rule(fileName, lineNumber),
m_name(std::make_shared<std::string>(name)) { }
RuleMarker(RuleMarker &&r) :
Rule(r),
m_name(std::move(r.m_name))
m_name(r.m_name)
{ };
RuleMarker(const RuleMarker &r) :
Rule(r),
m_name(std::move(r.m_name))
m_name(r.m_name)
{ };
virtual bool evaluate(Transaction *transaction) const override {

View File

@ -49,9 +49,9 @@ class RuleScript : public RuleWithActions {
RuleScript(const std::string &name,
Actions *actions,
Transformations *t,
std::unique_ptr<std::string> fileName,
std::shared_ptr<std::string> fileName,
int lineNumber)
: RuleWithActions(actions, t, std::move(fileName), lineNumber),
: RuleWithActions(actions, t, fileName, lineNumber),
m_name(name),
m_lua(std::unique_ptr<engine::Lua>(new engine::Lua())) { }

View File

@ -42,9 +42,9 @@ class RuleUnconditional : public RuleWithActions {
RuleUnconditional(
Actions *actions,
Transformations *transformations,
std::unique_ptr<std::string> fileName,
std::shared_ptr<std::string> fileName,
int lineNumber)
: RuleWithActions(actions, transformations, std::move(fileName), lineNumber) { }
: RuleWithActions(actions, transformations, fileName, lineNumber) { }
RuleUnconditional(const RuleUnconditional &r)
: RuleWithActions(r)

View File

@ -63,9 +63,9 @@ namespace modsecurity {
RuleWithActions::RuleWithActions(
Actions *actions,
Transformations *transformations,
std::unique_ptr<std::string> fileName,
std::shared_ptr<std::string> fileName,
int lineNumber)
: Rule(std::move(fileName), lineNumber),
: Rule(fileName, lineNumber),
RuleWithActionsProperties(transformations),
m_ruleId(0),
m_chainedRuleChild(nullptr),
@ -96,6 +96,7 @@ RuleWithActions::RuleWithActions(
}
newActions.push_back(a);
}
delete actions;
}

View File

@ -79,12 +79,15 @@ class RuleWithActions : public Rule, public RuleWithActionsProperties {
RuleWithActions(
Actions *a,
Transformations *t,
std::unique_ptr<std::string> fileName,
std::shared_ptr<std::string> fileName,
int lineNumber);
RuleWithActions(const RuleWithActions &r);
RuleWithActions &operator=(const RuleWithActions& r);
virtual ~RuleWithActions()
{ };
virtual bool evaluate(Transaction *transaction) const override;
void executeActionsIndependentOfChainedRuleResult(

View File

@ -44,8 +44,15 @@ RuleWithActionsProperties::RuleWithActionsProperties(Transformations *transforma
m_setVars(),
m_disruptiveAction(nullptr),
m_tags(),
m_transformations(transformations != nullptr ? *transformations : Transformations())
{ }
m_transformations()
{
if (transformations != nullptr) {
for (auto &t : *transformations) {
m_transformations.push_back(t);
}
}
delete transformations;
}

View File

@ -58,7 +58,7 @@ RuleWithOperator::RuleWithOperator(Operator *op,
variables::Variables *_variables,
Actions *actions,
Transformations *transformations,
std::unique_ptr<std::string> fileName,
std::shared_ptr<std::string> fileName,
int lineNumber)
: RuleWithActions(actions, transformations, std::move(fileName), lineNumber),
m_variables(std::unique_ptr<variables::Variables>(_variables)),
@ -74,8 +74,8 @@ RuleWithOperator::RuleWithOperator(Operator *op,
RuleWithOperator::~RuleWithOperator() {
}
RuleWithOperator::~RuleWithOperator()
{ }
void RuleWithOperator::updateMatchedVars(Transaction *trans,
@ -223,9 +223,9 @@ bool RuleWithOperator::evaluate(Transaction *trans) const {
variables::Variables *variables = m_variables.get();
bool recursiveGlobalRet;
std::string eparam;
variables::Variables vars;
variables::Variables vars(false);
vars.reserve(4);
variables::Variables exclusion;
variables::Variables exclusion(false);
RuleWithActions::evaluate(trans);
@ -278,6 +278,7 @@ bool RuleWithOperator::evaluate(Transaction *trans) const {
continue;
}
e.clear();
var->evaluate(trans, &e);
for (const auto &vv : e) {
TransformationsResults transformationsResults;

View File

@ -45,7 +45,7 @@ class RuleWithOperator : public RuleWithActions {
variables::Variables *variables,
Actions *actions,
Transformations *transformations,
std::unique_ptr<std::string> fileName,
std::shared_ptr<std::string> fileName,
int lineNumber);
RuleWithOperator(const RuleWithOperator &op)
@ -95,7 +95,7 @@ class RuleWithOperator : public RuleWithActions {
virtual void dump(std::stringstream &out) const override {
Rule::dump(out);
out << "# RuleWithOperator" << std::endl;
out << "# RuleWithOperator (op counter: " << m_operator.use_count() << ")" << std::endl;
out << "SecRule ";
out << m_variables->getVariableNames() << " ";
out << "\"" << "@" << m_operator->m_op << " " << m_operator->m_param << "\"";

View File

@ -62,6 +62,8 @@ class RunTimeString {
return *this;
}
~RunTimeString()
{ }
void append(const std::string &text);
void append(std::unique_ptr<Variable> var);

View File

@ -62,6 +62,11 @@ double cpu_seconds(void) {
}
std::string find_resource(const std::string& file, std::shared_ptr<std::string> config,
std::string *err) {
return find_resource(file, *config.get(), err);
}
std::string find_resource(const std::string& resource,
const std::string& config, std::string *err) {
std::ifstream *iss;

View File

@ -31,6 +31,10 @@ namespace utils {
double cpu_seconds(void);
std::string find_resource(const std::string& file, const std::string& config,
std::string *err);
std::string find_resource(const std::string& file, std::shared_ptr<std::string> config,
std::string *err);
std::string get_path(const std::string& file);
std::list<std::string> expandEnv(const std::string& var, int flags);
bool createDir(std::string dir, int mode, std::string *error);

View File

@ -711,6 +711,24 @@ class VariableRegex : public Variable {
class Variables : public std::vector<Variable *> {
public:
explicit Variables(bool cleanup = true)
: m_cleanup(cleanup)
{ }
Variables(const Variable &a) = delete;
Variables& operator=(const Variables& other) = delete;
~Variables() {
if (!m_cleanup) {
return;
}
for (auto *a : *this) {
delete a;
}
}
bool contains(Variable *v) {
return std::find_if(begin(), end(),
[v](Variable *m) -> bool { return *v == *m; }) != end();
@ -739,6 +757,8 @@ class Variables : public std::vector<Variable *> {
}
return names;
}
private:
bool m_cleanup;
};

View File

@ -174,6 +174,7 @@ modsecurity::RulesSet *setupModSecRules(RegressionTestResult *r) {
auto s = rules->getParserError();
if (regex_search(s, &match, re)) {
r->passed();
delete rules;
return nullptr;
}
}
@ -187,6 +188,7 @@ modsecurity::RulesSet *setupModSecRules(RegressionTestResult *r) {
reason << KWHT << "Expected: " << RESET << r->getExpectedParserError() << std::endl;
reason << KWHT << "Produced: " << RESET << rules->getParserError() << std::endl;
r->failed(reason.str());
delete rules;
return nullptr;
}
@ -278,6 +280,8 @@ void processRequest(
CustomDebugLog *d = reinterpret_cast<CustomDebugLog *>(rules->m_debugLog);
debug_log.assign(d->log_messages());
delete modsec_transaction;
*status_code = r.status;
}
@ -398,9 +402,9 @@ int main(int argc, char **argv) {
ModSecurityTest<RegressionTest> test;
std::string ver(MODSECURITY_VERSION);
std::string envvar("MODSECURITY=ModSecurity " + ver + " regression tests");
std::string envvar("ModSecurity " + ver + " regression tests");
putenv(strdup(envvar.c_str()));
setenv("MODSECURITY", envvar.c_str(), 0);
#ifndef NO_LOGS
int test_number = 0;
#endif

View File

@ -77,7 +77,8 @@ void perform_unit_test(ModSecurityTest<UnitTest> *test, UnitTest *t,
if (t->type == "op") {
Operator *op = Operator::instantiate(t->name, t->param);
op->init(t->filename, &error);
std::shared_ptr<std::string> fileName = std::make_shared<std::string>(t->filename);
op->init(fileName, &error);
int ret = op->evaluate(NULL, NULL, t->input, NULL);
t->obtained = ret;
if (ret != t->ret) {

View File

@ -41,7 +41,7 @@ int main(int argc, char **argv) {
if (*args == NULL) {
print_help(argv[0]);
return 0;
goto ret;
}
while (*args != NULL) {
@ -90,19 +90,22 @@ int main(int argc, char **argv) {
std::cout << "Loaded " << std::to_string(r) << " rules." << std::endl;
if (err.empty() == false) {
std::cerr << " " << err << std::endl;
goto end;
}
rules->dump();
next:
args++;
}
delete rules;
end:
if (ret < 0) {
std::cout << "Test failed." << std::endl;
} else {
std::cout << "Test ok." << std::endl;
}
ret:
delete rules;
return ret;
}