Skip to content

Commit

Permalink
Merge pull request owasp-modsecurity#3280 from eduar-hte/range-checke…
Browse files Browse the repository at this point in the history
…d-at

Replace usage of range-checked 'at' method when vector/string has already been size checked
  • Loading branch information
airween authored Oct 19, 2024
2 parents 99ce977 + 0613cee commit ec506da
Show file tree
Hide file tree
Showing 11 changed files with 26 additions and 30 deletions.
8 changes: 2 additions & 6 deletions headers/modsecurity/anchored_set_variable_translation_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,9 @@ class AnchoredSetVariableTranslationProxy {
return nullptr;
}

std::unique_ptr<std::string> ret(new std::string(""));
auto ret = std::make_unique<std::string>(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;
}

Expand Down
6 changes: 3 additions & 3 deletions headers/modsecurity/rules.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/actions/xmlns.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
7 changes: 4 additions & 3 deletions src/operators/contains_word.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/operators/inspect_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
}

Expand Down
2 changes: 1 addition & 1 deletion src/operators/pm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/operators/validate_byte_range.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions src/rule_with_operator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/rules_set_phases.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<RuleWithOperator *>(m_rulesAtPhase[i].at(z).get());
for (const auto &r : m_rulesAtPhase[i].m_rules) {
const auto *rule_ckc = dynamic_cast<const RuleWithOperator *>(r.get());
if (!rule_ckc) {
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions src/transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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++;
}
Expand Down
14 changes: 7 additions & 7 deletions src/utils/string.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -177,22 +177,22 @@ 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;
}


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);
}
Expand Down

0 comments on commit ec506da

Please sign in to comment.