From ab5658f2d4cfa5126db256cf3f9dcb299982366d Mon Sep 17 00:00:00 2001 From: Martin Vierula Date: Tue, 25 Jul 2023 05:50:16 -0700 Subject: [PATCH] Fix: worst-case time in implementation of four transformations --- CHANGES | 2 + .../transformations/remove_comments_char.cc | 64 +++++++++---------- src/actions/transformations/remove_nulls.cc | 24 +++---- .../transformations/remove_whitespace.cc | 30 ++++----- src/actions/transformations/replace_nulls.cc | 10 +-- 5 files changed, 56 insertions(+), 74 deletions(-) diff --git a/CHANGES b/CHANGES index 886e830d0a..868cdad129 100644 --- a/CHANGES +++ b/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 diff --git a/src/actions/transformations/remove_comments_char.cc b/src/actions/transformations/remove_comments_char.cc index 529f3a67f3..4c54e68aa7 100644 --- a/src/actions/transformations/remove_comments_char.cc +++ b/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 #include -#include -#include -#include -#include #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 diff --git a/src/actions/transformations/remove_nulls.cc b/src/actions/transformations/remove_nulls.cc index 5dd576cb49..2d479c73eb 100644 --- a/src/actions/transformations/remove_nulls.cc +++ b/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 -#include #include -#include -#include -#include -#include #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; } diff --git a/src/actions/transformations/remove_whitespace.cc b/src/actions/transformations/remove_whitespace.cc index b9ffcc0649..3796466385 100644 --- a/src/actions/transformations/remove_whitespace.cc +++ b/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 #include -#include -#include -#include -#include #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(value[i])) - || (value[i] == nonBreakingSpaces) - || value[i] == nonBreakingSpaces2) { - value.erase(i, 1); + if (std::isspace(static_cast(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 diff --git a/src/actions/transformations/replace_nulls.cc b/src/actions/transformations/replace_nulls.cc index 0a37149003..1af860f4e8 100644 --- a/src/actions/transformations/replace_nulls.cc +++ b/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 #include -#include -#include -#include -#include #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++; }