From 5e6fcbc60b378cb2901ebf24b024bce6978b6c96 Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Mon, 12 Aug 2024 07:58:06 -0700 Subject: [PATCH 1/2] Replace usage of std::ctime, which is not safe for use in multithreaded contexts - std::ctime returns a pointer to a string that "may be shared between std::asctime and std::ctime, and may be overwritten on each invocation of any of those functions.". - https://en.cppreference.com/w/cpp/chrono/c/ctime - Replaced with call to strftime to generate the same string representation (using the format string: %c) - Leveraged localtime_r (which is thread-safe) to convert time_t to struct tm, as required by strftime. --- src/utils/string.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/utils/string.h b/src/utils/string.h index 700f33b9dd..b0c3c17d97 100644 --- a/src/utils/string.h +++ b/src/utils/string.h @@ -21,6 +21,11 @@ #include #include #include +#include + +#ifdef WIN32 +#include "src/compat/msvc.h" +#endif #ifndef SRC_UTILS_STRING_H_ #define SRC_UTILS_STRING_H_ @@ -60,9 +65,11 @@ const char HEX2DEC[256] = { inline std::string ascTime(const time_t *t) { - std::string ts = std::ctime(t); - ts.pop_back(); - return ts; + struct tm timeinfo; + localtime_r(t, &timeinfo); + char tstr[std::size("Www Mmm dd hh:mm:ss yyyy")]; + strftime(tstr, std::size(tstr), "%c", &timeinfo); + return tstr; } From 23a341eb6a0d53148961ad412e3da7a12495523b Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Mon, 12 Aug 2024 08:47:04 -0700 Subject: [PATCH 2/2] Calculate sizes of strftime buffers based on format strings - Leverage std::size to determine buffer size at compile time. - Simplified 'TimeMon::evaluate' implementation as it was using strftime to get the month, convert the string to int, and then decrement it by one to make it zero based. This same value is already available in the 'struct tm' previously generated with the call to localtime_r (and where the month is already zero-based) --- src/audit_log/writer/parallel.cc | 23 +++++++++++------------ src/request_body_processor/multipart.cc | 6 +++--- src/transaction.cc | 15 +++++++-------- src/variables/time.cc | 8 +++----- src/variables/time_day.cc | 9 ++++----- src/variables/time_hour.cc | 9 ++++----- src/variables/time_min.cc | 9 ++++----- src/variables/time_mon.cc | 10 ++-------- src/variables/time_sec.cc | 9 ++++----- src/variables/time_wday.cc | 9 ++++----- src/variables/time_year.cc | 9 ++++----- 11 files changed, 50 insertions(+), 66 deletions(-) diff --git a/src/audit_log/writer/parallel.cc b/src/audit_log/writer/parallel.cc index d23e7958ae..5237adb74d 100644 --- a/src/audit_log/writer/parallel.cc +++ b/src/audit_log/writer/parallel.cc @@ -51,28 +51,27 @@ Parallel::~Parallel() { inline std::string Parallel::logFilePath(time_t *t, int part) { - struct tm timeinfo; - char tstr[300]; - std::string name(""); + std::string name; + struct tm timeinfo; localtime_r(t, &timeinfo); if (part & YearMonthDayDirectory) { - memset(tstr, '\0', 300); - strftime(tstr, 299, "/%Y%m%d", &timeinfo); - name = tstr; + char tstr[std::size("/yyyymmdd")]; + strftime(tstr, std::size(tstr), "/%Y%m%d", &timeinfo); + name.append(tstr); } if (part & YearMonthDayAndTimeDirectory) { - memset(tstr, '\0', 300); - strftime(tstr, 299, "/%Y%m%d-%H%M", &timeinfo); - name = name + tstr; + char tstr[std::size("/yyyymmdd-hhmm")]; + strftime(tstr, std::size(tstr), "/%Y%m%d-%H%M", &timeinfo); + name.append(tstr); } if (part & YearMonthDayAndTimeFileName) { - memset(tstr, '\0', 300); - strftime(tstr, 299, "/%Y%m%d-%H%M%S", &timeinfo); - name = name + tstr; + char tstr[std::size("/yyyymmdd-hhmmss")]; + strftime(tstr, std::size(tstr), "/%Y%m%d-%H%M%S", &timeinfo); + name.append(tstr); } return name; diff --git a/src/request_body_processor/multipart.cc b/src/request_body_processor/multipart.cc index c578638bc7..9a20b051ec 100644 --- a/src/request_body_processor/multipart.cc +++ b/src/request_body_processor/multipart.cc @@ -65,12 +65,12 @@ MultipartPartTmpFile::~MultipartPartTmpFile() { } void MultipartPartTmpFile::Open() { - struct tm timeinfo; - time_t tt = time(NULL); + time_t tt = time(nullptr); + struct tm timeinfo; localtime_r(&tt, &timeinfo); - char tstr[17]; + char tstr[std::size("/yyyymmdd-hhmmss")]; strftime(tstr, std::size(tstr), "/%Y%m%d-%H%M%S", &timeinfo); std::string path = m_transaction->m_rules->m_uploadDirectory.m_value; diff --git a/src/transaction.cc b/src/transaction.cc index bf24e2ad28..f69df1887a 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -1509,13 +1509,12 @@ bool Transaction::intervention(ModSecurityIntervention *it) { std::string Transaction::toOldAuditLogFormatIndex(const std::string &filename, double size, const std::string &md5) { std::stringstream ss; - struct tm timeinfo; - char tstr[300]; - memset(tstr, '\0', 300); + struct tm timeinfo; localtime_r(&this->m_timeStamp, &timeinfo); - strftime(tstr, 299, "[%d/%b/%Y:%H:%M:%S %z]", &timeinfo); + char tstr[std::size("[dd/Mmm/yyyy:hh:mm:ss shhmm]")]; + strftime(tstr, std::size(tstr), "[%d/%b/%Y:%H:%M:%S %z]", &timeinfo); ss << utils::string::dash_if_empty( m_variableRequestHeaders.resolveFirst("Host").get()) @@ -1572,14 +1571,14 @@ std::string Transaction::toOldAuditLogFormatIndex(const std::string &filename, std::string Transaction::toOldAuditLogFormat(int parts, const std::string &trailer) { std::stringstream audit_log; - struct tm timeinfo; - char tstr[300]; - memset(tstr, '\0', 300); + struct tm timeinfo; localtime_r(&this->m_timeStamp, &timeinfo); + char tstr[std::size("[dd/Mmm/yyyy:hh:mm:ss shhmm]")]; + strftime(tstr, std::size(tstr), "[%d/%b/%Y:%H:%M:%S %z]", &timeinfo); + audit_log << "--" << trailer << "-" << "A--" << std::endl; - strftime(tstr, 299, "[%d/%b/%Y:%H:%M:%S %z]", &timeinfo); audit_log << tstr; audit_log << " " << m_id->c_str(); audit_log << " " << this->m_clientIpAddress->c_str(); diff --git a/src/variables/time.cc b/src/variables/time.cc index 7d481ee6b0..c674f4e04f 100644 --- a/src/variables/time.cc +++ b/src/variables/time.cc @@ -40,15 +40,13 @@ namespace variables { void Time::evaluate(Transaction *transaction, RuleWithActions *rule, std::vector *l) { - - char tstr[200]; - struct tm timeinfo; time_t timer; - time(&timer); - memset(tstr, '\0', 200); + struct tm timeinfo; localtime_r(&timer, &timeinfo); + + char tstr[std::size("hh:mm:ss")]; strftime(tstr, 200, "%H:%M:%S", &timeinfo); transaction->m_variableTime.assign(tstr); diff --git a/src/variables/time_day.cc b/src/variables/time_day.cc index f094d4c922..eefe937fb5 100644 --- a/src/variables/time_day.cc +++ b/src/variables/time_day.cc @@ -40,15 +40,14 @@ namespace variables { void TimeDay::evaluate(Transaction *transaction, RuleWithActions *rule, std::vector *l) { - char tstr[200]; - struct tm timeinfo; time_t timer; - time(&timer); - memset(tstr, '\0', 200); + struct tm timeinfo; localtime_r(&timer, &timeinfo); - strftime(tstr, 200, "%d", &timeinfo); + + char tstr[std::size("dd")]; + strftime(tstr, std::size(tstr), "%d", &timeinfo); transaction->m_variableTimeDay.assign(tstr); diff --git a/src/variables/time_hour.cc b/src/variables/time_hour.cc index ab809ead00..1a9c1bf75f 100644 --- a/src/variables/time_hour.cc +++ b/src/variables/time_hour.cc @@ -40,15 +40,14 @@ namespace variables { void TimeHour::evaluate(Transaction *transaction, RuleWithActions *rule, std::vector *l) { - char tstr[200]; - struct tm timeinfo; time_t timer; - time(&timer); - memset(tstr, '\0', 200); + struct tm timeinfo; localtime_r(&timer, &timeinfo); - strftime(tstr, 200, "%H", &timeinfo); + + char tstr[std::size("hh")]; + strftime(tstr, std::size(tstr), "%H", &timeinfo); transaction->m_variableTimeHour.assign(tstr); diff --git a/src/variables/time_min.cc b/src/variables/time_min.cc index 9d03be4de1..134582de4e 100644 --- a/src/variables/time_min.cc +++ b/src/variables/time_min.cc @@ -40,15 +40,14 @@ namespace variables { void TimeMin::evaluate(Transaction *transaction, RuleWithActions *rule, std::vector *l) { - char tstr[200]; - struct tm timeinfo; time_t timer; - time(&timer); - memset(tstr, '\0', 200); + struct tm timeinfo; localtime_r(&timer, &timeinfo); - strftime(tstr, 200, "%M", &timeinfo); + + char tstr[std::size("mm")]; + strftime(tstr, std::size(tstr), "%M", &timeinfo); transaction->m_variableTimeMin.assign(tstr); diff --git a/src/variables/time_mon.cc b/src/variables/time_mon.cc index 17e74f3114..1805911216 100644 --- a/src/variables/time_mon.cc +++ b/src/variables/time_mon.cc @@ -40,19 +40,13 @@ namespace variables { void TimeMon::evaluate(Transaction *transaction, RuleWithActions *rule, std::vector *l) { - char tstr[200]; - struct tm timeinfo; time_t timer; - time(&timer); - memset(tstr, '\0', 200); + struct tm timeinfo; localtime_r(&timer, &timeinfo); - strftime(tstr, 200, "%m", &timeinfo); - int a = atoi(tstr); - a--; - transaction->m_variableTimeMin.assign(std::to_string(a)); + transaction->m_variableTimeMin.assign(std::to_string(timeinfo.tm_mon)); l->push_back(new VariableValue(&m_retName, &transaction->m_variableTimeMin)); diff --git a/src/variables/time_sec.cc b/src/variables/time_sec.cc index 5e39af7fe1..a960477100 100644 --- a/src/variables/time_sec.cc +++ b/src/variables/time_sec.cc @@ -40,15 +40,14 @@ namespace variables { void TimeSec::evaluate(Transaction *transaction, RuleWithActions *rule, std::vector *l) { - char tstr[200]; - struct tm timeinfo; time_t timer; - time(&timer); - memset(tstr, '\0', 200); + struct tm timeinfo; localtime_r(&timer, &timeinfo); - strftime(tstr, 200, "%S", &timeinfo); + + char tstr[std::size("ss")]; + strftime(tstr, std::size(tstr), "%S", &timeinfo); transaction->m_variableTimeSec.assign(tstr); diff --git a/src/variables/time_wday.cc b/src/variables/time_wday.cc index fd6a027846..e1840f5823 100644 --- a/src/variables/time_wday.cc +++ b/src/variables/time_wday.cc @@ -40,15 +40,14 @@ namespace variables { void TimeWDay::evaluate(Transaction *transaction, RuleWithActions *rule, std::vector *l) { - char tstr[200]; - struct tm timeinfo; time_t timer; - time(&timer); - memset(tstr, '\0', 200); + struct tm timeinfo; localtime_r(&timer, &timeinfo); - strftime(tstr, 200, "%u", &timeinfo); + + char tstr[std::size("d")]; + strftime(tstr, std::size(tstr), "%u", &timeinfo); transaction->m_variableTimeWDay.assign(tstr); diff --git a/src/variables/time_year.cc b/src/variables/time_year.cc index f68e8cd622..a8f1a50287 100644 --- a/src/variables/time_year.cc +++ b/src/variables/time_year.cc @@ -40,15 +40,14 @@ namespace variables { void TimeYear::evaluate(Transaction *transaction, RuleWithActions *rule, std::vector *l) { - char tstr[200]; - struct tm timeinfo; time_t timer; - time(&timer); - memset(tstr, '\0', 200); + struct tm timeinfo; localtime_r(&timer, &timeinfo); - strftime(tstr, 200, "%Y", &timeinfo); + + char tstr[std::size("yyyy")]; + strftime(tstr, std::size(tstr), "%Y", &timeinfo); transaction->m_variableTimeYear.assign(tstr);