From 4e8c7d1f7cac1936c08ae6932726895c5079cfd1 Mon Sep 17 00:00:00 2001 From: Peter Keresztes Schmidt Date: Sun, 7 Mar 2021 16:35:05 +0100 Subject: [PATCH] Eliminate non-thread-safe calls to localtime localtime uses an internal static storage to which a pointer is given as return value. Due to this it is not safe to call localtime from multiple threads since the same static storage is used. Use localtime_r instead which allows to pass in a tm struct. Fixes: https://github.com/ZoneMinder/zoneminder/security/code-scanning/24 https://github.com/ZoneMinder/zoneminder/security/code-scanning/25 https://github.com/ZoneMinder/zoneminder/security/code-scanning/26 https://github.com/ZoneMinder/zoneminder/security/code-scanning/27 https://github.com/ZoneMinder/zoneminder/security/code-scanning/28 https://github.com/ZoneMinder/zoneminder/security/code-scanning/30 https://github.com/ZoneMinder/zoneminder/security/code-scanning/31 https://github.com/ZoneMinder/zoneminder/security/code-scanning/33 https://github.com/ZoneMinder/zoneminder/security/code-scanning/58 https://github.com/ZoneMinder/zoneminder/security/code-scanning/59 https://github.com/ZoneMinder/zoneminder/security/code-scanning/63 https://github.com/ZoneMinder/zoneminder/security/code-scanning/64 https://github.com/ZoneMinder/zoneminder/security/code-scanning/65 --- src/zm_event.cpp | 33 +++++++++++++++++---------------- src/zm_event.h | 12 +++++++----- src/zm_eventstream.cpp | 18 ++++++++++-------- src/zm_image.cpp | 6 ++++-- src/zm_logger.cpp | 3 ++- src/zm_monitor.cpp | 3 ++- src/zm_user.cpp | 11 ++++++----- src/zmu.cpp | 6 ++++-- 8 files changed, 52 insertions(+), 40 deletions(-) diff --git a/src/zm_event.cpp b/src/zm_event.cpp index f0b2da694..9ab13fafc 100644 --- a/src/zm_event.cpp +++ b/src/zm_event.cpp @@ -71,8 +71,8 @@ Event::Event( std::string notes; createNotes(notes); - struct timeval now; - gettimeofday(&now, 0); + timeval now = {}; + gettimeofday(&now, nullptr); if ( !start_time.tv_sec ) { Warning("Event has zero time, setting to now"); @@ -80,12 +80,12 @@ Event::Event( } else if ( start_time.tv_sec > now.tv_sec ) { char buffer[26]; char buffer_now[26]; - struct tm* tm_info; + tm tm_info = {}; - tm_info = localtime(&start_time.tv_sec); - strftime(buffer, 26, "%Y:%m:%d %H:%M:%S", tm_info); - tm_info = localtime(&now.tv_sec); - strftime(buffer_now, 26, "%Y:%m:%d %H:%M:%S", tm_info); + localtime_r(&start_time.tv_sec, &tm_info); + strftime(buffer, 26, "%Y:%m:%d %H:%M:%S", &tm_info); + localtime_r(&now.tv_sec, &tm_info); + strftime(buffer_now, 26, "%Y:%m:%d %H:%M:%S", &tm_info); Error( "StartDateTime in the future starttime %u.%u >? now %u.%u difference %d\n%s\n%s", @@ -661,16 +661,17 @@ bool Event::SetPath(Storage *storage) { return false; } - struct tm *stime = localtime(&start_time.tv_sec); + tm stime = {}; + localtime_r(&start_time.tv_sec, &stime); if ( scheme == Storage::DEEP ) { int dt_parts[6]; - dt_parts[0] = stime->tm_year-100; - dt_parts[1] = stime->tm_mon+1; - dt_parts[2] = stime->tm_mday; - dt_parts[3] = stime->tm_hour; - dt_parts[4] = stime->tm_min; - dt_parts[5] = stime->tm_sec; + dt_parts[0] = stime.tm_year-100; + dt_parts[1] = stime.tm_mon+1; + dt_parts[2] = stime.tm_mday; + dt_parts[3] = stime.tm_hour; + dt_parts[4] = stime.tm_min; + dt_parts[5] = stime.tm_sec; std::string date_path; std::string time_path; @@ -685,7 +686,7 @@ bool Event::SetPath(Storage *storage) { if ( i == 2 ) date_path = path; } - time_path = stringtf("%02d/%02d/%02d", stime->tm_hour, stime->tm_min, stime->tm_sec); + time_path = stringtf("%02d/%02d/%02d", stime.tm_hour, stime.tm_min, stime.tm_sec); // Create event id symlink std::string id_file = stringtf("%s/.%" PRIu64, date_path.c_str(), id); @@ -695,7 +696,7 @@ bool Event::SetPath(Storage *storage) { } } else if ( scheme == Storage::MEDIUM ) { path += stringtf("/%04d-%02d-%02d", - stime->tm_year+1900, stime->tm_mon+1, stime->tm_mday + stime.tm_year+1900, stime.tm_mon+1, stime.tm_mday ); if ( mkdir(path.c_str(), 0755) and ( errno != EEXIST ) ) { Error("Can't mkdir %s: %s", path.c_str(), strerror(errno)); diff --git a/src/zm_event.h b/src/zm_event.h index 1eb3f2a7f..5e972eca2 100644 --- a/src/zm_event.h +++ b/src/zm_event.h @@ -138,18 +138,20 @@ class Event { bool SetPath(Storage *storage); public: - static const char *getSubPath(struct tm *time) { + static const char *getSubPath(tm time) { static char subpath[PATH_MAX] = ""; snprintf(subpath, sizeof(subpath), "%02d/%02d/%02d/%02d/%02d/%02d", - time->tm_year-100, time->tm_mon+1, time->tm_mday, - time->tm_hour, time->tm_min, time->tm_sec); + time.tm_year-100, time.tm_mon+1, time.tm_mday, + time.tm_hour, time.tm_min, time.tm_sec); return subpath; } static const char *getSubPath(time_t *time) { - return Event::getSubPath(localtime(time)); + tm time_tm = {}; + localtime_r(time, &time_tm); + return Event::getSubPath(time_tm); } - const char* getEventFile(void) const { + const char* getEventFile() const { return video_file.c_str(); } diff --git a/src/zm_eventstream.cpp b/src/zm_eventstream.cpp index 5ed424b7f..8610f2f04 100644 --- a/src/zm_eventstream.cpp +++ b/src/zm_eventstream.cpp @@ -171,33 +171,35 @@ bool EventStream::loadEventData(uint64_t event_id) { const char *storage_path = storage->Path(); if ( event_data->scheme == Storage::DEEP ) { - struct tm *event_time = localtime(&event_data->start_time); + tm event_time = {}; + localtime_r(&event_data->start_time, &event_time); if ( storage_path[0] == '/' ) snprintf(event_data->path, sizeof(event_data->path), "%s/%u/%02d/%02d/%02d/%02d/%02d/%02d", storage_path, event_data->monitor_id, - event_time->tm_year-100, event_time->tm_mon+1, event_time->tm_mday, - event_time->tm_hour, event_time->tm_min, event_time->tm_sec); + event_time.tm_year-100, event_time.tm_mon+1, event_time.tm_mday, + event_time.tm_hour, event_time.tm_min, event_time.tm_sec); else snprintf(event_data->path, sizeof(event_data->path), "%s/%s/%u/%02d/%02d/%02d/%02d/%02d/%02d", staticConfig.PATH_WEB.c_str(), storage_path, event_data->monitor_id, - event_time->tm_year-100, event_time->tm_mon+1, event_time->tm_mday, - event_time->tm_hour, event_time->tm_min, event_time->tm_sec); + event_time.tm_year-100, event_time.tm_mon+1, event_time.tm_mday, + event_time.tm_hour, event_time.tm_min, event_time.tm_sec); } else if ( event_data->scheme == Storage::MEDIUM ) { - struct tm *event_time = localtime(&event_data->start_time); + tm event_time = {}; + localtime_r(&event_data->start_time, &event_time); if ( storage_path[0] == '/' ) snprintf(event_data->path, sizeof(event_data->path), "%s/%u/%04d-%02d-%02d/%" PRIu64, storage_path, event_data->monitor_id, - event_time->tm_year+1900, event_time->tm_mon+1, event_time->tm_mday, + event_time.tm_year+1900, event_time.tm_mon+1, event_time.tm_mday, event_data->event_id); else snprintf(event_data->path, sizeof(event_data->path), "%s/%s/%u/%04d-%02d-%02d/%" PRIu64, staticConfig.PATH_WEB.c_str(), storage_path, event_data->monitor_id, - event_time->tm_year+1900, event_time->tm_mon+1, event_time->tm_mday, + event_time.tm_year+1900, event_time.tm_mon+1, event_time.tm_mday, event_data->event_id); } else { diff --git a/src/zm_image.cpp b/src/zm_image.cpp index 014ae783c..2e0c82670 100644 --- a/src/zm_image.cpp +++ b/src/zm_image.cpp @@ -1186,7 +1186,8 @@ cinfo->out_color_space = JCS_RGB; // This is a lot of stuff to allocate on the stack. Recommend char *timebuf[64]; char timebuf[64], msbuf[64]; - strftime(timebuf, sizeof timebuf, "%Y:%m:%d %H:%M:%S", localtime(&(timestamp.tv_sec))); + tm timestamp_tm = {}; + strftime(timebuf, sizeof timebuf, "%Y:%m:%d %H:%M:%S", localtime_r(×tamp.tv_sec, ×tamp_tm)); snprintf(msbuf, sizeof msbuf, "%06d",(int)(timestamp.tv_usec)); // we only use milliseconds because that's all defined in exif, but this is the whole microseconds because we have it unsigned char exiftimes[82] = { 0x45, 0x78, 0x69, 0x66, 0x00, 0x00, 0x49, 0x49, 0x2A, 0x00, 0x08, 0x00, 0x00, 0x00, 0x01, 0x00, @@ -2132,7 +2133,8 @@ void Image::Annotate( void Image::Timestamp( const char *label, const time_t when, const Coord &coord, const int size ) { char time_text[64]; - strftime(time_text, sizeof(time_text), "%y/%m/%d %H:%M:%S", localtime(&when)); + tm when_tm = {}; + strftime(time_text, sizeof(time_text), "%y/%m/%d %H:%M:%S", localtime_r(&when, &when_tm)); if ( label ) { // Assume label is max 64, + ' - ' + 64 chars of time_text char text[132]; diff --git a/src/zm_logger.cpp b/src/zm_logger.cpp index 3d0c0c2c5..d425764ae 100644 --- a/src/zm_logger.cpp +++ b/src/zm_logger.cpp @@ -447,7 +447,8 @@ void Logger::logPrint(bool hex, const char * const filepath, const int line, con } else { #endif char *timePtr = timeString; - timePtr += strftime(timePtr, sizeof(timeString), "%x %H:%M:%S", localtime(&timeVal.tv_sec)); + tm now_tm = {}; + timePtr += strftime(timePtr, sizeof(timeString), "%x %H:%M:%S", localtime_r(&timeVal.tv_sec, &now_tm)); snprintf(timePtr, sizeof(timeString)-(timePtr-timeString), ".%06ld", timeVal.tv_usec); #if 0 } diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 1ca51bdb9..f846e569f 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -2842,7 +2842,8 @@ void Monitor::TimestampImage(Image *ts_image, const struct timeval *ts_time) con // Expand the strftime macros first char label_time_text[256]; - strftime(label_time_text, sizeof(label_time_text), label_format, localtime(&ts_time->tv_sec)); + tm ts_tm = {}; + strftime(label_time_text, sizeof(label_time_text), label_format, localtime_r(&ts_time->tv_sec, &ts_tm)); char label_text[1024]; const char *s_ptr = label_time_text; char *d_ptr = label_text; diff --git a/src/zm_user.cpp b/src/zm_user.cpp index 512718ccb..13cf7c1ac 100644 --- a/src/zm_user.cpp +++ b/src/zm_user.cpp @@ -245,18 +245,19 @@ User *zmLoadAuthUser(const char *auth, bool use_remote_addr) { const char *pass = dbrow[2]; time_t our_now = now; + tm now_tm = {}; for ( unsigned int i = 0; i < hours; i++, our_now -= 3600 ) { - struct tm *now_tm = localtime(&our_now); + localtime_r(&our_now, &now_tm); snprintf(auth_key, sizeof(auth_key)-1, "%s%s%s%s%d%d%d%d", config.auth_hash_secret, user, pass, remote_addr, - now_tm->tm_hour, - now_tm->tm_mday, - now_tm->tm_mon, - now_tm->tm_year); + now_tm.tm_hour, + now_tm.tm_mday, + now_tm.tm_mon, + now_tm.tm_year); #if HAVE_DECL_MD5 MD5((unsigned char *)auth_key, strlen(auth_key), md5sum); diff --git a/src/zmu.cpp b/src/zmu.cpp index 0bfe90b3b..384af99a3 100644 --- a/src/zmu.cpp +++ b/src/zmu.cpp @@ -506,8 +506,10 @@ int main(int argc, char *argv[]) { struct timeval timestamp = monitor->GetTimestamp(image_idx); if ( verbose ) { char timestamp_str[64] = "None"; - if ( timestamp.tv_sec ) - strftime(timestamp_str, sizeof(timestamp_str), "%Y-%m-%d %H:%M:%S", localtime(×tamp.tv_sec)); + if ( timestamp.tv_sec ) { + tm tm_info = {}; + strftime(timestamp_str, sizeof(timestamp_str), "%Y-%m-%d %H:%M:%S", localtime_r(×tamp.tv_sec, &tm_info)); + } if ( image_idx == -1 ) printf("Time of last image capture: %s.%02ld\n", timestamp_str, timestamp.tv_usec/10000); else