From 0613ceeb754341b3dad4581457ee60d1d9f17026 Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Sun, 2 Jun 2024 20:07:19 +0000 Subject: [PATCH] Replace usage of range-checked 'at' method when vector/string has already been size checked --- .../anchored_set_variable_translation_proxy.h | 8 ++------ headers/modsecurity/rules.h | 6 +++--- src/actions/xmlns.cc | 2 +- src/operators/contains_word.cc | 7 ++++--- src/operators/inspect_file.cc | 2 +- src/operators/pm.cc | 2 +- src/operators/validate_byte_range.cc | 4 ++-- src/rule_with_operator.cc | 3 +-- src/rules_set_phases.cc | 4 ++-- src/transaction.cc | 4 ++-- src/utils/string.h | 14 +++++++------- 11 files changed, 26 insertions(+), 30 deletions(-) diff --git a/headers/modsecurity/anchored_set_variable_translation_proxy.h b/headers/modsecurity/anchored_set_variable_translation_proxy.h index f36c69b167..10f3f7f2d2 100644 --- a/headers/modsecurity/anchored_set_variable_translation_proxy.h +++ b/headers/modsecurity/anchored_set_variable_translation_proxy.h @@ -100,13 +100,9 @@ class AnchoredSetVariableTranslationProxy { return nullptr; } - std::unique_ptr ret(new std::string("")); + auto ret = std::make_unique(l[0]->getValue()); - ret->assign(l.at(0)->getValue()); - - while (!l.empty()) { - auto &a = l.back(); - l.pop_back(); + for(auto a : l) { delete a; } diff --git a/headers/modsecurity/rules.h b/headers/modsecurity/rules.h index 1aaa65b549..a9bfb80d3e 100644 --- a/headers/modsecurity/rules.h +++ b/headers/modsecurity/rules.h @@ -41,9 +41,9 @@ namespace modsecurity { class Rules { public: void dump() const { - for (int j = 0; j < m_rules.size(); j++) { - std::cout << " Rule ID: " << m_rules.at(j)->getReference(); - std::cout << "--" << m_rules.at(j) << std::endl; + for (const auto &r : m_rules) { + std::cout << " Rule ID: " << r->getReference(); + std::cout << "--" << r << std::endl; } } diff --git a/src/actions/xmlns.cc b/src/actions/xmlns.cc index bca3b32fe1..cb4044d839 100644 --- a/src/actions/xmlns.cc +++ b/src/actions/xmlns.cc @@ -43,7 +43,7 @@ bool XmlNS::init(std::string *error) { return false; } - if (m_href.at(0) == '\'' && m_href.size() > 3) { + if (m_href[0] == '\'' && m_href.size() > 3) { m_href.erase(0, 1); m_href.pop_back(); } diff --git a/src/operators/contains_word.cc b/src/operators/contains_word.cc index 3de6785166..e1ca9b869f 100644 --- a/src/operators/contains_word.cc +++ b/src/operators/contains_word.cc @@ -23,13 +23,14 @@ namespace modsecurity { namespace operators { -bool ContainsWord::acceptableChar(const std::string& a, size_t pos) { +inline bool ContainsWord::acceptableChar(const std::string& a, size_t pos) { if (a.size() - 1 < pos) { return false; } - if ((a.at(pos) >= 65 && a.at(pos) <= 90) || - (a.at(pos) >= 97 && a.at(pos) <= 122)) { + const auto ch = a[pos]; + if ((ch >= 65 && ch <= 90) || + (ch >= 97 && ch <= 122)) { return false; } diff --git a/src/operators/inspect_file.cc b/src/operators/inspect_file.cc index 72052a32da..3796d26e62 100644 --- a/src/operators/inspect_file.cc +++ b/src/operators/inspect_file.cc @@ -77,7 +77,7 @@ bool InspectFile::evaluate(Transaction *transaction, const std::string &str) { pclose(in); res.append(s.str()); - if (res.size() > 1 && res.at(0) != '1') { + if (res.size() > 1 && res[0] != '1') { return true; /* match */ } diff --git a/src/operators/pm.cc b/src/operators/pm.cc index 91e4f5d7b4..6a44b2860e 100644 --- a/src/operators/pm.cc +++ b/src/operators/pm.cc @@ -32,7 +32,7 @@ static inline std::string parse_pm_content(const std::string &op_parm) { auto size = op_parm.size() - offset; if (size >= 2 && - op_parm.at(offset) == '\"' && op_parm.back() == '\"') { + op_parm[offset] == '\"' && op_parm.back() == '\"') { offset++; size -= 2; } diff --git a/src/operators/validate_byte_range.cc b/src/operators/validate_byte_range.cc index 47802cef48..2553b9c1a4 100644 --- a/src/operators/validate_byte_range.cc +++ b/src/operators/validate_byte_range.cc @@ -115,8 +115,8 @@ bool ValidateByteRange::evaluate(Transaction *transaction, RuleWithActions *rule bool ret = true; size_t count = 0; - for (int i = 0; i < input.length(); i++) { - int x = (unsigned char) input.at(i); + for (std::string::size_type i = 0; i < input.length(); i++) { + int x = (unsigned char) input[i]; if (!(table[x >> 3] & (1 << (x & 0x7)))) { // debug(9, "Value " + std::to_string(x) + " in " + // input + " ouside range: " + param); diff --git a/src/rule_with_operator.cc b/src/rule_with_operator.cc index e2864eb03f..a3e35bf0ba 100644 --- a/src/rule_with_operator.cc +++ b/src/rule_with_operator.cc @@ -194,8 +194,7 @@ inline void RuleWithOperator::getFinalVars(variables::Variables *vars, vars->push_back(variable); } - for (int i = 0; i < addition.size(); i++) { - Variable *variable = addition.at(i); + for (auto *variable : addition) { vars->push_back(variable); } } diff --git a/src/rules_set_phases.cc b/src/rules_set_phases.cc index 1d93330caa..a781930498 100644 --- a/src/rules_set_phases.cc +++ b/src/rules_set_phases.cc @@ -45,8 +45,8 @@ int RulesSetPhases::append(RulesSetPhases *from, std::ostringstream *err) { for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { v.reserve(m_rulesAtPhase[i].size()); - for (size_t z = 0; z < m_rulesAtPhase[i].size(); z++) { - RuleWithOperator *rule_ckc = dynamic_cast(m_rulesAtPhase[i].at(z).get()); + for (const auto &r : m_rulesAtPhase[i].m_rules) { + const auto *rule_ckc = dynamic_cast(r.get()); if (!rule_ckc) { continue; } diff --git a/src/transaction.cc b/src/transaction.cc index 1892252310..f5ded52464 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -405,7 +405,7 @@ int Transaction::processURI(const char *uri, const char *method, std::string parsedURI = m_uri_decoded; // The more popular case is without domain - if (!m_uri_decoded.empty() && m_uri_decoded.at(0) != '/') { + if (!m_uri_decoded.empty() && m_uri_decoded[0] != '/') { bool fullDomain = true; size_t scheme = m_uri_decoded.find(":")+1; if (scheme == std::string::npos) { @@ -540,7 +540,7 @@ int Transaction::addRequestHeader(const std::string& key, } // ltrim the key - following the modsec v2 way - while (ckey.empty() == false && isspace(ckey.at(0))) { + while (ckey.empty() == false && isspace(ckey[0])) { ckey.erase(0, 1); localOffset++; } diff --git a/src/utils/string.h b/src/utils/string.h index f5e52ff06d..f5794686bf 100644 --- a/src/utils/string.h +++ b/src/utils/string.h @@ -107,12 +107,12 @@ inline std::string toHexIfNeeded(const std::string &str, bool escape_spec = fals // spec chars: '"' (quotation mark, ascii 34), '\' (backslash, ascii 92) std::stringstream res; - for (int i = 0; i < str.size(); i++) { - int c = (unsigned char)str.at(i); + for (const auto ch : str) { + int c = (unsigned char)ch; if (c < 32 || c > 126 || (escape_spec == true && (c == 34 || c == 92))) { res << "\\x" << std::setw(2) << std::setfill('0') << std::hex << c; } else { - res << str.at(i); + res << ch; } } @@ -177,10 +177,10 @@ inline void replaceAll(std::string &str, std::string_view from, inline std::string removeWhiteSpacesIfNeeded(std::string a) { - while (a.size() > 1 && a.at(0) == ' ') { + while (a.size() > 1 && a.front() == ' ') { a.erase(0, 1); } - while (a.size() > 1 && a.at(a.length()-1) == ' ') { + while (a.size() > 1 && a.back() == ' ') { a.pop_back(); } return a; @@ -188,11 +188,11 @@ inline std::string removeWhiteSpacesIfNeeded(std::string a) { inline std::string removeBracketsIfNeeded(std::string a) { - if (a.length() > 1 && a.at(0) == '"' && a.at(a.length()-1) == '"') { + if (a.length() > 1 && a.front() == '"' && a.back() == '"') { a.pop_back(); a.erase(0, 1); } - if (a.length() > 1 && a.at(0) == '\'' && a.at(a.length()-1) == '\'') { + if (a.length() > 1 && a.front() == '\'' && a.back() == '\'') { a.pop_back(); a.erase(0, 1); }