fixed bug where LONG_MAX could be used for localtime() etc, which is too big

diff --git a/conf.c b/conf.c
index 2d86efef029f0b826857132916e7f17178b51dc0..fd6284be77aa526cae88811fcabf59c6526f50f6 100644 (file)
--- a/conf.c
+++ b/conf.c
@@ -932,34 +932,34 @@ add_line_to_file(cl_t * cl, cf_t * cf, uid_t runas, char *runas_str,
                                            || is_runonce(cl->cl_option)))) {
             /* cl_first is always saved to disk for a volatile line */
             if (cl->cl_first == LONG_MAX) {
-                cl->cl_nextexe = LONG_MAX;
+                cl->cl_nextexe = TIME_T_MAX;
             }
             else {
                 cl->cl_nextexe = now + cl->cl_first;
-                if (cl->cl_nextexe < now) {
+                if (cl->cl_nextexe < now || cl->cl_nextexe > TIME_T_MAX) {
                     /* there was an integer overflow! */
                     error
                         ("Error while setting next exe time for job %s: cl_nextexe"
-                         " overflowed. now=%lu, cl_timefreq=%lu, cl_nextexe=%lu.",
+                         " overflowed (case1). now=%lu, cl_timefreq=%lu, cl_nextexe=%lu.",
                          cl->cl_shell, now, cl->cl_timefreq, cl->cl_nextexe);
                     error
-                        ("Setting cl_nextexe to LONG_MAX to prevent an infinite loop.");
-                    cl->cl_nextexe = LONG_MAX;
+                        ("Setting cl_nextexe to TIME_T_MAX to prevent an infinite loop.");
+                    cl->cl_nextexe = TIME_T_MAX;
                 }
             }
         }
         else {
             if (cl->cl_nextexe != LONG_MAX) {
                 cl->cl_nextexe += slept;
-                if (cl->cl_nextexe < now) {
+                if (cl->cl_nextexe < now || cl->cl_nextexe > TIME_T_MAX) {
                     /* there was an integer overflow! */
                     error
                         ("Error while setting next exe time for job %s: cl_nextexe"
-                         " overflowed. now=%lu, cl_timefreq=%lu, cl_nextexe=%lu.",
+                         " overflowed (case2). now=%lu, cl_timefreq=%lu, cl_nextexe=%lu.",
                          cl->cl_shell, now, cl->cl_timefreq, cl->cl_nextexe);
                     error
-                        ("Setting cl_nextexe to LONG_MAX to prevent an infinite loop.");
-                    cl->cl_nextexe = LONG_MAX;
+                        ("Setting cl_nextexe to TIME_T_MAX=%ld to prevent an infinite loop.", TIME_T_MAX);
+                    cl->cl_nextexe = TIME_T_MAX;
                 }
             }
         }

diff --git a/database.c b/database.c
index 0fdd184f3c70bc882f0fec5e680587126de22fd3..fd5cf17f884a9dedd5532b2c175f0ed44158485c 100644 (file)
--- a/database.c
+++ b/database.c
@@ -1226,20 +1226,20 @@ set_next_exe(cl_t * line, char option, int info_fd)
             /* NOTE: the options runonce/hasrun should be used to achieve this,
              *       but we keep this here as an extra safety */
             debug
-                ("Setting cl_nextexe to LONG_MAX to prevent the line from running again.");
-            line->cl_nextexe = LONG_MAX;
+                ("Setting cl_nextexe to TIME_T_MAX to prevent the line from running again.");
+            line->cl_nextexe = TIME_T_MAX;
         }
         else {
             line->cl_nextexe = basetime + line->cl_timefreq;
             if (line->cl_nextexe <= basetime) {
                 /* there was an integer overflow! */
                 error("Error while setting next exe time for job %s: cl_nextexe"
-                      " overflowed. basetime=%lu, cl_timefreq=%lu, cl_nextexe=%lu.",
+                      " overflowed (case3). basetime=%lu, cl_timefreq=%lu, cl_nextexe=%lu.",
                       line->cl_shell, basetime, line->cl_timefreq,
                       line->cl_nextexe);
                 error
-                    ("Setting cl_nextexe to LONG_MAX to prevent an infinite loop.");
-                line->cl_nextexe = LONG_MAX;
+                    ("Setting cl_nextexe to TIME_T_MAX to prevent an infinite loop.");
+                line->cl_nextexe = TIME_T_MAX;
             }
         }
 

diff --git a/doc/en/changes.sgml b/doc/en/changes.sgml
index f06c282187f60b921a367c6f4cebef87dec2e6c9..2f38dd4433c1bc8753ab79fc2cf88dd847b8757c 100644 (file)
--- a/doc/en/changes.sgml
+++ b/doc/en/changes.sgml
@@ -17,6 +17,9 @@ A copy of the license is included in gfdl.sgml.
            <listitem>
               <para>Fixed compilation on Solaris 10, and fixed fcrondyn authentication without a password on that platform.</para>
            </listitem>
+           <listitem>
+              <para>Fixed bug where LONG_MAX could potentially be used in functions like localtime(), which is too big for localtime().  This was unlikely to happen unless there was another problem in the first place.</para>
+           </listitem>
       </itemizedlist>
 
       <itemizedlist>

diff --git a/fcron.c b/fcron.c
index d678070246887ea653e1a81e5f51c8e96fcf5deb..c4e91e5905aad70cd4d2f417f96bd0c23e713e8f 100644 (file)
--- a/fcron.c
+++ b/fcron.c
@@ -374,14 +374,14 @@ parseopt(int argc, char *argv[])
 
         case 's':
             if ((save_time = strtol(optarg, NULL, 10)) < 60
-                || save_time >= LONG_MAX)
-                die("Save time can only be set between 60 and %d.", LONG_MAX);
+                || save_time >= TIME_T_MAX)
+                die("Save time can only be set between 60 and %d.", TIME_T_MAX);
             break;
 
         case 'l':
             if ((first_sleep = strtol(optarg, NULL, 10)) < 0
-                || first_sleep >= LONG_MAX)
-                die("First sleep can only be set between 0 and %d.", LONG_MAX);
+                || first_sleep >= TIME_T_MAX)
+                die("First sleep can only be set between 0 and %d.", TIME_T_MAX);
             break;
 
         case 'm':

diff --git a/global.h b/global.h
index 67f3efcb4813e7543a054fc9a3b17567c426dafc..76367d69d774953866b2c8b832f19f2077c35de1 100644 (file)
--- a/global.h
+++ b/global.h
@@ -147,6 +147,22 @@
 /* options for local functions */
 #define STD 0
 
+/* Approximate max value of time_t which localtime() allows: on a 64bits system,
+ * this is less than LONG_MAX (64bits) as this is limited by struct tm's tm_year
+ * which is a (not long) int (32bits).
+ * As a time_t of INT_MAX=2^31 is 'only' in year 2038, we try to use a larger value
+ * if we can. */
+// FIXME: test on 32bit system
+/* 2^33 = 8589934592, so LONG is 64bits at least */
+#if (LONG_MAX > INT_MAX) && (LONG_MAX > 8589934592)
+/* defined as time_t of 1st Jan of year (SHRT_MAX-1900) at 00:00:00 */
+#  define TIME_T_MAX 971859427200
+#else
+/* struct tm's tm_year is of type int, and tm_year will always be smaller than
+ * the equivalent time_t, so INT_MAX is always a safe max value for time_t. */
+#  define TIME_T_MAX INT_MAX
+#endif
+
 /* macros */
 #ifndef HAVE_SETEUID
 #define seteuid(arg) setresuid(-1,(arg),-1)