krb5 commit: Check return values of time functions

Greg Hudson ghudson at mit.edu
Fri Aug 31 12:53:15 EDT 2018


https://github.com/krb5/krb5/commit/39f4af68d436f2e7585eca86823029b337c349a9
commit 39f4af68d436f2e7585eca86823029b337c349a9
Author: Greg Hudson <ghudson at mit.edu>
Date:   Wed Aug 29 15:04:13 2018 -0400

    Check return values of time functions
    
    Where ctime(), localtime(), or localtime_r() is used, check for
    failure even if it is unlikely (reported by Bean Zhang).  Constify the
    strdate() return type in kdb5_mkey.c and kadmin.c and the
    ctime_uint32() return type in kproplog.c.  Use localtime_r()
    unconditionally in str_conv.c as there is already a wrapper in that
    file for the case where the platform doesn't have it.  Remove an
    inoperative localtime() call in ktutil.c.

 src/kadmin/cli/kadmin.c       |    6 ++++--
 src/kadmin/dbutil/kdb5_mkey.c |    7 +++++--
 src/kadmin/ktutil/ktutil.c    |    1 -
 src/kadmin/server/misc.c      |    2 ++
 src/lib/kadm5/chpass_util.c   |    4 +++-
 src/lib/kadm5/logger.c        |    9 +++++++--
 src/lib/krb5/krb/str_conv.c   |   13 ++++---------
 src/slave/kproplog.c          |    6 ++++--
 8 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/src/kadmin/cli/kadmin.c b/src/kadmin/cli/kadmin.c
index aee5c83..a0ba1a5 100644
--- a/src/kadmin/cli/kadmin.c
+++ b/src/kadmin/cli/kadmin.c
@@ -138,7 +138,7 @@ strdur(time_t duration)
     return out;
 }
 
-static char *
+static const char *
 strdate(krb5_timestamp when)
 {
     struct tm *tm;
@@ -146,7 +146,9 @@ strdate(krb5_timestamp when)
     time_t lcltim = ts2tt(when);
 
     tm = localtime(&lcltim);
-    strftime(out, sizeof(out), "%a %b %d %H:%M:%S %Z %Y", tm);
+    if (tm == NULL ||
+        strftime(out, sizeof(out), "%a %b %d %H:%M:%S %Z %Y", tm) == 0)
+        strlcpy(out, "(error)", sizeof(out));
     return out;
 }
 
diff --git a/src/kadmin/dbutil/kdb5_mkey.c b/src/kadmin/dbutil/kdb5_mkey.c
index 5395a60..19796c2 100644
--- a/src/kadmin/dbutil/kdb5_mkey.c
+++ b/src/kadmin/dbutil/kdb5_mkey.c
@@ -40,14 +40,17 @@ extern kadm5_config_params global_params;
 extern krb5_context util_context;
 extern time_t get_date(char *);
 
-static char *strdate(krb5_timestamp when)
+static const char *
+strdate(krb5_timestamp when)
 {
     struct tm *tm;
     static char out[40];
     time_t lcltim = ts2tt(when);
 
     tm = localtime(&lcltim);
-    strftime(out, sizeof(out), "%a %b %d %H:%M:%S %Z %Y", tm);
+    if (tm == NULL ||
+        strftime(out, sizeof(out), "%a %b %d %H:%M:%S %Z %Y", tm) == 0)
+        strlcpy(out, "(error)", sizeof(out));
     return out;
 }
 
diff --git a/src/kadmin/ktutil/ktutil.c b/src/kadmin/ktutil/ktutil.c
index 6a8586d..198cb13 100644
--- a/src/kadmin/ktutil/ktutil.c
+++ b/src/kadmin/ktutil/ktutil.c
@@ -249,7 +249,6 @@ void ktutil_list(argc, argv)
             time_t tstamp;
 
             tstamp = lp->entry->timestamp;
