From 8255ce86ca61e077f83549879afe34610c83e3b7 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 30 Sep 2015 14:35:08 -0300 Subject: [PATCH] Adds reference to filename and line number to lexer errors --- src/actions/phase.cc | 10 ++++++++ src/actions/phase.h | 1 + src/parser/seclang-parser.yy | 41 +++++++++++++++++++++++++-------- src/parser/seclang-scanner.ll | 4 ++-- test/benchmark/basic_rules.conf | 1 + 5 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/actions/phase.cc b/src/actions/phase.cc index 84551c72..63ecd3a8 100644 --- a/src/actions/phase.cc +++ b/src/actions/phase.cc @@ -68,6 +68,16 @@ Phase::Phase(std::string action) } } + +bool Phase::init(std::string *error) { + if (phase >= ModSecurity::Phases::NUMBER_OF_PHASES) { + error->assign("Unknown phase: " + std::to_string(phase)); + return false; + } + return true; +} + + bool Phase::evaluate(Rule *rule, Assay *assay) { rule->phase = this->phase; return true; diff --git a/src/actions/phase.h b/src/actions/phase.h index e7341556..a19043b7 100644 --- a/src/actions/phase.h +++ b/src/actions/phase.h @@ -34,6 +34,7 @@ class Phase : public Action { public: explicit Phase(std::string action); + bool init(std::string *error) override; bool evaluate(Rule *rule, Assay *assay) override; int phase; int m_secRulesPhase; diff --git a/src/parser/seclang-parser.yy b/src/parser/seclang-parser.yy index ef7ef4ec..c00869e3 100644 --- a/src/parser/seclang-parser.yy +++ b/src/parser/seclang-parser.yy @@ -224,6 +224,7 @@ using ModSecurity::Variables::Variable; %token ACTION_REDIRECT %token ACTION_SKIP_AFTER %token ACTION_AUDIT_LOG +%token ACTION_PHASE %token ACTION_SEVERITY %token ACTION_SETVAR %token ACTION_EXPIREVAR @@ -343,7 +344,7 @@ expression: Operator *op = Operator::instantiate($5); const char *error = NULL; if (op->init(&error) == false) { - driver.parserError << error; + driver.error(@0, error); YYERROR; } Rule *rule = new Rule( @@ -362,7 +363,7 @@ expression: Operator *op = Operator::instantiate("\"@rx " + $5 + "\""); const char *error = NULL; if (op->init(&error) == false) { - driver.parserError << error; + driver.error(@0, error); YYERROR; } Rule *rule = new Rule( @@ -370,7 +371,10 @@ expression: /* variables */ $3, /* actions */ $8 ); - driver.addSecRule(rule); + + if (driver.addSecRule(rule) == false) { + YYERROR; + } } | CONFIG_DIR_SEC_ACTION SPACE QUOTATION_MARK actions QUOTATION_MARK { @@ -405,12 +409,12 @@ expression: a->action_kind == Action::RunTimeBeforeMatchAttemptKind) { None *none = dynamic_cast(a); if (none != NULL) { - driver.parserError << "The transformation none is not suitable to be part of the SecDefaultActions"; + driver.error(@0, "The transformation none is not suitable to be part of the SecDefaultActions"); YYERROR; } checkedActions.push_back(a); } else { - driver.parserError << "The action '" << a->action << "' is not suitable to be part of the SecDefaultActions"; + driver.error(@0, "The action '" + a->action + "' is not suitable to be part of the SecDefaultActions"); YYERROR; } } @@ -419,7 +423,11 @@ expression: } if (!driver.defaultActions[definedPhase].empty()) { - driver.parserError << "SecDefaultActions can only be placed once per phase and configuration context. Phase " << secRuleDefinedPhase << " was informed already."; + std::stringstream ss; + ss << "SecDefaultActions can only be placed once per phase and configuration context. Phase "; + ss << secRuleDefinedPhase; + ss << " was informed already."; + driver.error(@0, ss.str()); YYERROR; } @@ -469,8 +477,10 @@ expression: if (driver.m_debugLog != NULL) { driver.m_debugLog->setDebugLogLevel(atoi($1.c_str())); } else { - driver.parserError << "Internal error, there is no DebugLog "; - driver.parserError << "object associated with the driver class"; + std::stringstream ss; + ss << "Internal error, there is no DebugLog "; + ss << "object associated with the driver class"; + driver.error(@0, ss.str()); YYERROR; } } @@ -479,8 +489,10 @@ expression: if (driver.m_debugLog != NULL) { driver.m_debugLog->setDebugLogFile($1); } else { - driver.parserError << "Internal error, there is no DebugLog "; - driver.parserError << "object associated with the driver class"; + std::stringstream ss; + ss << "Internal error, there is no DebugLog "; + ss << "object associated with the driver class"; + driver.error(@0, ss.str()); YYERROR; } } @@ -699,6 +711,15 @@ act: YYERROR; } } + | ACTION_PHASE + { + std::string error; + $$ = new Phase($1); + if ($$->init(&error) == false) { + driver.error(@0, error); + YYERROR; + } + } | ACTION_INITCOL { $$ = Action::instantiate($1); diff --git a/src/parser/seclang-scanner.ll b/src/parser/seclang-scanner.ll index f73a12a7..4158cdea 100755 --- a/src/parser/seclang-scanner.ll +++ b/src/parser/seclang-scanner.ll @@ -272,7 +272,7 @@ CONFIG_DIR_UNICODE_MAP_FILE (?i:SecUnicodeMapFile) } {ACTION} { return yy::seclang_parser::make_ACTION(yytext, *driver.loc.back()); } -{ACTION_PHASE} { return yy::seclang_parser::make_ACTION(yytext, *driver.loc.back()); } +{ACTION_PHASE} { return yy::seclang_parser::make_ACTION_PHASE(yytext, *driver.loc.back()); } {ACTION_SKIP_AFTER}:{FREE_TEXT} { return yy::seclang_parser::make_ACTION_SKIP_AFTER(strchr(yytext, ':') + 1, *driver.loc.back()); } {ACTION_AUDIT_LOG} { return yy::seclang_parser::make_ACTION_AUDIT_LOG(yytext, *driver.loc.back()); } @@ -398,7 +398,7 @@ CONFIG_DIR_UNICODE_MAP_FILE (?i:SecUnicodeMapFile) driver.error (*driver.loc.back(), "", s + std::string(": Not able to open file.")); throw yy::seclang_parser::syntax_error(*driver.loc.back(), ""); } - driver.ref.push_back(file); + driver.ref.push_back(s.c_str()); driver.loc.push_back(new yy::location()); yypush_buffer_state(yy_create_buffer( yyin, YY_BUF_SIZE )); diff --git a/test/benchmark/basic_rules.conf b/test/benchmark/basic_rules.conf index 65d9838d..d32552aa 100644 --- a/test/benchmark/basic_rules.conf +++ b/test/benchmark/basic_rules.conf @@ -1,2 +1,3 @@ include "owasp-modsecurity-crs-orig/modsecurity_crs_10_setup.conf" +include "owasp-modsecurity-crs-orig/rules/*.conf"