diff --git a/src/modsecurity.cc b/src/modsecurity.cc index 31582f75e..08a8bed04 100644 --- a/src/modsecurity.cc +++ b/src/modsecurity.cc @@ -32,6 +32,7 @@ #include #include +#include #include #include "modsecurity/rule.h" @@ -213,6 +214,11 @@ void ModSecurity::serverLog(void *data, const RuleMessage &rm) { } } +static inline int svtoi(std::string_view s) { + int value; + std::from_chars(s.data(), s.data() + s.size(), value); + return value; +} int ModSecurity::processContentOffset(const char *content, size_t len, const char *matchString, std::string *json, const char **err) { @@ -225,9 +231,9 @@ int ModSecurity::processContentOffset(const char *content, size_t len, const unsigned char *buf; size_t jsonSize; - std::list vars = variables.searchAll(matchString); - std::list ops = operators.searchAll(matchString); - std::list trans = transformations.searchAll(matchString); + const auto vars = variables.searchAll(matchString); + const auto ops = operators.searchAll(matchString); + const auto trans = transformations.searchAll(matchString); g = yajl_gen_alloc(NULL); if (g == NULL) { @@ -255,27 +261,27 @@ int ModSecurity::processContentOffset(const char *content, size_t len, for(auto [it, pending] = std::tuple{vars.rbegin(), vars.size()}; pending > 3; pending -= 3) { yajl_gen_map_open(g); it++; - const std::string &startingAt = it->str(); it++; - const std::string &size = it->str(); it++; + const auto startingAt = it->str(); it++; + const auto size = it->str(); it++; yajl_gen_string(g, reinterpret_cast("startingAt"), strlen("startingAt")); yajl_gen_string(g, - reinterpret_cast(startingAt.c_str()), + reinterpret_cast(startingAt.data()), startingAt.size()); yajl_gen_string(g, reinterpret_cast("size"), strlen("size")); yajl_gen_string(g, - reinterpret_cast(size.c_str()), + reinterpret_cast(size.data()), size.size()); yajl_gen_map_close(g); - if (stoi(startingAt) >= len) { + if (svtoi(startingAt) >= len) { *err = "Offset is out of the content limits."; return -1; } - const auto value = std::string(content, stoi(startingAt), stoi(size)); + const auto value = std::string(content, svtoi(startingAt), svtoi(size)); if (varValue.size() > 0) { varValue.push_back(' '); } @@ -295,21 +301,20 @@ int ModSecurity::processContentOffset(const char *content, size_t len, varValue.size()); yajl_gen_map_close(g); - while (!trans.empty()) { - modsecurity::actions::transformations::Transformation *t; + for (const auto &match : trans) { yajl_gen_map_open(g); yajl_gen_string(g, reinterpret_cast("transformation"), strlen("transformation")); yajl_gen_string(g, - reinterpret_cast(trans.back().str().c_str()), - trans.back().str().size()); + reinterpret_cast(match.str().data()), + match.str().size()); - t = modsecurity::actions::transformations::Transformation::instantiate( - trans.back().str().c_str()); + auto t = std::unique_ptr( + modsecurity::actions::transformations::Transformation::instantiate( + std::string(match.str().data(), match.str().size()))); t->transform(varValue, nullptr); - trans.pop_back(); yajl_gen_string(g, reinterpret_cast("value"), strlen("value")); @@ -317,8 +322,6 @@ int ModSecurity::processContentOffset(const char *content, size_t len, varValue.c_str()), varValue.size()); yajl_gen_map_close(g); - - delete t; } yajl_gen_array_close(g); @@ -333,22 +336,22 @@ int ModSecurity::processContentOffset(const char *content, size_t len, strlen("highlight")); yajl_gen_map_open(g); it++; - const std::string &startingAt = it->str(); it++; - const std::string &size = ops.back().str(); it++; + const auto startingAt = it->str(); it++; + const auto size = ops.back().str(); it++; yajl_gen_string(g, reinterpret_cast("startingAt"), strlen("startingAt")); yajl_gen_string(g, - reinterpret_cast(startingAt.c_str()), + reinterpret_cast(startingAt.data()), startingAt.size()); yajl_gen_string(g, reinterpret_cast("size"), strlen("size")); yajl_gen_string(g, - reinterpret_cast(size.c_str()), + reinterpret_cast(size.data()), size.size()); yajl_gen_map_close(g); - if (stoi(startingAt) >= varValue.size()) { + if (svtoi(startingAt) >= varValue.size()) { *err = "Offset is out of the variable limits."; return -1; } @@ -356,7 +359,7 @@ int ModSecurity::processContentOffset(const char *content, size_t len, reinterpret_cast("value"), strlen("value")); - const auto value = std::string(varValue, stoi(startingAt), stoi(size)); + const auto value = std::string(varValue, svtoi(startingAt), svtoi(size)); yajl_gen_string(g, reinterpret_cast(value.c_str()), diff --git a/src/operators/verify_cpf.cc b/src/operators/verify_cpf.cc index 0755a83b2..f63574d51 100644 --- a/src/operators/verify_cpf.cc +++ b/src/operators/verify_cpf.cc @@ -111,7 +111,6 @@ bool VerifyCPF::verify(const char *cpfnumber, int len) const { bool VerifyCPF::evaluate(Transaction *t, RuleWithActions *rule, const std::string& input, RuleMessage &ruleMessage) { - std::list matches; bool is_cpf = false; int i; @@ -120,9 +119,10 @@ bool VerifyCPF::evaluate(Transaction *t, RuleWithActions *rule, } for (i = 0; i < input.size() - 1 && is_cpf == false; i++) { - matches = m_re->searchAll(input.substr(i, input.size())); + const auto iv = std::string_view(input).substr(i, input.size()); + const auto matches = m_re->searchAll(iv); for (const auto & m : matches) { - is_cpf = verify(m.str().c_str(), m.str().size()); + is_cpf = verify(m.str().data(), m.str().size()); if (is_cpf) { logOffset(ruleMessage, m.offset(), m.str().size()); if (rule && t && rule->hasCaptureAction()) { diff --git a/src/operators/verify_ssn.cc b/src/operators/verify_ssn.cc index a5f790fe0..9fce20cf0 100644 --- a/src/operators/verify_ssn.cc +++ b/src/operators/verify_ssn.cc @@ -113,7 +113,6 @@ bool VerifySSN::verify(const char *ssnumber, int len) { bool VerifySSN::evaluate(Transaction *t, RuleWithActions *rule, const std::string& input, RuleMessage &ruleMessage) { - std::list matches; bool is_ssn = false; int i; @@ -122,9 +121,10 @@ bool VerifySSN::evaluate(Transaction *t, RuleWithActions *rule, } for (i = 0; i < input.size() - 1 && is_ssn == false; i++) { - matches = m_re->searchAll(input.substr(i, input.size())); + const auto iv = std::string_view(input).substr(i, input.size()); + const auto matches = m_re->searchAll(iv); for (const auto & j : matches) { - is_ssn = verify(j.str().c_str(), j.str().size()); + is_ssn = verify(j.str().data(), j.str().size()); if (is_ssn) { logOffset(ruleMessage, j.offset(), j.str().size()); if (rule && t && rule->hasCaptureAction()) { diff --git a/src/operators/verify_svnr.cc b/src/operators/verify_svnr.cc index e2a1d4c8b..a76be83ff 100644 --- a/src/operators/verify_svnr.cc +++ b/src/operators/verify_svnr.cc @@ -80,19 +80,18 @@ bool VerifySVNR::verify(const char *svnrnumber, int len) const { bool VerifySVNR::evaluate(Transaction *t, RuleWithActions *rule, const std::string& input, RuleMessage &ruleMessage) { - std::list matches; bool is_svnr = false; int i; if (m_param.empty()) { - return is_svnr; + return false; } for (i = 0; i < input.size() - 1 && is_svnr == false; i++) { - matches = m_re->searchAll(input.substr(i, input.size())); - + const auto iv = std::string_view(input).substr(i, input.size()); + const auto matches = m_re->searchAll(iv); for (const auto & j : matches) { - is_svnr = verify(j.str().c_str(), j.str().size()); + is_svnr = verify(j.str().data(), j.str().size()); if (is_svnr) { logOffset(ruleMessage, j.offset(), j.str().size()); if (rule && t && rule->hasCaptureAction()) { diff --git a/src/utils/regex.cc b/src/utils/regex.cc index 976c0c469..4657d8697 100644 --- a/src/utils/regex.cc +++ b/src/utils/regex.cc @@ -136,8 +136,8 @@ Regex::~Regex() { } -std::list Regex::searchAll(std::string_view s) const { - std::list retList; +std::vector Regex::searchAll(std::string_view s) const { + std::vector ret; int rc = 0; #ifdef WITH_PCRE2 PCRE2_SPTR pcre2_s = reinterpret_cast(s.data()); @@ -172,9 +172,9 @@ std::list Regex::searchAll(std::string_view s) const { rc = -1; break; } - std::string match = std::string(s, start, len); + const auto match = s.substr(start, len); offset = start + len; - retList.push_front(SMatch(match, start)); + ret.push_back(SMatch(match, start)); if (len == 0) { rc = 0; @@ -186,7 +186,7 @@ std::list Regex::searchAll(std::string_view s) const { #ifdef WITH_PCRE2 pcre2_match_data_free(match_data); #endif - return retList; + return ret; } RegexResult Regex::searchOneMatch(std::string_view s, std::vector& captures, unsigned long match_limit) const { @@ -343,7 +343,7 @@ RegexResult Regex::searchGlobal(std::string_view s, std::vector& return RegexResult::Ok; } -int Regex::search(std::string_view s, SMatch *match) const { +int Regex::search(std::string_view s, SMatch &match) const { #ifdef WITH_PCRE2 PCRE2_SPTR pcre2_s = reinterpret_cast(s.data()); pcre2_match_data *match_data = pcre2_match_data_create_from_pattern(m_pc, NULL); @@ -358,7 +358,7 @@ int Regex::search(std::string_view s, SMatch *match) const { 0, PCRE2_NO_JIT, match_data, NULL) > 0; } if (ret > 0) { // match - PCRE2_SIZE *ovector = pcre2_get_ovector_pointer(match_data); + const PCRE2_SIZE *ovector = pcre2_get_ovector_pointer(match_data); #else int ovector[OVECCOUNT]; int ret = pcre_exec(m_pc, m_pce, s.data(), @@ -366,8 +366,8 @@ int Regex::search(std::string_view s, SMatch *match) const { if (ret > 0) { #endif - *match = SMatch( - std::string(s, ovector[ret-1], ovector[ret] - ovector[ret-1]), + match = SMatch( + s.substr(ovector[ret-1], ovector[ret] - ovector[ret-1]), 0); } diff --git a/src/utils/regex.h b/src/utils/regex.h index 3cfe5e74d..c711d0ae9 100644 --- a/src/utils/regex.h +++ b/src/utils/regex.h @@ -46,15 +46,15 @@ class SMatch { m_match(), m_offset(0) { } - SMatch(const std::string &match, size_t offset) : + SMatch(std::string_view match, size_t offset) : m_match(match), m_offset(offset) { } - const std::string& str() const { return m_match; } + std::string_view str() const { return m_match; } size_t offset() const { return m_offset; } private: - std::string m_match; + std::string_view m_match; size_t m_offset; }; @@ -81,10 +81,10 @@ class Regex { bool hasError() const { return (m_pc == NULL); } - std::list searchAll(std::string_view s) const; + std::vector searchAll(std::string_view s) const; RegexResult searchOneMatch(std::string_view s, std::vector& captures, unsigned long match_limit = 0) const; RegexResult searchGlobal(std::string_view s, std::vector& captures, unsigned long match_limit = 0) const; - int search(std::string_view s, SMatch *match) const; + int search(std::string_view s, SMatch &match) const; int search(std::string_view s) const; const std::string pattern; @@ -101,7 +101,7 @@ class Regex { }; -static inline int regex_search(const std::string& s, SMatch *match, const Regex& regex) { +static inline int regex_search(const std::string& s, SMatch &match, const Regex& regex) { return regex.search(s, match); } diff --git a/test/regression/regression.cc b/test/regression/regression.cc index f8acffb19..34d43b82a 100644 --- a/test/regression/regression.cc +++ b/test/regression/regression.cc @@ -213,7 +213,7 @@ void perform_unit_test(const ModSecurityTest &test, SMatch match; const auto s = context.m_modsec_rules.getParserError(); - if (regex_search(s, &match, re)) { + if (regex_search(s, match, re)) { if (test.m_automake_output) { std::cout << ":test-result: PASS " << filename \ << ":" << t->name << std::endl; diff --git a/test/unit/unit_test.cc b/test/unit/unit_test.cc index d7c86c538..0cbccae64 100644 --- a/test/unit/unit_test.cc +++ b/test/unit/unit_test.cc @@ -30,48 +30,37 @@ namespace modsecurity_test { -void replaceAll(std::string *s, const std::string &search, +static inline void replaceAll(std::string &s, const std::string &search, const char replace) { for (size_t pos = 0; ; pos += 0) { - pos = s->find(search, pos); + pos = s.find(search, pos); if (pos == std::string::npos) { break; } - s->erase(pos, search.length()); - s->insert(pos, &replace, 1); + s.erase(pos, search.length()); + s.insert(pos, &replace, 1); } } - -void json2bin(std::string *str) { - modsecurity::Utils::Regex re("\\\\x([a-z0-9A-Z]{2})"); - modsecurity::Utils::Regex re2("\\\\u([a-z0-9A-Z]{4})"); +static inline void jsonReplace(std::string &str, const modsecurity::Utils::Regex &re, const char *fmt) { modsecurity::Utils::SMatch match; - while (modsecurity::Utils::regex_search(*str, &match, re)) { - unsigned int p; - std::string toBeReplaced = match.str(); + while (modsecurity::Utils::regex_search(str, match, re)) { + const auto search = std::string(match.str()); + auto toBeReplaced = search; toBeReplaced.erase(0, 2); - sscanf(toBeReplaced.c_str(), "%3x", &p); - replaceAll(str, match.str(), p); - } - - while (modsecurity::Utils::regex_search(*str, &match, re2)) { unsigned int p; - std::string toBeReplaced = match.str(); - toBeReplaced.erase(0, 2); - sscanf(toBeReplaced.c_str(), "%4x", &p); - replaceAll(str, match.str(), p); + sscanf(toBeReplaced.c_str(), fmt, &p); + replaceAll(str, search, p); } +} - /* - replaceAll(str, "\\0", '\0'); - replaceAll(str, "\\b", '\b'); - replaceAll(str, "\\t", '\t'); - replaceAll(str, "\\n", '\n'); - replaceAll(str, "\\r", '\r'); - */ -// replaceAll(str, "\\f", '\f'); +void json2bin(std::string &str) { + modsecurity::Utils::Regex re("\\\\x([a-z0-9A-Z]{2})"); + jsonReplace(str, re, "%3x"); + + modsecurity::Utils::Regex re2("\\\\u([a-z0-9A-Z]{4})"); + jsonReplace(str, re2, "%4x"); } @@ -118,7 +107,7 @@ UnitTest *UnitTest::from_yajl_node(const yajl_val &node) { u->param = YAJL_GET_STRING(val); } else if (strcmp(key, "input") == 0) { u->input = YAJL_GET_STRING(val); - json2bin(&u->input); + json2bin(u->input); } else if (strcmp(key, "resource") == 0) { u->resource = YAJL_GET_STRING(val); } else if (strcmp(key, "name") == 0) { @@ -129,7 +118,7 @@ UnitTest *UnitTest::from_yajl_node(const yajl_val &node) { u->ret = YAJL_GET_INTEGER(val); } else if (strcmp(key, "output") == 0) { u->output = std::string(YAJL_GET_STRING(val)); - json2bin(&u->output); + json2bin(u->output); /* * Converting \\u0000 to \0 due to the following gcc bug: * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53690