From bd47e0fffa80cd72b603d4e254e637ae7ce780b4 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Sat, 21 May 2022 16:41:58 +0100 Subject: [PATCH] More bounds checking in Adjust::adjustDateTime --- app/actions.cpp | 59 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/app/actions.cpp b/app/actions.cpp index 752d026cad..64170f1b8e 100644 --- a/app/actions.cpp +++ b/app/actions.cpp @@ -12,6 +12,8 @@ #include "image.hpp" #include "iptc.hpp" #include "preview.hpp" +#include "enforce.hpp" +#include "safe_op.hpp" #include "types.hpp" #include "xmp_exiv2.hpp" @@ -1407,19 +1409,48 @@ int Adjust::adjustDateTime(Exiv2::ExifData& exifData, const std::string& key, co std::cerr << path << ": " << _("Failed to parse timestamp") << " `" << timeStr << "'\n"; return 1; } - const long monOverflow = (tm.tm_mon + monthAdjustment_) / 12; - tm.tm_mon = (tm.tm_mon + monthAdjustment_) % 12; - tm.tm_year += yearAdjustment_ + monOverflow; + + // bounds checking for yearAdjustment_ + enforce(yearAdjustment_ >= std::numeric_limits::min(), + "year adjustment too low"); + enforce(yearAdjustment_ <= std::numeric_limits::max(), + "year adjustment too high"); + const auto yearAdjustment = static_cast(yearAdjustment_); + + // bounds checking for monthAdjustment_ + enforce(monthAdjustment_ >= std::numeric_limits::min(), + "month adjustment too low"); + enforce(monthAdjustment_ <= std::numeric_limits::max(), + "month adjustment too high"); + const auto monthAdjustment = static_cast(monthAdjustment_); + + // bounds checking for dayAdjustment_ + enforce(dayAdjustment_ >= std::numeric_limits::min() / 86400, + "day adjustment too low"); + enforce(dayAdjustment_ <= std::numeric_limits::max() / 86400, + "day adjustment too high"); + const auto dayAdjustment = static_cast(dayAdjustment_); + + // bounds checking for adjustment_ + enforce(adjustment_ >= std::numeric_limits::min(), + "day adjustment too low"); + enforce(adjustment_ <= std::numeric_limits::max(), + "day adjustment too high"); + const auto adjustment = static_cast(adjustment_); + + const auto monOverflow = Safe::add(tm.tm_mon, monthAdjustment) / 12; + tm.tm_mon = Safe::add(tm.tm_mon, monthAdjustment) % 12; + tm.tm_year = Safe::add(tm.tm_year, Safe::add(yearAdjustment, monOverflow)); // Let's not create files with non-4-digit years, we can't read them. if (tm.tm_year > 9999 - 1900 || tm.tm_year < 1000 - 1900) { if (Params::instance().verbose_) std::cout << std::endl; - std::cerr << path << ": " << _("Can't adjust timestamp by") << " " << yearAdjustment_ + monOverflow << " " + std::cerr << path << ": " << _("Can't adjust timestamp by") << " " << yearAdjustment + monOverflow << " " << _("years") << "\n"; return 1; } time_t time = mktime(&tm); - time += adjustment_ + dayAdjustment_ * 86400; + time = Safe::add(time, Safe::add(adjustment, dayAdjustment * 86400)); timeStr = time2Str(time); if (Params::instance().verbose_) { std::cout << " " << _("to") << " " << timeStr << std::endl; @@ -1590,22 +1621,28 @@ int str2Tm(const std::string& timeStr, struct tm* tm) { long tmp = 0; if (!Util::strtol(timeStr.substr(0, 4).c_str(), tmp)) return 5; - tm->tm_year = tmp - 1900; + // tmp is a 4-digit number so this cast cannot overflow + tm->tm_year = static_casttm_year)>(tmp - 1900); if (!Util::strtol(timeStr.substr(5, 2).c_str(), tmp)) return 6; - tm->tm_mon = tmp - 1; + // tmp is a 2-digit number so this cast cannot overflow + tm->tm_mon = static_casttm_mon)>(tmp - 1); if (!Util::strtol(timeStr.substr(8, 2).c_str(), tmp)) return 7; - tm->tm_mday = tmp; + // tmp is a 2-digit number so this cast cannot overflow + tm->tm_mday = static_casttm_mday)>(tmp); if (!Util::strtol(timeStr.substr(11, 2).c_str(), tmp)) return 8; - tm->tm_hour = tmp; + // tmp is a 2-digit number so this cast cannot overflow + tm->tm_hour = static_casttm_hour)>(tmp); if (!Util::strtol(timeStr.substr(14, 2).c_str(), tmp)) return 9; - tm->tm_min = tmp; + // tmp is a 2-digit number so this cast cannot overflow + tm->tm_min = static_casttm_min)>(tmp); if (!Util::strtol(timeStr.substr(17, 2).c_str(), tmp)) return 10; - tm->tm_sec = tmp; + // tmp is a 2-digit number so this cast cannot overflow + tm->tm_sec = static_casttm_sec)>(tmp); // Conversions to set remaining fields of the tm structure if (mktime(tm) == static_cast(-1))