Re: strftime segfault vs return error code

[OndÅej BÃlka <neleai at seznam dot cz>]

On Sun, Jul 12, 2015 at 04:34:23PM +1000, Adam Nielsen wrote:
> Hi all,
> 
> I'm debugging a long standing problem with the GKrellM app, and I have
> found that it is triggered by a bug that passes out-of-range data to
> the strftime() function.
> 
> This results in a segfault (in strlen(), which must be called
> internally by strftime) so I am wondering whether this is the ideal
> behaviour.  I would have expected out-of-range values to cause strftime
> to return an error (or an empty string) rather than crash.
> 
> You can reproduce this error by setting an out-of-range value for the
> month, and then supplying a format specifier for the month name.  Here
> is an example:
> 
>   #include <time.h>
>   #include <stdio.h>
>   #include <stdlib.h>
>   #include <string.h>
> 
>   int main(int argc, char *argv[]) {
>     char outstr[200];
>     struct tm tmp;
>     memset(&tmp, 0, sizeof(tmp));
>     tmp.tm_mon = 1000;
> 
>     if (strftime(outstr, sizeof(outstr), "%b", &tmp) == 0) {
>       fprintf(stderr, "strftime returned 0");
>       exit(EXIT_FAILURE);
>     }
> 
>     printf("Result string is \"%s\"\n", outstr);
>     exit(EXIT_SUCCESS);
>   }
> 
> Wouldn't it be better in this case for strftime() to return 0, rather
> than crashing?  I'm not sure if there are any security implications in
> this current behaviour.
> 
>From practical perspective crashing/abort tends to be best. Users
typically don't check return value and its better fail early and loudly
than silently corrupting data. As timespec is constructed by programmer
he wrote underlying bug that caused it, strptime doesn't set invalid
months.

You could write patch to add asserts in strftime to make debugging
easier.