[tz] Minor (unimportant really) technical UB bug in strftime() ?

Robert Elz kre at munnari.OZ.AU
Tue Nov 8 13:07:01 UTC 2022


The code for the %s strftime conversion starts as:
(this taken from tzcode2022f strftime.c - but I suspect
that many versions are the same)

                        case 's':
                                {
                                        struct tm       tm;
                                        char            buf[INT_STRLEN_MAXIMUM(
                                                                time_t) + 1];
                                        time_t          mkt;  
        
                                        tm = *t;
                                        mkt = mktime(&tm);

The "tm = *t;" line is the problem.   It (or more correctly, something
to replace that) is needed, as *t is const, strftime() is not permitted
to modify it, yet the struct tm * passed to mktime() is not const, as
mktime is sometimes required to modify that struct before returning.

However, mktime() refers only to the fields tm_year tm_mon tm_mday
tm_hour tm_min tm_sec and tm_isdst of the struct passed to it.  The
%s conversion is defined as producing (in a decimal string format) the
result of mktime() applied to the struct tm passed to strftime.
Hence an application which intends to use strftime(..., "%s", ...)
need only initialise those 7 fields, the other 2 (currently) standard
fields (tm_yday and tm_wday) are explicitly ignored by mktime() on
input, so there is no need to assign those values, and any other
fields that might exist in a particular implementation are out of
scope for a portable (conforming) application to touch, so cannot
be initialised.

Since referring to an uninitialised variable is, I believe, technically
undefined behaviour, doing a struct copy (copying all fields) can have
that effect.   (An application could memset() the entire struct before
providing the values it must add, but is not required to do that).

It seems likely that the simple fix is to replace the struct assignment
with 7 individual assignment statements

	tm.tm_year = t->tm_year;
	tm.tm_mon = t->tm_mon;
	/* ... */

On the other hand, hardware that balks at simply loading or copying,
uninitialised values, without using them for any purpose is not exactly
common (and half the world's code would probably break if an implementation
that worked in such a manner was ever created) so I'd call this one of
the less significant issues to worry about fixing.

kre



More information about the tz mailing list