Download raw body.
localtime(3) / gmtime(3) error checking for bin / libexec / sbin
localtime(3) / gmtime(3) error checking for bin / libexec / sbin
localtime(3) / gmtime(3) error checking for bin / libexec / sbin
localtime(3) / gmtime(3) error checking for bin / libexec / sbin
On 2024-04-28 08:48 -06, "Todd C. Miller" <Todd.Miller@sudo.ws> wrote:
> On Sun, 28 Apr 2024 12:03:04 +0200, Florian Obser wrote:
>
>> Mostly compile tested only. Most of these probably can't happen. At
>> least I don't expect the kernel to work well if the time is so far in
>> the future that the year can't be represented by a struct tm. But I
>> suppose just adding the error check is easier than arguing why it's not
>> needed.
>
> This looks good to me. I think it is worth adding the checks even
> if failure should be impossible. Some comments inline.
>
> - todd
>
>> diff --git sbin/isakmpd/x509.c sbin/isakmpd/x509.c
>> index 989553897e2..9a83d5d743e 100644
>> --- sbin/isakmpd/x509.c
>> +++ sbin/isakmpd/x509.c
>> @@ -222,8 +222,15 @@ x509_generate_kn(int id, X509 *cert)
>> if (((tm = X509_get_notBefore(cert)) == NULL) ||
>> (tm->type != V_ASN1_UTCTIME &&
>> tm->type != V_ASN1_GENERALIZEDTIME)) {
>> - tt = time(0);
>> - strftime(before, 14, "%Y%m%d%H%M%S", localtime(&tt));
>> + struct tm *ltm;
>> +
>> + tt = time(NULL);
>> + if ((ltm = localtime(&tt)) == NULL) {
>> + LOG_DBG((LOG_POLICY, 30,
>> + "x509_generate_kn: invalid local time"));
>> + goto fail;
>> + }
>> + strftime(before, 14, "%Y%m%d%H%M%S", ltm);
>> timecomp = "LocalTimeOfDay";
>> } else {
>> if (tm->data[tm->length - 1] == 'Z') {
>> @@ -312,7 +319,14 @@ x509_generate_kn(int id, X509 *cert)
>> if (tm == NULL ||
>> (tm->type != V_ASN1_UTCTIME &&
>> tm->type != V_ASN1_GENERALIZEDTIME)) {
>> + struct tm *ltm;
>> +
>> tt = time(0);
>> + if ((ltm = localtime(&tt)) == NULL) {
>> + LOG_DBG((LOG_POLICY, 30,
>> + "x509_generate_kn: invalid local time"));
>> + goto fail;
>> + }
>> strftime(after, 14, "%Y%m%d%H%M%S", localtime(&tt));
>
> In the strftime() call this should now be ltm.
fixed
>
>> timecomp2 = "LocalTimeOfDay";
>> } else {
>> diff --git sbin/shutdown/shutdown.c sbin/shutdown/shutdown.c
>> index 2c74ae3d25a..84f26575b26 100644
>> --- sbin/shutdown/shutdown.c
>> +++ sbin/shutdown/shutdown.c
>> @@ -345,8 +345,11 @@ timewarn(time_t timeleft)
>> if (timeleft > 10 * 60) {
>> shuttime = time(NULL) + timeleft;
>> lt = localtime(&shuttime);
>> - strftime(when, sizeof(when), "%H:%M %Z", lt);
>> - dprintf(fd[1], "System going down at %s\n\n", when);
>> + if (lt != NULL) {
>> + strftime(when, sizeof(when), "%H:%M %Z", lt);
>> + dprintf(fd[1], "System going down at %s\n\n", when);
>> + } else
>> + dprintf(fd[1], "System going down soon\n\n");
>
> Maybe just use the timeleft > 59 case if localtime() fails?
I like it
>
>> } else if (timeleft > 59) {
>> dprintf(fd[1], "System going down in %lld minute%s\n\n",
>> (long long)(timeleft / 60), (timeleft > 60) ? "s" : "");
>> @@ -525,6 +528,9 @@ getoffset(char *timearg)
>> time(&now);
>> lt = localtime(&now); /* current time val */
>>
>> + if (lt == NULL)
>> + badtime();
>> +
>> switch (strlen(timearg)) {
>> case 10:
>> this_year = lt->tm_year;
>> @@ -597,8 +603,13 @@ nolog(time_t timeleft)
>> tm = localtime(&shuttime);
>> if (tm && (logfd = open(_PATH_NOLOGIN, O_WRONLY|O_CREAT|O_TRUNC,
>> 0664)) >= 0) {
>> - strftime(when, sizeof(when), "at %H:%M %Z", tm);
>> - dprintf(logfd, "\n\nNO LOGINS: System going down %s\n\n", when)
>> ;
>> + if (tm) {
>> + strftime(when, sizeof(when), "at %H:%M %Z", tm);
>> + dprintf(logfd,
>> + "\n\nNO LOGINS: System going down %s\n\n", when);
>> + } else
>> + dprintf(logfd,
>> + "\n\nNO LOGINS: System going down soon\n\n");
>
> How about this instead?
> dprintf(logfd,
> "\n\nNO LOGINS: System going down in %lld minute%s\n\n",
> (long long)(timeleft / 60), (timeleft > 60) ? "s" : "");
>
this as well, while there is precedent for "soon", I think it's better
to go with minutes. I've also spotted that this one actually had a check
for localtime failing (it would just be silent). So I've removed the
outer check and added the minutes output.
>> close(logfd);
>> }
>> }
updated diff:
diff --git bin/date/date.c bin/date/date.c
index 0330ffc5f57..43553bf3de1 100644
--- bin/date/date.c
+++ bin/date/date.c
@@ -151,6 +151,8 @@ setthetime(char *p, const char *pformat)
err(1, "pledge");
lt = localtime(&tval);
+ if (lt == NULL)
+ errx(1, "conversion error");
lt->tm_isdst = -1; /* correct for DST */
diff --git bin/ksh/lex.c bin/ksh/lex.c
index 41178694fa6..51128a31bb1 100644
--- bin/ksh/lex.c
+++ bin/ksh/lex.c
@@ -1251,7 +1251,11 @@ dopprompt(const char *sp, int ntruncate, const char **spp, int doprint)
case 'd': /* '\' 'd' Dow Mon DD */
time(&t);
tm = localtime(&t);
- strftime(strbuf, sizeof strbuf, "%a %b %d", tm);
+ if (tm)
+ strftime(strbuf, sizeof strbuf,
+ "%a %b %d", tm);
+ else
+ strbuf[0] = '\0';
break;
case 'D': /* '\' 'D' '{' strftime format '}' */
p = strchr(cp + 2, '}');
@@ -1266,7 +1270,11 @@ dopprompt(const char *sp, int ntruncate, const char **spp, int doprint)
*p = '\0';
time(&t);
tm = localtime(&t);
- strftime(strbuf, sizeof strbuf, tmpbuf, tm);
+ if (tm)
+ strftime(strbuf, sizeof strbuf, tmpbuf,
+ tm);
+ else
+ strbuf[0] = '\0';
cp = strchr(cp + 2, '}');
break;
case 'e': /* '\' 'e' escape */
@@ -1315,22 +1323,38 @@ dopprompt(const char *sp, int ntruncate, const char **spp, int doprint)
case 't': /* '\' 't' 24 hour HH:MM:SS */
time(&t);
tm = localtime(&t);
- strftime(strbuf, sizeof strbuf, "%T", tm);
+ if (tm)
+ strftime(strbuf, sizeof strbuf, "%T",
+ tm);
+ else
+ strbuf[0] = '\0';
break;
case 'T': /* '\' 'T' 12 hour HH:MM:SS */
time(&t);
tm = localtime(&t);
- strftime(strbuf, sizeof strbuf, "%l:%M:%S", tm);
+ if (tm)
+ strftime(strbuf, sizeof strbuf,
+ "%l:%M:%S", tm);
+ else
+ strbuf[0] = '\0';
break;
case '@': /* '\' '@' 12 hour am/pm format */
time(&t);
tm = localtime(&t);
- strftime(strbuf, sizeof strbuf, "%r", tm);
+ if (tm)
+ strftime(strbuf, sizeof strbuf, "%r",
+ tm);
+ else
+ strbuf[0] = '\0';
break;
case 'A': /* '\' 'A' 24 hour HH:MM */
time(&t);
tm = localtime(&t);
- strftime(strbuf, sizeof strbuf, "%R", tm);
+ if (tm)
+ strftime(strbuf, sizeof strbuf, "%R",
+ tm);
+ else
+ strbuf[0] = '\0';
break;
case 'u': /* '\' 'u' username */
strlcpy(strbuf, username, sizeof strbuf);
diff --git bin/pax/sel_subs.c bin/pax/sel_subs.c
index bd0138990b3..f825f2d719b 100644
--- bin/pax/sel_subs.c
+++ bin/pax/sel_subs.c
@@ -572,7 +572,8 @@ str_sec(const char *p, time_t *tval)
return(-1);
}
- lt = localtime(tval);
+ if ((lt = localtime(tval)) == NULL)
+ return (-1);
if (dot != NULL) { /* .SS */
if (strlen(++dot) != 2)
diff --git bin/ps/print.c bin/ps/print.c
index 7c7f75d74b8..a87a9f3ae67 100644
--- bin/ps/print.c
+++ bin/ps/print.c
@@ -524,6 +524,10 @@ started(const struct pinfo *pi, VARENT *ve)
startt = kp->p_ustart_sec;
tp = localtime(&startt);
+ if (tp == NULL) {
+ (void)printf("%-*s", v->width, "-");
+ return;
+ }
if (!now)
(void)time(&now);
if (now - kp->p_ustart_sec < 12 * SECSPERHOUR) {
diff --git libexec/ftpd/ftpcmd.y libexec/ftpd/ftpcmd.y
index e85d7ccb72f..4222dd2e253 100644
--- libexec/ftpd/ftpcmd.y
+++ libexec/ftpd/ftpcmd.y
@@ -613,6 +613,11 @@ cmd
} else {
struct tm *t;
t = gmtime(&stbuf.st_mtime);
+ if (t == NULL) {
+ /* invalid time, use epoch */
+ stbuf.st_mtime = 0;
+ t = gmtime(&stbuf.st_mtime);
+ }
reply(213,
"%04d%02d%02d%02d%02d%02d",
1900 + t->tm_year,
diff --git libexec/getty/main.c libexec/getty/main.c
index 3ac0939d8c8..0a77d14ad04 100644
--- libexec/getty/main.c
+++ libexec/getty/main.c
@@ -562,10 +562,12 @@ putf(char *cp)
break;
case 'd': {
- (void)time(&t);
- (void)strftime(db, sizeof(db),
- "%l:%M%p on %A, %d %B %Y", localtime(&t));
- xputs(db);
+ struct tm *tm;
+ time(&t);
+ if ((tm = localtime(&t)) != NULL)
+ if (strftime(db, sizeof(db),
+ "%l:%M%p on %A, %d %B %Y", tm) != 0)
+ xputs(db);
break;
}
diff --git libexec/snmpd/snmpd_metrics/mib.c libexec/snmpd/snmpd_metrics/mib.c
index a3b80ddf9ac..87116770a16 100644
--- libexec/snmpd/snmpd_metrics/mib.c
+++ libexec/snmpd/snmpd_metrics/mib.c
@@ -296,27 +296,29 @@ mib_hrsystemdate(struct agentx_varbind *vb)
int tzoffset;
unsigned short year;
+ memset(s, 0, sizeof(s));
(void)time(&now);
ptm = localtime(&now);
- year = htons(ptm->tm_year + 1900);
- memcpy(s, &year, 2);
- s[2] = ptm->tm_mon + 1;
- s[3] = ptm->tm_mday;
- s[4] = ptm->tm_hour;
- s[5] = ptm->tm_min;
- s[6] = ptm->tm_sec;
- s[7] = 0;
-
- tzoffset = ptm->tm_gmtoff;
- if (tzoffset < 0)
- s[8] = '-';
- else
- s[8] = '+';
-
- s[9] = abs(tzoffset) / 3600;
- s[10] = (abs(tzoffset) - (s[9] * 3600)) / 60;
+ if (ptm != NULL) {
+ year = htons(ptm->tm_year + 1900);
+ memcpy(s, &year, 2);
+ s[2] = ptm->tm_mon + 1;
+ s[3] = ptm->tm_mday;
+ s[4] = ptm->tm_hour;
+ s[5] = ptm->tm_min;
+ s[6] = ptm->tm_sec;
+ s[7] = 0;
+
+ tzoffset = ptm->tm_gmtoff;
+ if (tzoffset < 0)
+ s[8] = '-';
+ else
+ s[8] = '+';
+ s[9] = abs(tzoffset) / 3600;
+ s[10] = (abs(tzoffset) - (s[9] * 3600)) / 60;
+ }
agentx_varbind_nstring(vb, s, sizeof(s));
}
diff --git libexec/talkd/announce.c libexec/talkd/announce.c
index 00bdd0b8c96..66ad22157cb 100644
--- libexec/talkd/announce.c
+++ libexec/talkd/announce.c
@@ -102,9 +102,14 @@ print_mesg(FILE *tf, CTL_MSG *request, char *remote_machine)
sizes[i] = strlen(line_buf[i]);
max_size = max(max_size, sizes[i]);
i++;
- (void)snprintf(line_buf[i], N_CHARS,
- "Message from Talk_Daemon@%s at %d:%02d ...",
- hostname, localclock->tm_hour , localclock->tm_min );
+ if (localclock) {
+ (void)snprintf(line_buf[i], N_CHARS,
+ "Message from Talk_Daemon@%s at %d:%02d ...",
+ hostname, localclock->tm_hour , localclock->tm_min );
+ } else {
+ (void)snprintf(line_buf[i], N_CHARS,
+ "Message from Talk_Daemon@%s ...", hostname);
+ }
sizes[i] = strlen(line_buf[i]);
max_size = max(max_size, sizes[i]);
i++;
diff --git sbin/dhclient/dhclient.c sbin/dhclient/dhclient.c
index 97fc9b5b2fe..dc65bdf18e0 100644
--- sbin/dhclient/dhclient.c
+++ sbin/dhclient/dhclient.c
@@ -2079,6 +2079,7 @@ lease_as_string(char *type, struct client_lease *lease)
static char string[8192];
char timebuf[27]; /* 6 2017/04/08 05:47:50 UTC; */
struct option_data *opt;
+ struct tm *tm;
char *buf, *name;
time_t t;
size_t rslt;
@@ -2138,19 +2139,25 @@ lease_as_string(char *type, struct client_lease *lease)
free(buf);
t = lease->epoch + lease_renewal(lease);
- rslt = strftime(timebuf, sizeof(timebuf), DB_TIMEFMT, gmtime(&t));
+ if ((tm = gmtime(&t)) == NULL)
+ return NULL;
+ rslt = strftime(timebuf, sizeof(timebuf), DB_TIMEFMT, tm);
if (rslt == 0)
return NULL;
append_statement(string, sizeof(string), " renew ", timebuf);
t = lease->epoch + lease_rebind(lease);
- rslt = strftime(timebuf, sizeof(timebuf), DB_TIMEFMT, gmtime(&t));
+ if ((tm = gmtime(&t)) == NULL)
+ return NULL;
+ rslt = strftime(timebuf, sizeof(timebuf), DB_TIMEFMT, tm);
if (rslt == 0)
return NULL;
append_statement(string, sizeof(string), " rebind ", timebuf);
t = lease->epoch + lease_expiry(lease);
- rslt = strftime(timebuf, sizeof(timebuf), DB_TIMEFMT, gmtime(&t));
+ if ((tm = gmtime(&t)) == NULL)
+ return NULL;
+ rslt = strftime(timebuf, sizeof(timebuf), DB_TIMEFMT, tm);
if (rslt == 0)
return NULL;
append_statement(string, sizeof(string), " expire ", timebuf);
diff --git sbin/isakmpd/log.c sbin/isakmpd/log.c
index 5a0df1df5e9..aeab7b085c4 100644
--- sbin/isakmpd/log.c
+++ sbin/isakmpd/log.c
@@ -182,7 +182,11 @@ _log_print(int error, int syslog_level, const char *fmt, va_list ap,
if (log_output) {
gettimeofday(&now, 0);
t = now.tv_sec;
- tm = localtime(&t);
+ if ((tm = localtime(&t)) == NULL) {
+ /* Invalid time, use the epoch. */
+ t = 0;
+ tm = localtime(&t);
+ }
if (class >= 0)
snprintf(nbuf, sizeof nbuf,
"%02d%02d%02d.%06ld %s %02d ",
diff --git sbin/isakmpd/policy.c sbin/isakmpd/policy.c
index 9dbf339c0db..1df46dca0fc 100644
--- sbin/isakmpd/policy.c
+++ sbin/isakmpd/policy.c
@@ -1728,13 +1728,23 @@ policy_callback(char *name)
return phase_1;
if (strcmp(name, "GMTTimeOfDay") == 0) {
+ struct tm *tm;
tt = time(NULL);
- strftime(mytimeofday, 14, "%Y%m%d%H%M%S", gmtime(&tt));
+ if ((tm = gmtime(&tt)) == NULL) {
+ log_error("policy_callback: invalid time %lld", tt);
+ goto bad;
+ }
+ strftime(mytimeofday, 14, "%Y%m%d%H%M%S", tm);
return mytimeofday;
}
if (strcmp(name, "LocalTimeOfDay") == 0) {
+ struct tm *tm;
tt = time(NULL);
- strftime(mytimeofday, 14, "%Y%m%d%H%M%S", localtime(&tt));
+ if ((tm = localtime(&tt)) == NULL) {
+ log_error("policy_callback: invalid time %lld", tt);
+ goto bad;
+ }
+ strftime(mytimeofday, 14, "%Y%m%d%H%M%S", tm);
return mytimeofday;
}
if (strcmp(name, "initiator") == 0)
diff --git sbin/isakmpd/x509.c sbin/isakmpd/x509.c
index 989553897e2..ffeff089967 100644
--- sbin/isakmpd/x509.c
+++ sbin/isakmpd/x509.c
@@ -222,8 +222,15 @@ x509_generate_kn(int id, X509 *cert)
if (((tm = X509_get_notBefore(cert)) == NULL) ||
(tm->type != V_ASN1_UTCTIME &&
tm->type != V_ASN1_GENERALIZEDTIME)) {
- tt = time(0);
- strftime(before, 14, "%Y%m%d%H%M%S", localtime(&tt));
+ struct tm *ltm;
+
+ tt = time(NULL);
+ if ((ltm = localtime(&tt)) == NULL) {
+ LOG_DBG((LOG_POLICY, 30,
+ "x509_generate_kn: invalid local time"));
+ goto fail;
+ }
+ strftime(before, 14, "%Y%m%d%H%M%S", ltm);
timecomp = "LocalTimeOfDay";
} else {
if (tm->data[tm->length - 1] == 'Z') {
@@ -312,8 +319,15 @@ x509_generate_kn(int id, X509 *cert)
if (tm == NULL ||
(tm->type != V_ASN1_UTCTIME &&
tm->type != V_ASN1_GENERALIZEDTIME)) {
+ struct tm *ltm;
+
tt = time(0);
- strftime(after, 14, "%Y%m%d%H%M%S", localtime(&tt));
+ if ((ltm = localtime(&tt)) == NULL) {
+ LOG_DBG((LOG_POLICY, 30,
+ "x509_generate_kn: invalid local time"));
+ goto fail;
+ }
+ strftime(after, 14, "%Y%m%d%H%M%S", ltm);
timecomp2 = "LocalTimeOfDay";
} else {
if (tm->data[tm->length - 1] == 'Z') {
diff --git sbin/newfs_msdos/newfs_msdos.c sbin/newfs_msdos/newfs_msdos.c
index dae0a704daa..f170a70399f 100644
--- sbin/newfs_msdos/newfs_msdos.c
+++ sbin/newfs_msdos/newfs_msdos.c
@@ -550,7 +550,9 @@ main(int argc, char *argv[])
if (!opt_N) {
gettimeofday(&tv, NULL);
now = tv.tv_sec;
- tm = localtime(&now);
+ if ((tm = localtime(&now)) == NULL)
+ errx(1, "Invalid time");
+
if (!(img = malloc(bpb.bps)))
err(1, NULL);
dir = bpb.res + (bpb.spf ? bpb.spf : bpb.bspf) * bpb.nft;
diff --git sbin/route/route.c sbin/route/route.c
index 755e548e9cc..f4e45584234 100644
--- sbin/route/route.c
+++ sbin/route/route.c
@@ -1878,7 +1878,10 @@ bfd_calc_uptime(time_t time)
fmt = "%Ss";
tp = localtime(&time);
- (void)strftime(buf, sizeof(buf), fmt, tp);
+ if (tp)
+ (void)strftime(buf, sizeof(buf), fmt, tp);
+ else
+ buf[0] = '\0';
return (buf);
}
diff --git sbin/shutdown/shutdown.c sbin/shutdown/shutdown.c
index 2c74ae3d25a..618edd20776 100644
--- sbin/shutdown/shutdown.c
+++ sbin/shutdown/shutdown.c
@@ -345,8 +345,13 @@ timewarn(time_t timeleft)
if (timeleft > 10 * 60) {
shuttime = time(NULL) + timeleft;
lt = localtime(&shuttime);
- strftime(when, sizeof(when), "%H:%M %Z", lt);
- dprintf(fd[1], "System going down at %s\n\n", when);
+ if (lt != NULL) {
+ strftime(when, sizeof(when), "%H:%M %Z", lt);
+ dprintf(fd[1], "System going down at %s\n\n", when);
+ } else
+ dprintf(fd[1], "System going down in %lld minute%s\n\n",
+ (long long)(timeleft / 60),
+ (timeleft > 60) ? "s" : "");
} else if (timeleft > 59) {
dprintf(fd[1], "System going down in %lld minute%s\n\n",
(long long)(timeleft / 60), (timeleft > 60) ? "s" : "");
@@ -525,6 +530,9 @@ getoffset(char *timearg)
time(&now);
lt = localtime(&now); /* current time val */
+ if (lt == NULL)
+ badtime();
+
switch (strlen(timearg)) {
case 10:
this_year = lt->tm_year;
@@ -595,10 +603,17 @@ nolog(time_t timeleft)
(void)signal(SIGTERM, finish);
shuttime = time(NULL) + timeleft;
tm = localtime(&shuttime);
- if (tm && (logfd = open(_PATH_NOLOGIN, O_WRONLY|O_CREAT|O_TRUNC,
+ if ((logfd = open(_PATH_NOLOGIN, O_WRONLY|O_CREAT|O_TRUNC,
0664)) >= 0) {
- strftime(when, sizeof(when), "at %H:%M %Z", tm);
- dprintf(logfd, "\n\nNO LOGINS: System going down %s\n\n", when);
+ if (tm) {
+ strftime(when, sizeof(when), "at %H:%M %Z", tm);
+ dprintf(logfd,
+ "\n\nNO LOGINS: System going down %s\n\n", when);
+ } else
+ dprintf(logfd,
+ "\n\nNO LOGINS: System going in %lld minute%s\n\n",
+ (long long)(timeleft / 60),
+ (timeleft > 60) ? "s" : "");
close(logfd);
}
}
diff --git sbin/unwind/libunbound/util/log.c sbin/unwind/libunbound/util/log.c
index a15ee920c0f..86a3acee43c 100644
--- sbin/unwind/libunbound/util/log.c
+++ sbin/unwind/libunbound/util/log.c
@@ -271,8 +271,8 @@ log_vmsg(int pri, const char* type,
}
now = (time_t)time(NULL);
#if defined(HAVE_STRFTIME) && defined(HAVE_LOCALTIME_R)
- if(log_time_asc && strftime(tmbuf, sizeof(tmbuf), "%b %d %H:%M:%S",
- localtime_r(&now, &tm))%(sizeof(tmbuf)) != 0) {
+ if(log_time_asc && localtime_r(&now, &tm) && strftime(tmbuf,
+ sizeof(tmbuf), "%b %d %H:%M:%S", &tm)%(sizeof(tmbuf)) != 0) {
/* %sizeof buf!=0 because old strftime returned max on error */
fprintf(logfile, "%s %s[%d:%x] %s: %s\n", tmbuf,
ident, (int)getpid(), tid?*tid:0, type, message);
--
In my defence, I have been left unsupervised.
localtime(3) / gmtime(3) error checking for bin / libexec / sbin
localtime(3) / gmtime(3) error checking for bin / libexec / sbin
localtime(3) / gmtime(3) error checking for bin / libexec / sbin
localtime(3) / gmtime(3) error checking for bin / libexec / sbin