Merge pull request #3233 from eduar-hte/remove-copies-pm-operator

Removed multiple heap-allocated copies in Pm::init & parse_pm_content

src/Makefile.am

@@ -219,7 +219,6 @@ OPERATORS = \
 	operators/no_match.cc \
 	operators/operator.cc \
 	operators/pm.cc \
-	operators/pm_f.cc \
 	operators/pm_from_file.cc \
 	operators/rbl.cc \
 	operators/rsub.cc \

src/operators/pm.cc

@@ -15,20 +15,80 @@
 
 #include "src/operators/pm.h"
 
-#include <string.h>
-
-#include <string>
 #include <algorithm>
 #include <iterator>
 #include <sstream>
-#include <vector>
-#include <list>
-#include <memory>
 
-#include "src/operators/operator.h"
 #include "src/utils/acmp.h"
 #include "src/utils/string.h"
 
+
+static inline std::string parse_pm_content(const std::string &op_parm) {
+    auto offset = op_parm.find_first_not_of(" \t");
+    if (offset == std::string::npos) {
+        return op_parm;
+    }
+
+    auto size = op_parm.size() - offset;
+    if (size >= 2 &&
+        op_parm.at(offset) == '\"' && op_parm.back() == '\"') {
+        offset++;
+        size -= 2;
+    }
+
+    if (size == 0) {
+        return op_parm;
+    }
+
+    std::string parsed_parm(op_parm.c_str() + offset, size);
+
+    unsigned char bin_offset = 0;
+    unsigned char bin_parm[3] = { 0 };
+    bool bin = false, esc = false;
+
+    char *d = parsed_parm.data();
+    for(const char *s = d, *e = d + size; s != e; ++s) {
+        if (*s == '|') {
+            bin = !bin;
+        } else if(!esc && *s == '\\') {
+            esc = true;
+        } else {
+            if (bin) {
+                if (VALID_HEX(*s)) {
+                    bin_parm[bin_offset] = (char)*s;
+                    bin_offset++;
+                    if (bin_offset == 2) {
+                        unsigned char c = strtol((char *)bin_parm, (char **) nullptr, 16) & 0xFF;
+                        bin_offset = 0;
+                        *d++ = c;
+                    }
+                } else {
+                    // Invalid hex character
+                    return op_parm;
+                }
+            } else if (esc) {
+                if (*s == ':' ||
+                        *s == ';' ||
+                        *s == '\\' ||
+                        *s == '\"')
+                {
+                    *d++ = *s;
+                } else {
+                    // Unsupported escape sequence
+                    return op_parm;
+                }
+                esc = false;
+            } else {
+                *d++ = *s;
+            }
+        }
+    }
+
+    parsed_parm.resize(d - parsed_parm.c_str());
+    return parsed_parm;
+}
+
+
 namespace modsecurity {
 namespace operators {
 
@@ -105,36 +165,19 @@ bool Pm::evaluate(Transaction *transaction, RuleWithActions *rule,
 
 
 bool Pm::init(const std::string &file, std::string *error) {
-    std::vector<std::string> vec;
-    std::istringstream *iss;
-    const char *err = NULL;
-
-    char *content = parse_pm_content(m_param.c_str(), m_param.length(), &err);
-    if (content == NULL) {
-        iss = new std::istringstream(m_param);
-    } else {
-        iss = new std::istringstream(content);
-    }
+    const auto op_parm = parse_pm_content(m_param);
 
-    std::copy(std::istream_iterator<std::string>(*iss),
+    std::istringstream iss{op_parm};
+    std::for_each(std::istream_iterator<std::string>(iss),
         std::istream_iterator<std::string>(),
-        back_inserter(vec));
-
-    for (auto &a : vec) {
-        acmp_add_pattern(m_p, a.c_str(), NULL, NULL, a.length());
-    }
+        [this](const auto &a) {
+            acmp_add_pattern(m_p, a.c_str(), NULL, NULL, a.length());
+        });
 
     while (m_p->is_failtree_done == 0) {
         acmp_prepare(m_p);
     }
 
-    if (content) {
-        free(content);
-        content = NULL;
-    }
-
-    delete iss;
-
     return true;
 }
 

src/operators/pm.h

@@ -17,7 +17,6 @@
 #define SRC_OPERATORS_PM_H_
 
 #include <string>
-#include <list>
 #include <memory>
 #include <utility>
 #include <mutex>

src/utils/acmp.cc

@@ -33,135 +33,8 @@
  *       that should be mitigated. This ACMP parser should be re-written to
  *       consume less memory.
  */
-extern "C" {
-
-char *parse_pm_content(const char *op_parm, unsigned short int op_len, const char **error_msg)  {
-    char *parm = NULL;
-    char *content;
-    unsigned short int offset = 0;
-//    char converted = 0;
-    int i, x;
-    unsigned char bin = 0, esc = 0, bin_offset = 0;
-    unsigned char c = 0;
-    unsigned char bin_parm[3] = { 0 };
-    char *processed = NULL;
-
-    content = strdup(op_parm);
-
-    if (content == NULL) {
-        *error_msg = std::string("Error allocating memory for pattern matching content.").c_str();
-        return NULL;
-    }
-
-    while (offset < op_len && (content[offset] == ' ' || content[offset] == '\t')) {
-        offset++;
-    };
-
-    op_len = strlen(content);
-
-    if (content[offset] == '\"' && content[op_len-1] == '\"') {
-        parm = strdup(content + offset + 1);
-        if (parm  == NULL) {
-            *error_msg = std::string("Error allocating memory for pattern matching content.").c_str();
-            free(content);
-            content = NULL;
-            return NULL;
-        }
-        parm[op_len - offset - 2] = '\0';
-    } else {
-        parm = strdup(content + offset);
-        if (parm == NULL) {
-            free(content);
-            content = NULL;
-            *error_msg = std::string("Error allocating memory for pattern matching content.").c_str();
-            return NULL;
-        }
-    }
-
-    free(content);
-    content = NULL;
 
-    op_len = strlen(parm);
-
-    if (op_len == 0)   {
-        *error_msg = "Content length is 0.";
-        free(parm);
-        return NULL;
-    }
-
-    for (i = 0, x = 0; i < op_len; i++) {
-        if (parm[i] == '|') {
-            if (bin) {
-                bin = 0;
-            } else {
-                bin = 1;
-            }
-        } else if(!esc && parm[i] == '\\') {
-            esc = 1;
-        } else {
-            if (bin) {
-                if (parm[i] == 0 || parm[i] == 1 || parm[i] == 2 ||
-                    parm[i] == 3 || parm[i] == 4 || parm[i] == 5 ||
-                    parm[i] == 6 || parm[i] == 7 || parm[i] == 8 ||
-                    parm[i] == 9 ||
-                    parm[i] == 'A' || parm[i] == 'a' ||
-                    parm[i] == 'B' || parm[i] == 'b' ||
-                    parm[i] == 'C' || parm[i] == 'c' ||
-                    parm[i] == 'D' || parm[i] == 'd' ||
-                    parm[i] == 'E' || parm[i] == 'e' ||
-                    parm[i] == 'F' || parm[i] == 'f')
-                {
-                    bin_parm[bin_offset] = (char)parm[i];
-                    bin_offset++;
-                    if (bin_offset == 2) {
-                        c = strtol((char *)bin_parm, (char **) NULL, 16) & 0xFF;
-                        bin_offset = 0;
-                        parm[x] = c;
-                        x++;
-                        //converted = 1;
-                    }
-                } else if (parm[i] == ' ') {
-                }
-            } else if (esc) {
-                if (parm[i] == ':' ||
-                        parm[i] == ';' ||
-                        parm[i] == '\\' ||
-                        parm[i] == '\"')
-                {
-                    parm[x] = parm[i];
-                    x++;
-                } else {
-                    *error_msg = std::string("Unsupported escape sequence.").c_str();
-                    free(parm);
-                    return NULL;
-                }
-                esc = 0;
-                //converted = 1;
-            } else {
-                parm[x] = parm[i];
-                x++;
-            }
-        }
-    }
-
-#if 0
-    if (converted) {
-        op_len = x;
-    }
-#endif
-
-    //processed = memcpy(processed, parm, op_len);
-    processed = strdup(parm);
-    free(parm);
-    parm = NULL;
-
-    if (processed == NULL) {
-        *error_msg = std::string("Error allocating memory for pattern matching content.").c_str();
-        return NULL;
-    }
-
-    return processed;
-}
+extern "C" {
 
 /*
  *******************************************************************************

src/utils/acmp.h

@@ -23,6 +23,7 @@
 #endif
 
 #include <cstddef>
+#include <string>
 
 
 extern "C" {
@@ -189,8 +190,6 @@ int acmp_process_quick(ACMPT *acmpt, const char **match, const char *data, size_
  */
 int acmp_prepare(ACMP *parser);
 
-char *parse_pm_content(const char *op_parm, unsigned short int op_len, const char **error_msg);
-
 }
 
 #endif /*ACMP_H_*/