Merge pull request #2934 from martinhsv/v3/master

Fix: worst-case time in implementation of four transformations

CHANGES

@@ -1,6 +1,8 @@
 v3.x.y - YYYY-MMM-DD (to be released)
 -------------------------------------
 
+  - Fix: worst-case time in implementation of four transformations
+    [Issue #2934 - @martinhsv]
   - Add TX synonym for MSC_PCRE_LIMITS_EXCEEDED
     [Issue #2901 - @airween]
   - Make MULTIPART_PART_HEADERS accessible to lua

src/actions/transformations/remove_comments_char.cc

@@ -1,6 +1,6 @@
 /*
  * ModSecurity, http://www.modsecurity.org/
- * Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ * Copyright (c) 2015 - 2023 Trustwave Holdings, Inc. (http://www.trustwave.com/)
  *
  * You may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
@@ -15,12 +15,7 @@
 
 #include "src/actions/transformations/remove_comments_char.h"
 
-#include <iostream>
 #include <string>
-#include <algorithm>
-#include <functional>
-#include <cctype>
-#include <locale>
 
 #include "modsecurity/transaction.h"
 #include "src/actions/transformations/transformation.h"
@@ -37,39 +32,40 @@ RemoveCommentsChar::RemoveCommentsChar(const std::string &action)
 
 std::string RemoveCommentsChar::evaluate(const std::string &val,
     Transaction *transaction) {
-    int64_t i;
-    std::string value(val);
+    size_t i = 0;
+    std::string transformed_value;
+    transformed_value.reserve(val.size());
 
-    i = 0;
-    while (i < value.size()) {
-        if (value.at(i) == '/'
-            && (i+1 < value.size()) && value.at(i+1) == '*') {
-            value.erase(i, 2);
-        } else if (value.at(i) == '*'
-            && (i+1 < value.size()) && value.at(i+1) == '/') {
-            value.erase(i, 2);
-        } else if (value.at(i) == '<'
-            && (i+1 < value.size())
-            && value.at(i+1) == '!'
-            && (i+2 < value.size())
-            && value.at(i+2) == '-'
-            && (i+3 < value.size())
-            && value.at(i+3) == '-') {
-            value.erase(i, 4);
-        } else if (value.at(i) == '-'
-            && (i+1 < value.size()) && value.at(i+1) == '-'
-            && (i+2 < value.size()) && value.at(i+2) == '>') {
-            value.erase(i, 3);
-        } else if (value.at(i) == '-'
-            && (i+1 < value.size()) && value.at(i+1) == '-') {
-            value.erase(i, 2);
-        } else if (value.at(i) == '#') {
-            value.erase(i, 1);
+    while (i < val.size()) {
+        if (val.at(i) == '/'
+            && (i+1 < val.size()) && val.at(i+1) == '*') {
+            i += 2;
+        } else if (val.at(i) == '*'
+            && (i+1 < val.size()) && val.at(i+1) == '/') {
+            i += 2;
+        } else if (val.at(i) == '<'
+            && (i+1 < val.size())
+            && val.at(i+1) == '!'
+            && (i+2 < val.size())
+            && val.at(i+2) == '-'
+            && (i+3 < val.size())
+            && val.at(i+3) == '-') {
+            i += 4;
+        } else if (val.at(i) == '-'
+            && (i+1 < val.size()) && val.at(i+1) == '-'
+            && (i+2 < val.size()) && val.at(i+2) == '>') {
+            i += 3;
+        } else if (val.at(i) == '-'
+            && (i+1 < val.size()) && val.at(i+1) == '-') {
+            i += 2;
+        } else if (val.at(i) == '#') {
+            i += 1;
         } else {
+            transformed_value += val.at(i);
             i++;
         }
     }
-    return value;
+    return transformed_value;
 }
 
 }  // namespace transformations

src/actions/transformations/remove_nulls.cc

@@ -1,6 +1,6 @@
 /*
  * ModSecurity, http://www.modsecurity.org/
- * Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ * Copyright (c) 2015 - 2023 Trustwave Holdings, Inc. (http://www.trustwave.com/)
  *
  * You may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
@@ -17,12 +17,7 @@
 
 #include <string.h>
 
-#include <iostream>
 #include <string>
-#include <algorithm>
-#include <functional>
-#include <cctype>
-#include <locale>
 
 #include "modsecurity/transaction.h"
 #include "src/actions/transformations/transformation.h"
@@ -35,19 +30,20 @@ namespace transformations {
 
 std::string RemoveNulls::evaluate(const std::string &val,
     Transaction *transaction) {
-    int64_t i;
-    std::string value(val);
+    size_t i = 0;
+    std::string transformed_value;
+    transformed_value.reserve(val.size());
 
-    i = 0;
-    while (i < value.size()) {
-        if (value.at(i) == '\0') {
-            value.erase(i, 1);
+    while (i < val.size()) {
+        if (val.at(i) == '\0') {
+            // do nothing; continue on to next char in original val
         } else {
-            i++;
+            transformed_value += val.at(i);
         }
+        i++;
     }
 
-    return value;
+    return transformed_value;
 }
 
 

src/actions/transformations/remove_whitespace.cc

@@ -1,6 +1,6 @@
 /*
  * ModSecurity, http://www.modsecurity.org/
- * Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ * Copyright (c) 2015 - 2023 Trustwave Holdings, Inc. (http://www.trustwave.com/)
  *
  * You may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
@@ -15,12 +15,7 @@
 
 #include "src/actions/transformations/remove_whitespace.h"
 
-#include <iostream>
 #include <string>
-#include <algorithm>
-#include <functional>
-#include <cctype>
-#include <locale>
 
 #include "modsecurity/transaction.h"
 #include "src/actions/transformations/transformation.h"
@@ -37,28 +32,27 @@ RemoveWhitespace::RemoveWhitespace(const std::string &action)
 
 std::string RemoveWhitespace::evaluate(const std::string &val,
     Transaction *transaction) {
-    std::string value(val);
+    std::string transformed_value;
+    transformed_value.reserve(val.size());
 
-    int64_t i = 0;
+    size_t i = 0;
     const char nonBreakingSpaces = 0xa0;
     const char nonBreakingSpaces2 = 0xc2;
 
     // loop through all the chars
-    while (i < value.size()) {
+    while (i < val.size()) {
         // remove whitespaces and non breaking spaces (NBSP)
-        if (std::isspace(static_cast<unsigned char>(value[i]))
-            || (value[i] == nonBreakingSpaces)
-            || value[i] == nonBreakingSpaces2) {
-            value.erase(i, 1);
+        if (std::isspace(static_cast<unsigned char>(val[i]))
+            || (val[i] == nonBreakingSpaces)
+            || val[i] == nonBreakingSpaces2) {
+            // don't copy; continue on to next char in original val
         } else {
-          /* if the space is not a whitespace char, increment counter
-           counter should not be incremented if a character is erased because
-           the index erased will be replaced by the following character */
-          i++;
+            transformed_value += val.at(i);
         }
+        i++;
     }
 
-    return value;
+    return transformed_value;
 }
 
 }  // namespace transformations

src/actions/transformations/replace_nulls.cc

@@ -1,6 +1,6 @@
 /*
  * ModSecurity, http://www.modsecurity.org/
- * Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ * Copyright (c) 2015 - 2023 Trustwave Holdings, Inc. (http://www.trustwave.com/)
  *
  * You may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
@@ -15,12 +15,7 @@
 
 #include "src/actions/transformations/replace_nulls.h"
 
-#include <iostream>
 #include <string>
-#include <algorithm>
-#include <functional>
-#include <cctype>
-#include <locale>
 
 #include "modsecurity/transaction.h"
 #include "src/actions/transformations/transformation.h"
@@ -43,8 +38,7 @@ std::string ReplaceNulls::evaluate(const std::string &val,
     i = 0;
     while (i < value.size()) {
         if (value.at(i) == '\0') {
-            value.erase(i, 1);
-            value.insert(i, " ", 1);
+            value[i] = ' ';
         } else {
             i++;
         }