github_mirrors/ModSecurity
3
0
Fork 1
mirror of https://github.com/owasp-modsecurity/ModSecurity.git synced 2025-09-30 11:44:32 +03:00
Code Issues Packages Projects Releases Wiki Activity

Reference RuleWithActions & Transaction object instead of copying values in RuleMessage

Browse Source 
- Because the lifetime of the RuleMessage instances do not extend beyond
  the lifetime of the enclosing RuleWithActions & Transaction,
  RuleMessage can just reference it and simplify its definition.
- Additionally, make the references const to show that it doesn't modify it.
- Replace RuleMessage copy constructor with default implementations.
- Removed unused RuleMessage assignment operator (which cannot be implemented
  now that it has reference members).
- Removed constructor from RuleMessage pointer.
- Addressed Sonarcloud suggestions: Do not use the constructor's
  initializer list for data member "xxx". Use the in-class initializer
  instead.
This commit is contained in:
Eduardo Arias
2024-05-05 15:15:47 -03:00
parent 2ec640fd76
commit 2ad87f640f
11 changed files with 85 additions and 205 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/audit_log/writer/parallel.cc
View File

@@ -123,7 +123,7 @@ bool Parallel::write(Transaction *transaction, int parts, std::string *error) {
}
const auto &logPath = m_audit->m_storage_dir;
fileName = logPath + fileName + "-" + *transaction->m_id.get();
fileName = logPath + fileName + "-" + transaction->m_id;
if (logPath.empty()) {
error->assign("Log path is not valid.");

2
src/request_body_processor/multipart.cc
View File

@@ -74,7 +74,7 @@ void MultipartPartTmpFile::Open() {
strftime(tstr, std::size(tstr), "/%Y%m%d-%H%M%S", &timeinfo);
std::string path = m_transaction->m_rules->m_uploadDirectory.m_value;
path = path + tstr + "-" + *m_transaction->m_id.get();
path = path + tstr + "-" + m_transaction->m_id;
path += "-file-XXXXXX";
#ifndef WIN32

32
src/rule_message.cc
View File

@@ -26,26 +26,26 @@ namespace modsecurity {
std::string RuleMessage::_details(const RuleMessage *rm) {
std::string msg;
msg.append(" [file \"" + std::string(*rm->m_ruleFile.get()) + "\"]");
msg.append(" [line \"" + std::to_string(rm->m_ruleLine) + "\"]");
msg.append(" [id \"" + std::to_string(rm->m_ruleId) + "\"]");
msg.append(" [rev \"" + utils::string::toHexIfNeeded(rm->m_rev, true) + "\"]");
msg.append(" [file \"" + rm->m_rule.getFileName() + "\"]");
msg.append(" [line \"" + std::to_string(rm->m_rule.getLineNumber()) + "\"]");
msg.append(" [id \"" + std::to_string(rm->m_rule.m_ruleId) + "\"]");
msg.append(" [rev \"" + utils::string::toHexIfNeeded(rm->m_rule.m_rev, true) + "\"]");
msg.append(" [msg \"" + rm->m_message + "\"]");
msg.append(" [data \"" + utils::string::toHexIfNeeded(utils::string::limitTo(200, rm->m_data), true) + "\"]");
msg.append(" [severity \"" +
std::to_string(rm->m_severity) + "\"]");
msg.append(" [ver \"" + utils::string::toHexIfNeeded(rm->m_ver, true) + "\"]");
msg.append(" [maturity \"" + std::to_string(rm->m_maturity) + "\"]");
msg.append(" [accuracy \"" + std::to_string(rm->m_accuracy) + "\"]");
msg.append(" [ver \"" + utils::string::toHexIfNeeded(rm->m_rule.m_ver, true) + "\"]");
msg.append(" [maturity \"" + std::to_string(rm->m_rule.m_maturity) + "\"]");
msg.append(" [accuracy \"" + std::to_string(rm->m_rule.m_accuracy) + "\"]");
for (const auto &a : rm->m_tags) {
msg.append(" [tag \"" + utils::string::toHexIfNeeded(a, true) + "\"]");
}
msg.append(" [hostname \"" + *rm->m_requestHostName.get() + "\"]");
msg.append(" [uri \"" + utils::string::limitTo(200, *rm->m_uriNoQueryStringDecoded.get()) + "\"]");
msg.append(" [unique_id \"" + *rm->m_id + "\"]");
msg.append(" [hostname \"" + rm->m_transaction.m_requestHostName \
+ "\"]");
msg.append(" [uri \"" + utils::string::limitTo(200, rm->m_transaction.m_uri_no_query_string_decoded) + "\"]");
msg.append(" [unique_id \"" + rm->m_transaction.m_id + "\"]");
msg.append(" [ref \"" + utils::string::limitTo(200, rm->m_reference) + "\"]");
return msg;
@@ -55,9 +55,9 @@ std::string RuleMessage::_details(const RuleMessage *rm) {
std::string RuleMessage::_errorLogTail(const RuleMessage *rm) {
std::string msg;
msg.append("[hostname \"" + *rm->m_serverIpAddress.get() + "\"]");
msg.append(" [uri \"" + utils::string::limitTo(200, *rm->m_uriNoQueryStringDecoded.get()) + "\"]");
msg.append(" [unique_id \"" + *rm->m_id + "\"]");
msg.append("[hostname \"" + rm->m_transaction.m_serverIpAddress + "\"]");
msg.append(" [uri \"" + utils::string::limitTo(200, rm->m_transaction.m_uri_no_query_string_decoded) + "\"]");
msg.append(" [unique_id \"" + rm->m_transaction.m_id + "\"]");
return msg;
}
@@ -68,7 +68,7 @@ std::string RuleMessage::log(const RuleMessage *rm, int props, int code) {
msg.reserve(2048);
if (props & ClientLogMessageInfo) {
msg.append("[client " + std::string(*rm->m_clientIpAddress.get()) + "] ");
msg.append("[client " + rm->m_transaction.m_clientIpAddress + "] ");
}
if (rm->m_isDisruptive) {
@@ -79,7 +79,7 @@ std::string RuleMessage::log(const RuleMessage *rm, int props, int code) {
msg.append(std::to_string(code));
}
msg.append(" (phase ");
msg.append(std::to_string(rm->m_rule->getPhase() - 1) + "). ");
msg.append(std::to_string(rm->getPhase()) + "). ");
} else {
msg.append("ModSecurity: Warning. ");
}

6
src/rule_with_actions.cc
View File

@@ -179,7 +179,7 @@ RuleWithActions::~RuleWithActions() {
bool RuleWithActions::evaluate(Transaction *transaction) {
return evaluate(transaction, std::make_shared<RuleMessage>(this, transaction));
return evaluate(transaction, std::make_shared<RuleMessage>(*this, *transaction));
}
@@ -494,7 +494,7 @@ std::vector<actions::Action *> RuleWithActions::getActionsByName(const std::stri
void RuleWithActions::performLogging(Transaction *trans,
std::shared_ptr<RuleMessage> ruleMessage,
bool lastLog,
bool chainedParentNull) {
bool chainedParentNull) const {
/* last rule in the chain. */
bool isItToBeLogged = ruleMessage->m_saveMessage;
@@ -551,7 +551,7 @@ void RuleWithActions::performLogging(Transaction *trans,
trans->serverLog(ruleMessage);
}
RuleMessage *rm = new RuleMessage(this, trans);
RuleMessage *rm = new RuleMessage(*this, *trans);
rm->m_saveMessage = ruleMessage->m_saveMessage;
ruleMessage.reset(rm);
}

75
src/transaction.cc
View File

@@ -104,12 +104,12 @@ namespace modsecurity {
*/
Transaction::Transaction(ModSecurity *ms, RulesSet *rules, void *logCbData)
: m_creationTimeStamp(utils::cpu_seconds()),
m_clientIpAddress(std::make_shared<std::string>("")),
m_clientIpAddress(""),
m_httpVersion(""),
m_serverIpAddress(std::make_shared<std::string>("")),
m_requestHostName(std::make_shared<std::string>("")),
m_serverIpAddress(""),
m_requestHostName(""),
m_uri(""),
m_uri_no_query_string_decoded(std::make_shared<std::string>("")),
m_uri_no_query_string_decoded(""),
m_ARGScombinedSizeDouble(0),
m_clientPort(0),
m_highestSeverityAction(255),
@@ -166,9 +166,8 @@ Transaction::Transaction(ModSecurity *ms, RulesSet *rules, void *logCbData)
m_variableTimeYear(""),
m_logCbData(logCbData),
TransactionAnchoredVariables(this) {
m_id = std::unique_ptr<std::string>( new std::string(
std::to_string(m_timeStamp)
+ std::to_string(modsecurity::utils::generate_transaction_unique_id())));
m_id = std::to_string(m_timeStamp) +
std::to_string(modsecurity::utils::generate_transaction_unique_id());
m_variableUrlEncodedError.set("0", 0);
m_variableMscPcreError.set("0", 0);
@@ -181,12 +180,12 @@ Transaction::Transaction(ModSecurity *ms, RulesSet *rules, void *logCbData)
Transaction::Transaction(ModSecurity *ms, RulesSet *rules, char *id, void *logCbData)
: m_creationTimeStamp(utils::cpu_seconds()),
m_clientIpAddress(std::make_shared<std::string>("")),
m_clientIpAddress(""),
m_httpVersion(""),
m_serverIpAddress(std::make_shared<std::string>("")),
m_requestHostName(std::make_shared<std::string>("")),
m_serverIpAddress(""),
m_requestHostName(""),
m_uri(""),
m_uri_no_query_string_decoded(std::make_shared<std::string>("")),
m_uri_no_query_string_decoded(""),
m_ARGScombinedSizeDouble(0),
m_clientPort(0),
m_highestSeverityAction(255),
@@ -207,7 +206,7 @@ Transaction::Transaction(ModSecurity *ms, RulesSet *rules, char *id, void *logCb
m_rulesMessages(),
m_requestBody(),
m_responseBody(),
m_id(std::unique_ptr<std::string>(new std::string(id))),
m_id(id),
m_skip_next(0),
m_allowType(modsecurity::actions::disruptive::NoneAllowType),
m_uri_decoded(""),
@@ -292,7 +291,7 @@ void Transaction::debug(int level, const std::string& message) const {
return;
}
m_rules->debug(level, *m_id.get(), m_uri, message);
m_rules->debug(level, m_id, m_uri, message);
}
#endif
@@ -319,19 +318,19 @@ void Transaction::debug(int level, const std::string& message) const {
*/
int Transaction::processConnection(const char *client, int cPort,
const char *server, int sPort) {
m_clientIpAddress = std::unique_ptr<std::string>(new std::string(client));
m_serverIpAddress = std::unique_ptr<std::string>(new std::string(server));
m_requestHostName = std::unique_ptr<std::string>(new std::string(server));
m_clientIpAddress = client;
m_serverIpAddress = server;
m_requestHostName = server;
this->m_clientPort = cPort;
this->m_serverPort = sPort;
ms_dbg(4, "Transaction context created.");
ms_dbg(4, "Starting phase CONNECTION. (SecRules 0)");
m_variableRemoteHost.set(*m_clientIpAddress.get(), m_variableOffset);
m_variableUniqueID.set(*m_id.get(), m_variableOffset);
m_variableRemoteAddr.set(*m_clientIpAddress.get(), m_variableOffset);
m_variableServerAddr.set(*m_serverIpAddress.get(), m_variableOffset);
m_variableRemoteHost.set(m_clientIpAddress, m_variableOffset);
m_variableUniqueID.set(m_id, m_variableOffset);
m_variableRemoteAddr.set(m_clientIpAddress, m_variableOffset);
m_variableServerAddr.set(m_serverIpAddress, m_variableOffset);
m_variableServerPort.set(std::to_string(this->m_serverPort),
m_variableOffset);
m_variableRemotePort.set(std::to_string(this->m_clientPort),
@@ -467,9 +466,7 @@ int Transaction::processURI(const char *uri, const char *method,
m_variableRequestProtocol.set("HTTP/" + std::string(http_version),
m_variableOffset + requestLine.size() + 1);
m_uri_no_query_string_decoded = std::unique_ptr<std::string>(
new std::string(path_info));
m_uri_no_query_string_decoded = path_info;
if (pos_raw_query != std::string::npos) {
std::string qry = std::string(uri_s, pos_raw_query + 1,
@@ -1495,7 +1492,7 @@ std::string Transaction::toOldAuditLogFormatIndex(const std::string &filename,
ss << utils::string::dash_if_empty(
m_variableRequestHeaders.resolveFirst("Host").get())
<< " ";
ss << utils::string::dash_if_empty(this->m_clientIpAddress.get()) << " ";
ss << utils::string::dash_if_empty(&this->m_clientIpAddress) << " ";
/** TODO: Check variable */
variables::RemoteUser *r = new variables::RemoteUser("REMOTE_USER");
std::vector<const VariableValue *> l;
@@ -1530,7 +1527,7 @@ std::string Transaction::toOldAuditLogFormatIndex(const std::string &filename,
ss << utils::string::dash_if_empty(
m_variableRequestHeaders.resolveFirst("User-Agent").get());
ss << "\" ";
ss << *m_id.get() << " ";
ss << m_id << " ";
/** TODO: Check variable */
ss << utils::string::dash_if_empty(
m_variableRequestHeaders.resolveFirst("REFERER").get()) << " ";
@@ -1556,10 +1553,10 @@ std::string Transaction::toOldAuditLogFormat(int parts,
audit_log << "--" << trailer << "-" << "A--" << std::endl;
audit_log << tstr;
audit_log << " " << m_id->c_str();
audit_log << " " << this->m_clientIpAddress->c_str();
audit_log << " " << m_id;
audit_log << " " << this->m_clientIpAddress;
audit_log << " " << this->m_clientPort;
audit_log << " " << m_serverIpAddress->c_str();
audit_log << " " << m_serverIpAddress;
audit_log << " " << this->m_serverPort;
audit_log << std::endl;
@@ -1676,13 +1673,13 @@ std::string Transaction::toJSON(int parts) {
yajl_gen_map_open(g);
/* Part: A (header mandatory) */
LOGFY_ADD("client_ip", this->m_clientIpAddress->c_str());
LOGFY_ADD("client_ip", m_clientIpAddress.c_str());
LOGFY_ADD("time_stamp", ts.c_str());
LOGFY_ADD("server_id", uniqueId.c_str());
LOGFY_ADD_NUM("client_port", m_clientPort);
LOGFY_ADD("host_ip", m_serverIpAddress->c_str());
LOGFY_ADD("host_ip", m_serverIpAddress.c_str());
LOGFY_ADD_NUM("host_port", m_serverPort);
LOGFY_ADD("unique_id", m_id->c_str());
LOGFY_ADD("unique_id", m_id.c_str());
/* request */
yajl_gen_string(g, reinterpret_cast<const unsigned char*>("request"),
@@ -1797,13 +1794,13 @@ std::string Transaction::toJSON(int parts) {
yajl_gen_map_open(g);
LOGFY_ADD("match", a.m_match.c_str());
LOGFY_ADD("reference", a.m_reference.c_str());
LOGFY_ADD("ruleId", std::to_string(a.m_ruleId).c_str());
LOGFY_ADD("file", a.m_ruleFile->c_str());
LOGFY_ADD("lineNumber", std::to_string(a.m_ruleLine).c_str());
LOGFY_ADD("ruleId", std::to_string(a.m_rule.m_ruleId).c_str());
LOGFY_ADD("file", a.m_rule.getFileName().c_str());
LOGFY_ADD("lineNumber", std::to_string(a.m_rule.getLineNumber()).c_str());
LOGFY_ADD("data", a.m_data.c_str());
LOGFY_ADD("severity", std::to_string(a.m_severity).c_str());
LOGFY_ADD("ver", a.m_ver.c_str());
LOGFY_ADD("rev", a.m_rev.c_str());
LOGFY_ADD("ver", a.m_rule.m_ver.c_str());
LOGFY_ADD("rev", a.m_rule.m_rev.c_str());
yajl_gen_string(g,
reinterpret_cast<const unsigned char*>("tags"),
@@ -1816,8 +1813,8 @@ std::string Transaction::toJSON(int parts) {
}
yajl_gen_array_close(g);
LOGFY_ADD("maturity", std::to_string(a.m_maturity).c_str());
LOGFY_ADD("accuracy", std::to_string(a.m_accuracy).c_str());
LOGFY_ADD("maturity", std::to_string(a.m_rule.m_maturity).c_str());
LOGFY_ADD("accuracy", std::to_string(a.m_rule.m_accuracy).c_str());
yajl_gen_map_close(g);
yajl_gen_map_close(g);
}
@@ -2384,7 +2381,7 @@ extern "C" int msc_update_status_code(Transaction *transaction, int status) {
int Transaction::setRequestHostName(const std::string& hostname) {
if (hostname != "") {
m_requestHostName = std::unique_ptr<std::string>(new std::string(hostname));
m_requestHostName = hostname;
}
return true;