From 4797c4e17e582caf1ad74a7e9e3936a30ce5eebc Mon Sep 17 00:00:00 2001
From: "Dustin J. Mitchell" <dustin@v.igoro.us>
Date: Fri, 29 Nov 2024 09:12:20 -0500
Subject: [PATCH] Check Datetime addition when performing recurrence (#3708)

---
 src/main.h         |  4 +++-
 src/recur.cpp      | 42 +++++++++++++++++++++++++++++++++++++-----
 test/util_test.cpp |  7 +++++++
 3 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/src/main.h b/src/main.h
index 9d15a09fa..4b2ff2a57 100644
--- a/src/main.h
+++ b/src/main.h
@@ -35,13 +35,15 @@
 #include <algorithm>
 #include <list>
 #include <map>
+#include <optional>
 #include <string>
 #include <vector>
 
 // recur.cpp
 void handleRecurrence();
 void handleUntil();
-Datetime getNextRecurrence(Datetime&, std::string&);
+std::optional<Datetime> checked_add_datetime(Datetime& base, time_t delta);
+std::optional<Datetime> getNextRecurrence(Datetime&, std::string&);
 bool generateDueDates(Task&, std::vector<Datetime>&);
 void updateRecurrenceMask(Task&);
 
diff --git a/src/recur.cpp b/src/recur.cpp
index f5c9f3979..d6bb47454 100644
--- a/src/recur.cpp
+++ b/src/recur.cpp
@@ -47,8 +47,24 @@
 #include <fstream>
 #include <iomanip>
 #include <iostream>
+#include <limits>
+#include <optional>
 #include <sstream>
 
+// Add a `time_t` delta to a Datetime, checking for and returning nullopt on integer overflow.
+std::optional<Datetime> checked_add_datetime(Datetime& base, time_t delta) {
+  // Datetime::operator+ takes an integer delta, so check that range
+  if (static_cast<time_t>(std::numeric_limits<int>::max()) < delta) {
+    return std::nullopt;
+  }
+
+  // Check for time_t overflow in the Datetime.
+  if (std::numeric_limits<time_t>::max() - base.toEpoch() < delta) {
+    return std::nullopt;
+  }
+  return base + delta;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Scans all tasks, and for any recurring tasks, determines whether any new
 // child tasks need to be generated to fill gaps.
@@ -95,7 +111,12 @@ void handleRecurrence() {
             Datetime old_wait(t.get_date("wait"));
             Datetime old_due(t.get_date("due"));
             Datetime due(d);
-            rec.set("wait", format((due + (old_wait - old_due)).toEpoch()));
+            auto wait = checked_add_datetime(due, old_wait - old_due);
+            if (wait) {
+              rec.set("wait", format(wait->toEpoch()));
+            } else {
+              rec.remove("wait");
+            }
             rec.setStatus(Task::waiting);
             mask += 'W';
           } else {
@@ -148,7 +169,8 @@ bool generateDueDates(Task& parent, std::vector<Datetime>& allDue) {
   auto recurrence_limit = Context::getContext().config.getInteger("recurrence.limit");
   int recurrence_counter = 0;
   Datetime now;
-  for (Datetime i = due;; i = getNextRecurrence(i, recur)) {
+  Datetime i = due;
+  while (1) {
     allDue.push_back(i);
 
     if (specificEnd && i > until) {
@@ -164,13 +186,23 @@ bool generateDueDates(Task& parent, std::vector<Datetime>& allDue) {
     if (i > now) ++recurrence_counter;
 
     if (recurrence_counter >= recurrence_limit) return true;
+    auto next = getNextRecurrence(i, recur);
+    if (next) {
+      i = *next;
+    } else {
+      return true;
+    }
   }
 
   return true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-Datetime getNextRecurrence(Datetime& current, std::string& period) {
+/// Determine the next recurrence of the given period.
+///
+/// If no such date can be calculated, such as with a very large period, returns
+/// nullopt.
+std::optional<Datetime> getNextRecurrence(Datetime& current, std::string& period) {
   auto m = current.month();
   auto d = current.day();
   auto y = current.year();
@@ -201,7 +233,7 @@ Datetime getNextRecurrence(Datetime& current, std::string& period) {
     else
       days = 1;
 
-    return current + (days * 86400);
+    return checked_add_datetime(current, days * 86400);
   }
 
   else if (unicodeLatinDigit(period[0]) && period[period.length() - 1] == 'm') {
@@ -317,7 +349,7 @@ Datetime getNextRecurrence(Datetime& current, std::string& period) {
   if (!p.parse(period, idx))
     throw std::string(format("The recurrence value '{1}' is not valid.", period));
 
-  return current + p.toTime_t();
+  return checked_add_datetime(current, p.toTime_t());
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/test/util_test.cpp b/test/util_test.cpp
index c4cf4e0bd..8e52a723a 100644
--- a/test/util_test.cpp
+++ b/test/util_test.cpp
@@ -33,6 +33,7 @@
 #include <util.h>
 
 #include <iostream>
+#include <limits>
 
 ////////////////////////////////////////////////////////////////////////////////
 int TEST_NAME(int, char**) {
@@ -91,6 +92,12 @@ int TEST_NAME(int, char**) {
   t.ok(nontrivial("  \t\ta"), "nontrivial '  \\t\\ta' -> true");
   t.ok(nontrivial("a\t\t  "), "nontrivial 'a\\t\\t  ' -> true");
 
+  Datetime dt(1234526400);
+  Datetime max(std::numeric_limits<time_t>::max());
+  t.ok(checked_add_datetime(dt, 10).has_value(), "small delta");
+  t.ok(!checked_add_datetime(dt, 0x100000000).has_value(), "delta > 32bit");
+  t.ok(!checked_add_datetime(max, 1).has_value(), "huge base time");
+
   return 0;
 }