-            (void) localtime(&tstamp);
             lp->entry->timestamp = tstamp;
             fill = ' ';
             if (!krb5_timestamp_to_sfstring((krb5_timestamp)lp->entry->
diff --git a/src/kadmin/server/misc.c b/src/kadmin/server/misc.c
index 6b258a6..45e1f81 100644
--- a/src/kadmin/server/misc.c
+++ b/src/kadmin/server/misc.c
@@ -95,6 +95,8 @@ check_min_life(void *server_handle, krb5_principal principal,
                 until = princ.last_pwd_change + pol.pw_min_life;
 
                 time_string = ctime(&until);
+                if (time_string == NULL)
+                    time_string = "(error)";
                 errstr = error_message(CHPASS_UTIL_PASSWORD_TOO_SOON);
 
                 if (strlen(errstr) + strlen(time_string) < msg_len) {
diff --git a/src/lib/kadm5/chpass_util.c b/src/lib/kadm5/chpass_util.c
index 1680a55..9a1d62d 100644
--- a/src/lib/kadm5/chpass_util.c
+++ b/src/lib/kadm5/chpass_util.c
@@ -217,7 +217,9 @@ kadm5_ret_t _kadm5_chpass_principal_util(void *server_handle,
         until = ts_incr(princ_ent.last_pwd_change, policy_ent.pw_min_life);
 
         time_string = ctime(&until);
-        if (*(ptr = &time_string[strlen(time_string)-1]) == '\n')
+        if (time_string == NULL)
+            time_string = "(error)";
+        else if (*(ptr = &time_string[strlen(time_string)-1]) == '\n')
             *ptr = '\0';
 
         snprintf(msg_ret, msg_len, string_text(CHPASS_UTIL_PASSWORD_TOO_SOON),
diff --git a/src/lib/kadm5/logger.c b/src/lib/kadm5/logger.c
index 2da8f92..eff8a8a 100644
--- a/src/lib/kadm5/logger.c
+++ b/src/lib/kadm5/logger.c
@@ -638,7 +638,9 @@ klog_vsyslog(int priority, const char *format, va_list arglist)
     time_t      now;
 #ifdef  HAVE_STRFTIME
     size_t      soff;
-#endif  /* HAVE_STRFTIME */
+#else
+    char       *r;
+#endif
 
     /*
      * Format a syslog-esque message of the format:
@@ -667,7 +669,10 @@ klog_vsyslog(int priority, const char *format, va_list arglist)
      *  dow mon dd hh:mm:ss tzs yyyy\n
      *  012345678901234567890123456789
      */
-    strncpy(outbuf, ctime(&now) + 4, 15);
+    r = ctime(&now);
+    if (r == NULL)
+        return(-1);
+    strncpy(outbuf, r + 4, 15);
     cp += 15;
 #endif  /* HAVE_STRFTIME */
 #ifdef VERBOSE_LOGS
diff --git a/src/lib/krb5/krb/str_conv.c b/src/lib/krb5/krb/str_conv.c
index f0a2ae2..efb3096 100644
--- a/src/lib/krb5/krb/str_conv.c
+++ b/src/lib/krb5/krb/str_conv.c
@@ -212,11 +212,8 @@ krb5_timestamp_to_string(krb5_timestamp timestamp, char *buffer, size_t buflen)
     const char *fmt = "%c"; /* This is to get around gcc -Wall warning that
                                the year returned might be two digits */
 
-#ifdef HAVE_LOCALTIME_R
-    (void) localtime_r(&timestamp2, &tmbuf);
-#else
-    memcpy(&tmbuf, localtime(&timestamp2), sizeof(tmbuf));
-#endif
+    if (localtime_r(&timestamp2, &tmbuf) == NULL)
+        return(ENOMEM);
     ret = strftime(buffer, buflen, fmt, &tmbuf);
     if (ret == 0 || ret == buflen)
         return(ENOMEM);
@@ -246,11 +243,9 @@ krb5_timestamp_to_sfstring(krb5_timestamp timestamp, char *buffer, size_t buflen
     static const unsigned int sftime_format_table_nents =
         sizeof(sftime_format_table)/sizeof(sftime_format_table[0]);
 
-#ifdef HAVE_LOCALTIME_R
     tmp = localtime_r(&timestamp2, &tmbuf);
-#else
-    memcpy((tmp = &tmbuf), localtime(&timestamp2), sizeof(tmbuf));
-#endif
+    if (tmp == NULL)
+        return errno;
     ndone = 0;
     for (i=0; i<sftime_format_table_nents; i++) {
         if ((ndone = strftime(buffer, buflen, sftime_format_table[i], tmp)))
diff --git a/src/slave/kproplog.c b/src/slave/kproplog.c
index 7e4434e..232a16f 100644
--- a/src/slave/kproplog.c
+++ b/src/slave/kproplog.c
@@ -71,13 +71,15 @@ print_flags(unsigned int flags)
 }
 
 /* ctime() for uint32_t* */
-static char *
+static const char *
 ctime_uint32(uint32_t *time32)
 {
     time_t tmp;
+    const char *r;
 
     tmp = *time32;
-    return ctime(&tmp);
+    r = ctime(&tmp);
+    return (r == NULL) ? "(error)" : r;
 }
 
 /* Display time information. */


More information about the cvs-krb5 mailing list