From 61c11251b686c10d7886d96ee6d524b95cfff890 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Tue, 23 Apr 2019 13:09:22 -0300 Subject: [PATCH] parser: Fix filename --- CHANGES | 2 ++ src/parser/driver.cc | 8 +++--- src/parser/driver.h | 10 ++++++- src/parser/seclang-parser.cc | 22 ++++++++-------- src/parser/seclang-parser.yy | 22 ++++++++-------- src/parser/seclang-scanner.cc | 49 ++++++++++++++++------------------- src/parser/seclang-scanner.ll | 35 ++++++++++++------------- 7 files changed, 75 insertions(+), 73 deletions(-) diff --git a/CHANGES b/CHANGES index 3861ef1b..6fed5953 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ v3.0.4 - YYYY-MMM-DD (to be released) ------------------------------------- + - parser: fix parsed file names + [@zimmerle] - Allow empty anchored variable [Issue #2024 - @airween] - Fixed FILES_NAMES collection after the end of multipart parsing diff --git a/src/parser/driver.cc b/src/parser/driver.cc index f4b722ac..f9a729a0 100644 --- a/src/parser/driver.cc +++ b/src/parser/driver.cc @@ -138,9 +138,9 @@ int Driver::parse(const std::string &f, const std::string &ref) { lastRule = NULL; loc.push_back(new yy::location()); if (ref.empty()) { - this->ref.push_back("<>"); + loc.back()->begin.filename = loc.back()->end.filename = new std::string("<>"); } else { - this->ref.push_back(ref); + loc.back()->begin.filename = loc.back()->end.filename = new std::string(ref); } if (f.empty()) { @@ -195,9 +195,7 @@ void Driver::error(const yy::location& l, const std::string& m, const std::string& c) { if (m_parserError.tellp() == 0) { m_parserError << "Rules error. "; - if (ref.empty() == false) { - m_parserError << "File: " << ref.back() << ". "; - } + m_parserError << "File: " << *l.end.filename << ". "; m_parserError << "Line: " << l.end.line << ". "; m_parserError << "Column: " << l.end.column - 1 << ". "; } diff --git a/src/parser/driver.h b/src/parser/driver.h index 2d9cfc7a..32d51fee 100644 --- a/src/parser/driver.h +++ b/src/parser/driver.h @@ -21,6 +21,7 @@ #include #endif + #ifndef SRC_PARSER_DRIVER_H_ #define SRC_PARSER_DRIVER_H_ @@ -50,6 +51,14 @@ typedef struct Driver_t Driver; #endif +/** + * + * FIXME: There is a memory leak in the filename at yy::location. + * The filename should be converted into a shared string to + * save memory or be associated with the life cycle of the + * driver class. + * + **/ class Driver : public RulesProperties { public: Driver(); @@ -77,7 +86,6 @@ class Driver : public RulesProperties { std::list loc; - std::list ref; std::string buffer; Rule *lastRule; }; diff --git a/src/parser/seclang-parser.cc b/src/parser/seclang-parser.cc index 7049eacd..178e1abe 100644 --- a/src/parser/seclang-parser.cc +++ b/src/parser/seclang-parser.cc @@ -1116,7 +1116,7 @@ namespace yy { #line 316 "seclang-parser.yy" // lalr1.cc:788 { // Initialize the initial location. - yyla.location.begin.filename = yyla.location.end.filename = &driver.file; + yyla.location.begin.filename = yyla.location.end.filename = new std::string(driver.file); } #line 1123 "seclang-parser.cc" // lalr1.cc:788 @@ -1707,7 +1707,7 @@ namespace yy { { yylhs.value.as < std::unique_ptr > () = std::move(yystack_[0].value.as < std::unique_ptr > ()); std::string error; - if (yylhs.value.as < std::unique_ptr > ()->init(driver.ref.back(), &error) == false) { + if (yylhs.value.as < std::unique_ptr > ()->init(*yystack_[0].location.end.filename, &error) == false) { driver.error(yystack_[1].location, error); YYERROR; } @@ -1721,7 +1721,7 @@ namespace yy { yylhs.value.as < std::unique_ptr > () = std::move(yystack_[0].value.as < std::unique_ptr > ()); yylhs.value.as < std::unique_ptr > ()->m_negation = true; std::string error; - if (yylhs.value.as < std::unique_ptr > ()->init(driver.ref.back(), &error) == false) { + if (yylhs.value.as < std::unique_ptr > ()->init(*yystack_[1].location.end.filename, &error) == false) { driver.error(yystack_[2].location, error); YYERROR; } @@ -1734,7 +1734,7 @@ namespace yy { { OPERATOR_CONTAINER(yylhs.value.as < std::unique_ptr > (), new operators::Rx(std::move(yystack_[0].value.as < std::unique_ptr > ()))); std::string error; - if (yylhs.value.as < std::unique_ptr > ()->init(driver.ref.back(), &error) == false) { + if (yylhs.value.as < std::unique_ptr > ()->init(*yystack_[0].location.end.filename, &error) == false) { driver.error(yystack_[1].location, error); YYERROR; } @@ -1748,7 +1748,7 @@ namespace yy { OPERATOR_CONTAINER(yylhs.value.as < std::unique_ptr > (), new operators::Rx(std::move(yystack_[0].value.as < std::unique_ptr > ()))); yylhs.value.as < std::unique_ptr > ()->m_negation = true; std::string error; - if (yylhs.value.as < std::unique_ptr > ()->init(driver.ref.back(), &error) == false) { + if (yylhs.value.as < std::unique_ptr > ()->init(*yystack_[1].location.end.filename, &error) == false) { driver.error(yystack_[2].location, error); YYERROR; } @@ -2063,7 +2063,7 @@ namespace yy { /* op */ op, /* variables */ v, /* actions */ a, - /* file name */ driver.ref.back(), + /* file name */ *yystack_[3].location.end.filename, /* line number */ yystack_[3].location.end.line ); @@ -2087,7 +2087,7 @@ namespace yy { /* op */ yystack_[0].value.as < std::unique_ptr > ().release(), /* variables */ v, /* actions */ NULL, - /* file name */ driver.ref.back(), + /* file name */ *yystack_[2].location.end.filename, /* line number */ yystack_[2].location.end.line ); if (driver.addSecRule(rule) == false) { @@ -2109,7 +2109,7 @@ namespace yy { /* op */ NULL, /* variables */ NULL, /* actions */ a, - /* file name */ driver.ref.back(), + /* file name */ *yystack_[1].location.end.filename, /* line number */ yystack_[1].location.end.line ); driver.addSecAction(rule); @@ -2128,7 +2128,7 @@ namespace yy { RuleScript *r = new RuleScript( /* path to script */ yystack_[1].value.as < std::string > (), /* actions */ a, - /* file name */ driver.ref.back(), + /* file name */ *yystack_[1].location.end.filename, /* line number */ yystack_[1].location.end.line ); @@ -2721,7 +2721,7 @@ namespace yy { #if defined(WITH_GEOIP) or defined(WITH_MAXMIND) std::string err; std::string file = modsecurity::utils::find_resource(yystack_[0].value.as < std::string > (), - driver.ref.back(), &err); + *yystack_[0].location.end.filename, &err); if (file.empty()) { std::stringstream ss; ss << "Failed to load locate the GeoDB file from: " << yystack_[0].value.as < std::string > () << " "; @@ -2949,7 +2949,7 @@ namespace yy { param.pop_back(); } - file = modsecurity::utils::find_resource(f, driver.ref.back(), &err); + file = modsecurity::utils::find_resource(f, *yystack_[0].location.end.filename, &err); if (file.empty()) { std::stringstream ss; ss << "Failed to locate the unicode map file from: " << f << " "; diff --git a/src/parser/seclang-parser.yy b/src/parser/seclang-parser.yy index 369b3ed8..24aaa836 100644 --- a/src/parser/seclang-parser.yy +++ b/src/parser/seclang-parser.yy @@ -315,7 +315,7 @@ using namespace modsecurity::operators; %initial-action { // Initialize the initial location. - @$.begin.filename = @$.end.filename = &driver.file; + @$.begin.filename = @$.end.filename = new std::string(driver.file); }; %define parse.trace %define parse.error verbose @@ -866,7 +866,7 @@ op: { $$ = std::move($1); std::string error; - if ($$->init(driver.ref.back(), &error) == false) { + if ($$->init(*@1.end.filename, &error) == false) { driver.error(@0, error); YYERROR; } @@ -876,7 +876,7 @@ op: $$ = std::move($2); $$->m_negation = true; std::string error; - if ($$->init(driver.ref.back(), &error) == false) { + if ($$->init(*@1.end.filename, &error) == false) { driver.error(@0, error); YYERROR; } @@ -885,7 +885,7 @@ op: { OPERATOR_CONTAINER($$, new operators::Rx(std::move($1))); std::string error; - if ($$->init(driver.ref.back(), &error) == false) { + if ($$->init(*@1.end.filename, &error) == false) { driver.error(@0, error); YYERROR; } @@ -895,7 +895,7 @@ op: OPERATOR_CONTAINER($$, new operators::Rx(std::move($2))); $$->m_negation = true; std::string error; - if ($$->init(driver.ref.back(), &error) == false) { + if ($$->init(*@1.end.filename, &error) == false) { driver.error(@0, error); YYERROR; } @@ -1073,7 +1073,7 @@ expression: /* op */ op, /* variables */ v, /* actions */ a, - /* file name */ driver.ref.back(), + /* file name */ *@1.end.filename, /* line number */ @1.end.line ); @@ -1093,7 +1093,7 @@ expression: /* op */ $3.release(), /* variables */ v, /* actions */ NULL, - /* file name */ driver.ref.back(), + /* file name */ *@1.end.filename, /* line number */ @1.end.line ); if (driver.addSecRule(rule) == false) { @@ -1111,7 +1111,7 @@ expression: /* op */ NULL, /* variables */ NULL, /* actions */ a, - /* file name */ driver.ref.back(), + /* file name */ *@1.end.filename, /* line number */ @1.end.line ); driver.addSecAction(rule); @@ -1126,7 +1126,7 @@ expression: RuleScript *r = new RuleScript( /* path to script */ $1, /* actions */ a, - /* file name */ driver.ref.back(), + /* file name */ *@1.end.filename, /* line number */ @1.end.line ); @@ -1525,7 +1525,7 @@ expression: #if defined(WITH_GEOIP) or defined(WITH_MAXMIND) std::string err; std::string file = modsecurity::utils::find_resource($1, - driver.ref.back(), &err); + *@1.end.filename, &err); if (file.empty()) { std::stringstream ss; ss << "Failed to load locate the GeoDB file from: " << $1 << " "; @@ -1707,7 +1707,7 @@ expression: param.pop_back(); } - file = modsecurity::utils::find_resource(f, driver.ref.back(), &err); + file = modsecurity::utils::find_resource(f, *@1.end.filename, &err); if (file.empty()) { std::stringstream ss; ss << "Failed to locate the unicode map file from: " << f << " "; diff --git a/src/parser/seclang-scanner.cc b/src/parser/seclang-scanner.cc index bfaff535..b5d2fe31 100644 --- a/src/parser/seclang-scanner.cc +++ b/src/parser/seclang-scanner.cc @@ -5053,7 +5053,7 @@ static const flex_int16_t yy_rule_linenum[536] = 1156, 1161, 1163, 1164, 1165, 1166, 1168, 1169, 1170, 1171, 1173, 1174, 1175, 1176, 1178, 1180, 1181, 1183, 1184, 1185, 1186, 1188, 1193, 1194, 1195, 1199, 1200, 1201, 1206, 1208, - 1209, 1210, 1235, 1261, 1289 + 1209, 1210, 1229, 1256, 1286 } ; /* The intent behind this definition is that it'll catch @@ -8419,16 +8419,6 @@ case YY_STATE_EOF(SETVAR_ACTION_QUOTED_WAITING_OPERATION): case YY_STATE_EOF(SETVAR_ACTION_QUOTED_WAITING_CONTENT): #line 1213 "seclang-scanner.ll" { - if (driver.ref.size() > 1) { - driver.ref.pop_back(); - } - - if (driver.loc.size() > 1) { - yy::location *l = driver.loc.back(); - driver.loc.pop_back(); - delete l; - } - if (yyin) { fclose(yyin); } @@ -8437,15 +8427,19 @@ case YY_STATE_EOF(SETVAR_ACTION_QUOTED_WAITING_CONTENT): if (!YY_CURRENT_BUFFER) { return p::make_END(*driver.loc.back()); } + + yy::location *l = driver.loc.back(); + driver.loc.pop_back(); + delete l; } YY_BREAK case 533: YY_RULE_SETUP -#line 1235 "seclang-scanner.ll" +#line 1229 "seclang-scanner.ll" { std::string err; const char *file = strchr(yytext, ' ') + 1; - std::string fi = modsecurity::utils::find_resource(file, driver.ref.back(), &err); + std::string fi = modsecurity::utils::find_resource(file, *driver.loc.back()->end.filename, &err); if (fi.empty() == true) { BEGIN(INITIAL); driver.error (*driver.loc.back(), "", file + std::string(": Not able to open file. ") + err); @@ -8455,28 +8449,29 @@ YY_RULE_SETUP files.reverse(); for (auto& s: files) { std::string err; - std::string f = modsecurity::utils::find_resource(s, driver.ref.back(), &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); yyin = fopen(f.c_str(), "r" ); if (!yyin) { BEGIN(INITIAL); + driver.loc.pop_back(); driver.error (*driver.loc.back(), "", s + std::string(": Not able to open file. ") + err); throw p::syntax_error(*driver.loc.back(), ""); } - driver.ref.push_back(f); - driver.loc.push_back(new yy::location()); yypush_buffer_state(yy_create_buffer( yyin, YY_BUF_SIZE )); } } YY_BREAK case 534: YY_RULE_SETUP -#line 1261 "seclang-scanner.ll" +#line 1256 "seclang-scanner.ll" { std::string err; const char *file = strchr(yytext, ' ') + 1; char *f = strdup(file + 1); f[strlen(f)-1] = '\0'; - std::string fi = modsecurity::utils::find_resource(f, driver.ref.back(), &err); + std::string fi = modsecurity::utils::find_resource(f, *driver.loc.back()->end.filename, &err); if (fi.empty() == true) { BEGIN(INITIAL); driver.error (*driver.loc.back(), "", file + std::string(": Not able to open file. ") + err); @@ -8485,15 +8480,17 @@ YY_RULE_SETUP std::list files = modsecurity::utils::expandEnv(fi, 0); files.reverse(); for (auto& s: files) { - std::string f = modsecurity::utils::find_resource(s, driver.ref.back(), &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); + yyin = fopen(f.c_str(), "r" ); if (!yyin) { BEGIN(INITIAL); + driver.loc.pop_back(); driver.error (*driver.loc.back(), "", s + std::string(": Not able to open file. ") + err); throw p::syntax_error(*driver.loc.back(), ""); } - driver.ref.push_back(f.c_str()); - driver.loc.push_back(new yy::location()); yypush_buffer_state(yy_create_buffer( yyin, YY_BUF_SIZE )); } free(f); @@ -8502,7 +8499,7 @@ YY_RULE_SETUP case 535: /* rule 535 can match eol */ YY_RULE_SETUP -#line 1289 "seclang-scanner.ll" +#line 1286 "seclang-scanner.ll" { HttpsClient c; std::string key; @@ -8517,8 +8514,8 @@ YY_RULE_SETUP url = conf[2]; c.setKey(key); - driver.ref.push_back(url); driver.loc.push_back(new yy::location()); + driver.loc.back()->begin.filename = driver.loc.back()->end.filename = new std::string(url); YY_BUFFER_STATE temp = YY_CURRENT_BUFFER; yypush_buffer_state(temp); @@ -8540,10 +8537,10 @@ YY_RULE_SETUP YY_BREAK case 536: YY_RULE_SETUP -#line 1325 "seclang-scanner.ll" +#line 1322 "seclang-scanner.ll" ECHO; YY_BREAK -#line 8546 "seclang-scanner.cc" +#line 8543 "seclang-scanner.cc" case YY_END_OF_BUFFER: { @@ -9648,7 +9645,7 @@ void yyfree (void * ptr ) /* %ok-for-header */ -#line 1325 "seclang-scanner.ll" +#line 1322 "seclang-scanner.ll" namespace modsecurity { diff --git a/src/parser/seclang-scanner.ll b/src/parser/seclang-scanner.ll index d438def2..c8a64cba 100755 --- a/src/parser/seclang-scanner.ll +++ b/src/parser/seclang-scanner.ll @@ -1210,16 +1210,6 @@ EQUALS_MINUS (?i:=\-) <> { - if (driver.ref.size() > 1) { - driver.ref.pop_back(); - } - - if (driver.loc.size() > 1) { - yy::location *l = driver.loc.back(); - driver.loc.pop_back(); - delete l; - } - if (yyin) { fclose(yyin); } @@ -1228,13 +1218,17 @@ EQUALS_MINUS (?i:=\-) if (!YY_CURRENT_BUFFER) { return p::make_END(*driver.loc.back()); } + + yy::location *l = driver.loc.back(); + driver.loc.pop_back(); + delete l; } {CONFIG_INCLUDE}[ \t]+{CONFIG_VALUE_PATH} { std::string err; const char *file = strchr(yytext, ' ') + 1; - std::string fi = modsecurity::utils::find_resource(file, driver.ref.back(), &err); + std::string fi = modsecurity::utils::find_resource(file, *driver.loc.back()->end.filename, &err); if (fi.empty() == true) { BEGIN(INITIAL); driver.error (*driver.loc.back(), "", file + std::string(": Not able to open file. ") + err); @@ -1244,15 +1238,16 @@ EQUALS_MINUS (?i:=\-) files.reverse(); for (auto& s: files) { std::string err; - std::string f = modsecurity::utils::find_resource(s, driver.ref.back(), &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); yyin = fopen(f.c_str(), "r" ); if (!yyin) { BEGIN(INITIAL); + driver.loc.pop_back(); driver.error (*driver.loc.back(), "", s + std::string(": Not able to open file. ") + err); throw p::syntax_error(*driver.loc.back(), ""); } - driver.ref.push_back(f); - driver.loc.push_back(new yy::location()); yypush_buffer_state(yy_create_buffer( yyin, YY_BUF_SIZE )); } } @@ -1262,7 +1257,7 @@ EQUALS_MINUS (?i:=\-) const char *file = strchr(yytext, ' ') + 1; char *f = strdup(file + 1); f[strlen(f)-1] = '\0'; - std::string fi = modsecurity::utils::find_resource(f, driver.ref.back(), &err); + std::string fi = modsecurity::utils::find_resource(f, *driver.loc.back()->end.filename, &err); if (fi.empty() == true) { BEGIN(INITIAL); driver.error (*driver.loc.back(), "", file + std::string(": Not able to open file. ") + err); @@ -1271,15 +1266,17 @@ EQUALS_MINUS (?i:=\-) std::list files = modsecurity::utils::expandEnv(fi, 0); files.reverse(); for (auto& s: files) { - std::string f = modsecurity::utils::find_resource(s, driver.ref.back(), &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); + yyin = fopen(f.c_str(), "r" ); if (!yyin) { BEGIN(INITIAL); + driver.loc.pop_back(); driver.error (*driver.loc.back(), "", s + std::string(": Not able to open file. ") + err); throw p::syntax_error(*driver.loc.back(), ""); } - driver.ref.push_back(f.c_str()); - driver.loc.push_back(new yy::location()); yypush_buffer_state(yy_create_buffer( yyin, YY_BUF_SIZE )); } free(f); @@ -1299,8 +1296,8 @@ EQUALS_MINUS (?i:=\-) url = conf[2]; c.setKey(key); - driver.ref.push_back(url); driver.loc.push_back(new yy::location()); + driver.loc.back()->begin.filename = driver.loc.back()->end.filename = new std::string(url); YY_BUFFER_STATE temp = YY_CURRENT_BUFFER; yypush_buffer_state(temp);