Fix crash on SecRuleRemoveById malformated parameter

Fix issue #1440

src/parser/seclang-parser.cc

@@ -849,7 +849,7 @@ namespace yy {
 
 
     // User initialization code.
-    #line 339 "/root/ModSec/ModSecurity-v3/src/parser/seclang-parser.yy" // lalr1.cc:741
+    #line 339 "/home/zimmerle/core-trustwave/ModSecurity/src/parser/seclang-parser.yy" // lalr1.cc:741
 {
   // Initialize the initial location.
   yyla.location.begin.filename = yyla.location.end.filename = &driver.file;

src/parser/seclang-scanner.ll

@@ -11,6 +11,7 @@
 
 using modsecurity::Parser::Driver;
 using modsecurity::Utils::HttpsClient;
+using modsecurity::utils::string::parserSanitizer;
 
 typedef yy::seclang_parser p;
 
@@ -579,7 +580,7 @@ EQUALS_MINUS                            (?i:=\-)
 {CONFIG_DIR_SEC_MARKER}[ \t]+["]{NEW_LINE_FREE_TEXT}["]                 { return p::make_CONFIG_DIR_SEC_MARKER(strchr(yytext, ' ') + 1, *driver.loc.back()); }
 {CONFIG_DIR_SEC_MARKER}[ \t]+{NEW_LINE_FREE_TEXT}                       { return p::make_CONFIG_DIR_SEC_MARKER(strchr(yytext, ' ') + 1, *driver.loc.back()); }
 {CONFIG_DIR_UNICODE_MAP_FILE}[ ]{FREE_TEXT_NEW_LINE}                    { return p::make_CONFIG_DIR_UNICODE_MAP_FILE(strchr(yytext, ' ') + 1, *driver.loc.back()); }
-{CONFIG_SEC_REMOVE_RULES_BY_ID}[ ]{FREE_TEXT_NEW_LINE}                  { return p::make_CONFIG_SEC_RULE_REMOVE_BY_ID(strchr(yytext, ' ') + 1, *driver.loc.back()); }
+{CONFIG_SEC_REMOVE_RULES_BY_ID}[ ]+{FREE_TEXT_NEW_LINE}                 { return p::make_CONFIG_SEC_RULE_REMOVE_BY_ID(parserSanitizer(strchr(yytext, ' ') + 1), *driver.loc.back()); }
 {CONFIG_UPDLOAD_KEEP_FILES}                                             { return p::make_CONFIG_UPDLOAD_KEEP_FILES(yytext, *driver.loc.back()); }
 {CONFIG_UPDLOAD_SAVE_TMP_FILES}                                         { return p::make_CONFIG_UPDLOAD_SAVE_TMP_FILES(yytext, *driver.loc.back()); }
 {CONFIG_UPLOAD_DIR}[ ]{CONFIG_VALUE_PATH}                               { return p::make_CONFIG_UPLOAD_DIR(strchr(yytext, ' ') + 1, *driver.loc.back()); }

src/rules_exceptions.cc

@@ -24,9 +24,13 @@ namespace modsecurity {
 
 
 bool RulesExceptions::load(const std::string &a, std::string *error) {
+    bool added = false;
     std::vector<std::string> toRemove = utils::string::split(a, ' ');
     for (std::string &a : toRemove) {
-        std::string b = utils::string::removeBracketsIfNeeded(a);
+        std::string b = modsecurity::utils::string::parserSanitizer(a);
+        if (b.size() == 0) {
+            continue;
+        }
 
         size_t dash = b.find('-');
         if (dash != std::string::npos) {
@@ -36,12 +40,14 @@ bool RulesExceptions::load(const std::string &a, std::string *error) {
             int n2n = 0;
             try {
                 n1n = std::stoi(n1s);
+                added = true;
             } catch (...) {
                 error->assign("Not a number: " + n1s);
                 return false;
             }
             try {
                 n2n = std::stoi(n2s);
+                added = true;
             } catch (...) {
                 error->assign("Not a number: " + n2s);
                 return false;
@@ -52,10 +58,12 @@ bool RulesExceptions::load(const std::string &a, std::string *error) {
                 return false;
             }
             addRange(n1n, n2n);
+            added = true;
         } else {
             try {
                 int num = std::stoi(b);
                 addNumber(num);
+                added = true;
             } catch (...) {
                 error->assign("Not a number or range: " + b);
                 return false;
@@ -63,9 +71,14 @@ bool RulesExceptions::load(const std::string &a, std::string *error) {
         }
     }
 
+    if (added) {
         return true;
     }
 
+    error->assign("Not a number or range: " + a);
+    return false;
+}
+
 
 bool RulesExceptions::addNumber(int a) {
     m_numbers.push_back(a);

src/utils/string.cc

@@ -47,6 +47,24 @@ namespace utils {
 namespace string {
 
 
+std::string parserSanitizer(std::string a) {
+    a = removeWhiteSpacesIfNeeded(a);
+    a = removeBracketsIfNeeded(a);
+    return a;
+}
+
+
+std::string removeWhiteSpacesIfNeeded(std::string a) {
+    while (a.size() > 1 && a.at(0) == ' ') {
+        a.erase(0, 1);
+    }
+    while (a.size() > 1 && a.at(a.length()-1) == ' ') {
+        a.pop_back();
+    }
+    return a;
+}
+
+
 std::string ascTime(time_t *t) {
     std::string ts = std::ctime(t);
     ts.pop_back();
@@ -87,7 +105,7 @@ std::string limitTo(int amount, const std::string &str) {
 
 
 std::string removeBracketsIfNeeded(std::string a) {
-    if ((a.at(0) == '"') && (a.at(a.length()-1) == '"')) {
+    if (a.length() > 1 && a.at(0) == '"' && a.at(a.length()-1) == '"') {
         a.pop_back();
         a.erase(0, 1);
     }

src/utils/string.h

@@ -68,6 +68,8 @@ std::vector<std::string> split(std::string str, char delimiter);
 void chomp(std::string *str);
 void replaceAll(std::string *str, const std::string& from,
     const std::string& to);
+std::string removeWhiteSpacesIfNeeded(std::string a);
+std::string parserSanitizer(std::string a);
 
 unsigned char x2c(unsigned char *what);
 unsigned char xsingle2c(unsigned char *what);