From: Thibault Godouet Date: Sun, 13 Apr 2014 10:06:45 +0000 (+0100) Subject: fixed bug where LONG_MAX could be used for localtime() etc, which is too big X-Git-Tag: ver3_1_3~11 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=35658d7d5d8ee9dd3ff52dfe629379f1b005caeb;p=thirdparty%2Ffcron.git fixed bug where LONG_MAX could be used for localtime() etc, which is too big --- diff --git a/conf.c b/conf.c index 2d86efe..fd6284b 100644 --- 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 0fdd184..fd5cf17 100644 --- 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 f06c282..2f38dd4 100644 --- a/doc/en/changes.sgml +++ b/doc/en/changes.sgml @@ -17,6 +17,9 @@ A copy of the license is included in gfdl.sgml. Fixed compilation on Solaris 10, and fixed fcrondyn authentication without a password on that platform. + + 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. + diff --git a/fcron.c b/fcron.c index d678070..c4e91e5 100644 --- 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 67f3efc..76367d6 100644 --- 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)