github_mirrors/ModSecurity
3
0
Fork 1
mirror of https://github.com/owasp-modsecurity/ModSecurity.git synced 2025-08-14 05:45:59 +03:00
Code Issues Packages Projects Releases Wiki Activity

Respond to code review feedback

Browse Source
This commit is contained in:
Brandon Payton 2023-01-19 18:35:08 -05:00
parent 0c42ee229e
commit f3d8198b84
12 changed files with 2380 additions and 2398 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
headers/modsecurity/transaction.h
View File

@ -134,7 +134,7 @@ class TransactionAnchoredVariables {
m_variableInboundDataError(t, "INBOUND_DATA_ERROR"),
m_variableMatchedVar(t, "MATCHED_VAR"),
m_variableMatchedVarName(t, "MATCHED_VAR_NAME"),
m_variableMscPcreErrored(t, "MSC_PCRE_ERRORED"),
m_variableMscPcreError(t, "MSC_PCRE_ERROR"),
m_variableMscPcreLimitsExceeded(t, "MSC_PCRE_LIMITS_EXCEEDED"),
m_variableMultipartBoundaryQuoted(t, "MULTIPART_BOUNDARY_QUOTED"),
m_variableMultipartBoundaryWhiteSpace(t,
@ -221,7 +221,7 @@ class TransactionAnchoredVariables {
AnchoredVariable m_variableInboundDataError;
AnchoredVariable m_variableMatchedVar;
AnchoredVariable m_variableMatchedVarName;
AnchoredVariable m_variableMscPcreErrored;
AnchoredVariable m_variableMscPcreError;
AnchoredVariable m_variableMscPcreLimitsExceeded;
AnchoredVariable m_variableMultipartBoundaryQuoted;
AnchoredVariable m_variableMultipartBoundaryWhiteSpace;

2
src/operators/rx.cc
View File

@ -68,7 +68,7 @@ bool Rx::evaluate(Transaction *transaction, RuleWithActions *rule,
// FIXME: DRY regex error reporting. This logic is currently duplicated in other operators.
if (regex_result != Utils::RegexResult::Ok) {
transaction->m_variableMscPcreErrored.set("1", transaction->m_variableOffset);
transaction->m_variableMscPcreError.set("1", transaction->m_variableOffset);
std::string regex_error_str = "OTHER";
if (regex_result == Utils::RegexResult::ErrorMatchLimit) {

2
src/operators/rx_global.cc
View File

@ -62,7 +62,7 @@ bool RxGlobal::evaluate(Transaction *transaction, RuleWithActions *rule,
// FIXME: DRY regex error reporting. This logic is currently duplicated in other operators.
if (regex_result != Utils::RegexResult::Ok) {
transaction->m_variableMscPcreErrored.set("1", transaction->m_variableOffset);
transaction->m_variableMscPcreError.set("1", transaction->m_variableOffset);
std::string regex_error_str = "OTHER";
if (regex_result == Utils::RegexResult::ErrorMatchLimit) {

6
src/parser/seclang-parser.cc
View File

@ -4279,10 +4279,10 @@ namespace yy {
#line 4280 "seclang-parser.cc"
break;
case 275: // var: "MSC_PCRE_ERRORED"
case 275: // var: "MSC_PCRE_ERROR"
#line 2329 "seclang-parser.yy"
{
VARIABLE_CONTAINER(yylhs.value.as < std::unique_ptr<Variable> > (), new variables::MscPcreErrored());
VARIABLE_CONTAINER(yylhs.value.as < std::unique_ptr<Variable> > (), new variables::MscPcreError());
}
#line 4288 "seclang-parser.cc"
break;
@ -7060,7 +7060,7 @@ namespace yy {
"VARIABLE_RULE", "\"Variable ARGS_NAMES\"", "VARIABLE_ARGS_POST_NAMES",
"\"AUTH_TYPE\"", "\"FILES_COMBINED_SIZE\"", "\"FILES_TMPNAMES\"",
"\"FULL_REQUEST\"", "\"FULL_REQUEST_LENGTH\"", "\"INBOUND_DATA_ERROR\"",
"\"MATCHED_VAR\"", "\"MATCHED_VAR_NAME\"", "\"MSC_PCRE_ERRORED\"",
"\"MATCHED_VAR\"", "\"MATCHED_VAR_NAME\"", "\"MSC_PCRE_ERROR\"",
"\"MSC_PCRE_LIMITS_EXCEEDED\"", "VARIABLE_MULTIPART_BOUNDARY_QUOTED",
"VARIABLE_MULTIPART_BOUNDARY_WHITESPACE", "\"MULTIPART_CRLF_LF_LINES\"",
"\"MULTIPART_DATA_AFTER\"", "VARIABLE_MULTIPART_DATA_BEFORE",

14
src/parser/seclang-parser.hh
View File

@ -223,7 +223,7 @@ class Driver;
#include "src/variables/matched_vars.h"
#include "src/variables/matched_vars_names.h"
#include "src/variables/modsec_build.h"
#include "src/variables/msc_pcre_errored.h"
#include "src/variables/msc_pcre_error.h"
#include "src/variables/msc_pcre_limits_exceeded.h"
#include "src/variables/multipart_boundary_quoted.h"
#include "src/variables/multipart_boundary_whitespace.h"
@ -1025,7 +1025,7 @@ namespace yy {
TOK_VARIABLE_INBOUND_DATA_ERROR = 292, // "INBOUND_DATA_ERROR"
TOK_VARIABLE_MATCHED_VAR = 293, // "MATCHED_VAR"
TOK_VARIABLE_MATCHED_VAR_NAME = 294, // "MATCHED_VAR_NAME"
TOK_VARIABLE_MSC_PCRE_ERRORED = 295, // "MSC_PCRE_ERRORED"
TOK_VARIABLE_MSC_PCRE_ERROR = 295, // "MSC_PCRE_ERROR"
TOK_VARIABLE_MSC_PCRE_LIMITS_EXCEEDED = 296, // "MSC_PCRE_LIMITS_EXCEEDED"
TOK_VARIABLE_MULTIPART_BOUNDARY_QUOTED = 297, // VARIABLE_MULTIPART_BOUNDARY_QUOTED
TOK_VARIABLE_MULTIPART_BOUNDARY_WHITESPACE = 298, // VARIABLE_MULTIPART_BOUNDARY_WHITESPACE
@ -1391,7 +1391,7 @@ namespace yy {
S_VARIABLE_INBOUND_DATA_ERROR = 37, // "INBOUND_DATA_ERROR"
S_VARIABLE_MATCHED_VAR = 38, // "MATCHED_VAR"
S_VARIABLE_MATCHED_VAR_NAME = 39, // "MATCHED_VAR_NAME"
S_VARIABLE_MSC_PCRE_ERRORED = 40, // "MSC_PCRE_ERRORED"
S_VARIABLE_MSC_PCRE_ERROR = 40, // "MSC_PCRE_ERROR"
S_VARIABLE_MSC_PCRE_LIMITS_EXCEEDED = 41, // "MSC_PCRE_LIMITS_EXCEEDED"
S_VARIABLE_MULTIPART_BOUNDARY_QUOTED = 42, // VARIABLE_MULTIPART_BOUNDARY_QUOTED
S_VARIABLE_MULTIPART_BOUNDARY_WHITESPACE = 43, // VARIABLE_MULTIPART_BOUNDARY_WHITESPACE
@ -3122,16 +3122,16 @@ switch (yykind)
#if 201103L <= YY_CPLUSPLUS
static
symbol_type
make_VARIABLE_MSC_PCRE_ERRORED (location_type l)
make_VARIABLE_MSC_PCRE_ERROR (location_type l)
{
return symbol_type (token::TOK_VARIABLE_MSC_PCRE_ERRORED, std::move (l));
return symbol_type (token::TOK_VARIABLE_MSC_PCRE_ERROR, std::move (l));
}
#else
static
symbol_type
make_VARIABLE_MSC_PCRE_ERRORED (const location_type& l)
make_VARIABLE_MSC_PCRE_ERROR (const location_type& l)
{
return symbol_type (token::TOK_VARIABLE_MSC_PCRE_ERRORED, l);
return symbol_type (token::TOK_VARIABLE_MSC_PCRE_ERROR, l);
}
#endif
#if 201103L <= YY_CPLUSPLUS

8
src/parser/seclang-parser.yy
View File

@ -184,7 +184,7 @@ class Driver;
#include "src/variables/matched_vars.h"
#include "src/variables/matched_vars_names.h"
#include "src/variables/modsec_build.h"
#include "src/variables/msc_pcre_errored.h"
#include "src/variables/msc_pcre_error.h"
#include "src/variables/msc_pcre_limits_exceeded.h"
#include "src/variables/multipart_boundary_quoted.h"
#include "src/variables/multipart_boundary_whitespace.h"
@ -370,7 +370,7 @@ using namespace modsecurity::operators;
VARIABLE_INBOUND_DATA_ERROR "INBOUND_DATA_ERROR"
VARIABLE_MATCHED_VAR "MATCHED_VAR"
VARIABLE_MATCHED_VAR_NAME "MATCHED_VAR_NAME"
VARIABLE_MSC_PCRE_ERRORED "MSC_PCRE_ERRORED"
VARIABLE_MSC_PCRE_ERROR "MSC_PCRE_ERROR"
VARIABLE_MSC_PCRE_LIMITS_EXCEEDED "MSC_PCRE_LIMITS_EXCEEDED"
VARIABLE_MULTIPART_BOUNDARY_QUOTED
VARIABLE_MULTIPART_BOUNDARY_WHITESPACE
@ -2325,9 +2325,9 @@ var:
{
VARIABLE_CONTAINER($$, new variables::MatchedVarName());
}
| VARIABLE_MSC_PCRE_ERRORED
| VARIABLE_MSC_PCRE_ERROR
{
VARIABLE_CONTAINER($$, new variables::MscPcreErrored());
VARIABLE_CONTAINER($$, new variables::MscPcreError());
}
| VARIABLE_MSC_PCRE_LIMITS_EXCEEDED
{

4672
src/parser/seclang-scanner.cc
View File

File diff suppressed because it is too large Load Diff

4
src/parser/seclang-scanner.ll
View File

@ -186,7 +186,7 @@ VARIABLE_GLOBAL (?i:GLOBAL)
VARIABLE_INBOUND_DATA_ERROR (?i:INBOUND_DATA_ERROR)
VARIABLE_MATCHED_VAR (?i:MATCHED_VAR)
VARIABLE_MATCHED_VAR_NAME (?i:MATCHED_VAR_NAME)
VARIABLE_MSC_PCRE_ERRORED (?i:MSC_PCRE_ERRORED)
VARIABLE_MSC_PCRE_ERROR (?i:MSC_PCRE_ERROR)
VARIABLE_MSC_PCRE_LIMITS_EXCEEDED (?i:MSC_PCRE_LIMITS_EXCEEDED)
VARIABLE_MULTIPART_BOUNDARY_QUOTED (?i:MULTIPART_BOUNDARY_QUOTED)
VARIABLE_MULTIPART_BOUNDARY_WHITESPACE (?i:MULTIPART_BOUNDARY_WHITESPACE)
@ -912,7 +912,7 @@ EQUALS_MINUS (?i:=\-)
{VARIABLE_INBOUND_DATA_ERROR} { return p::make_VARIABLE_INBOUND_DATA_ERROR(*driver.loc.back()); }
{VARIABLE_MATCHED_VAR_NAME} { return p::make_VARIABLE_MATCHED_VAR_NAME(*driver.loc.back()); }
{VARIABLE_MATCHED_VAR} { return p::make_VARIABLE_MATCHED_VAR(*driver.loc.back()); }
{VARIABLE_MSC_PCRE_ERRORED} { return p::make_VARIABLE_MSC_PCRE_ERRORED(*driver.loc.back()); }
{VARIABLE_MSC_PCRE_ERROR} { return p::make_VARIABLE_MSC_PCRE_ERROR(*driver.loc.back()); }
{VARIABLE_MSC_PCRE_LIMITS_EXCEEDED} { return p::make_VARIABLE_MSC_PCRE_LIMITS_EXCEEDED(*driver.loc.back()); }
{VARIABLE_MULTIPART_BOUNDARY_QUOTED} { return p::make_VARIABLE_MULTIPART_BOUNDARY_QUOTED(*driver.loc.back()); }
{VARIABLE_MULTIPART_BOUNDARY_WHITESPACE} { return p::make_VARIABLE_MULTIPART_BOUNDARY_WHITESPACE(*driver.loc.back()); }

43
src/utils/regex.cc
View File

@ -42,6 +42,7 @@ class Pcre2MatchContextPtr {
: m_match_context(pcre2_match_context_create(NULL)) {}
Pcre2MatchContextPtr(const Pcre2MatchContextPtr&) = delete;
Pcre2MatchContextPtr& operator=(const Pcre2MatchContextPtr&) = delete;
~Pcre2MatchContextPtr() {
pcre2_match_context_free(m_match_context);
@ -188,16 +189,13 @@ std::list<SMatch> Regex::searchAll(const std::string& s) const {
return retList;
}
RegexResult Regex::searchOneMatch(const std::string& s, std::vector<SMatchCapture>& captures) const {
return searchOneMatch(s, captures, get_default_match_limit());
}
RegexResult Regex::searchOneMatch(const std::string& s, std::vector<SMatchCapture>& captures, unsigned long match_limit) const {
#ifdef WITH_PCRE2
Pcre2MatchContextPtr match_context;
// TODO: What if setting the match limit fails?
pcre2_set_match_limit(match_context, match_limit);
if (match_limit > 0) {
// TODO: What if setting the match limit fails?
pcre2_set_match_limit(match_context, match_limit);
}
PCRE2_SPTR pcre2_s = reinterpret_cast<PCRE2_SPTR>(s.c_str());
pcre2_match_data *match_data = pcre2_match_data_create_from_pattern(m_pc, NULL);
@ -214,9 +212,9 @@ RegexResult Regex::searchOneMatch(const std::string& s, std::vector<SMatchCaptur
const char *subject = s.c_str();
int ovector[OVECCOUNT];
pcre_extra local_pce;
pcre_extra *pce = NULL;
pcre_extra *pce = m_pce;
if (m_pce != NULL) {
if (m_pce != NULL && match_limit > 0) {
local_pce = *m_pce;
local_pce.match_limit = match_limit;
local_pce.flags |= PCRE_EXTRA_MATCH_LIMIT;
@ -243,18 +241,16 @@ RegexResult Regex::searchOneMatch(const std::string& s, std::vector<SMatchCaptur
return to_regex_result(rc);
}
RegexResult Regex::searchGlobal(const std::string& s, std::vector<SMatchCapture>& captures) const {
return searchGlobal(s, captures, get_default_match_limit());
}
RegexResult Regex::searchGlobal(const std::string& s, std::vector<SMatchCapture>& captures, unsigned long match_limit) const {
const char *subject = s.c_str();
bool prev_match_zero_length = false;
#ifdef WITH_PCRE2
Pcre2MatchContextPtr match_context;
// TODO: What if setting the match limit fails?
pcre2_set_match_limit(match_context, match_limit);
if (match_limit > 0) {
// TODO: What if setting the match limit fails?
pcre2_set_match_limit(match_context, match_limit);
}
PCRE2_SPTR pcre2_s = reinterpret_cast<PCRE2_SPTR>(s.c_str());
PCRE2_SIZE startOffset = 0;
@ -271,9 +267,9 @@ RegexResult Regex::searchGlobal(const std::string& s, std::vector<SMatchCapture>
#else
pcre_extra local_pce;
pcre_extra *pce = NULL;
pcre_extra *pce = m_pce;
if (m_pce != NULL) {
if (m_pce != NULL && match_limit > 0) {
local_pce = *m_pce;
local_pce.match_limit = match_limit;
local_pce.flags |= PCRE_EXTRA_MATCH_LIMIT;
@ -407,19 +403,6 @@ int Regex::search(const std::string& s) const {
#endif
}
unsigned long Regex::get_default_match_limit() const {
unsigned long default_match_limit;
#ifdef WITH_PCRE2
int ret = pcre2_config(PCRE2_CONFIG_MATCHLIMIT, &default_match_limit);
#else
int ret = pcre_config(PCRE_CONFIG_MATCH_LIMIT, &default_match_limit);
#endif
if (ret < 0) {
default_match_limit = 10000000;
}
return default_match_limit;
}
RegexResult Regex::to_regex_result(int pcre_exec_result) const {
if (
pcre_exec_result > 0 ||

7
src/utils/regex.h
View File

@ -82,16 +82,13 @@ class Regex {
return (m_pc == NULL);
}
std::list<SMatch> searchAll(const std::string& s) const;
RegexResult searchOneMatch(const std::string& s, std::vector<SMatchCapture>& captures) const;
RegexResult searchOneMatch(const std::string& s, std::vector<SMatchCapture>& captures, unsigned long match_limit) const;
RegexResult searchGlobal(const std::string& s, std::vector<SMatchCapture>& captures) const;
RegexResult searchGlobal(const std::string& s, std::vector<SMatchCapture>& captures, unsigned long match_limit) const;
RegexResult searchOneMatch(const std::string& s, std::vector<SMatchCapture>& captures, unsigned long match_limit = 0) const;
RegexResult searchGlobal(const std::string& s, std::vector<SMatchCapture>& captures, unsigned long match_limit = 0) const;
int search(const std::string &s, SMatch *match) const;
int search(const std::string &s) const;
const std::string pattern;
private:
unsigned long get_default_match_limit() const;
RegexResult to_regex_result(int pcre_exec_result) const;
#if WITH_PCRE2

8
src/variables/msc_pcre_errored.h → src/variables/msc_pcre_error.h
View File

@ -19,8 +19,8 @@
#include <list>
#include <utility>
#ifndef SRC_VARIABLES_MSC_PCRE_ERRORED_H_
#define SRC_VARIABLES_MSC_PCRE_ERRORED_H_
#ifndef SRC_VARIABLES_MSC_PCRE_ERROR_H_
#define SRC_VARIABLES_MSC_PCRE_ERROR_H_
#include "src/variables/variable.h"
@ -30,10 +30,10 @@ class Transaction;
namespace variables {
DEFINE_VARIABLE(MscPcreErrored, MSC_PCRE_ERRORED, m_variableMscPcreErrored)
DEFINE_VARIABLE(MscPcreError, MSC_PCRE_ERROR, m_variableMscPcreError)
} // namespace variables
} // namespace modsecurity
#endif // SRC_VARIABLES_MSC_PCRE_ERRORED_H_
#endif // SRC_VARIABLES_MSC_PCRE_ERROR_H_

8
src/variables/variable.h
View File

@ -202,8 +202,8 @@ class VariableMonkeyResolution {
t->m_variableMatchedVar.evaluate(l);
} else if (comp(variable, "MATCHED_VAR_NAME")) {
t->m_variableMatchedVarName.evaluate(l);
} else if (comp(variable, "MSC_PCRE_ERRORED")) {
t->m_variableMscPcreErrored.evaluate(l);
} else if (comp(variable, "MSC_PCRE_ERROR")) {
t->m_variableMscPcreError.evaluate(l);
} else if (comp(variable, "MSC_PCRE_LIMITS_EXCEEDED")) {
t->m_variableMscPcreLimitsExceeded.evaluate(l);
} else if (comp(variable, "MULTIPART_CRLF_LF_LINES")) {
@ -369,8 +369,8 @@ class VariableMonkeyResolution {
vv = t->m_variableMatchedVar.resolveFirst();
} else if (comp(variable, "MATCHED_VAR_NAME")) {
vv = t->m_variableMatchedVarName.resolveFirst();
} else if (comp(variable, "MSC_PCRE_ERRORED")) {
vv = t->m_variableMscPcreErrored.resolveFirst();
} else if (comp(variable, "MSC_PCRE_ERROR")) {
vv = t->m_variableMscPcreError.resolveFirst();
} else if (comp(variable, "MSC_PCRE_LIMITS_EXCEEDED")) {
vv = t->m_variableMscPcreLimitsExceeded.resolveFirst();
} else if (comp(variable, "MULTIPART_CRLF_LF_LINES")) {