[Bug time/31007] New: strftime() crashes due to %Z in standards-compliant mode

[Bug time/31007] New: strftime() crashes due to %Z in standards-compliant mode

[someplaceguy at wizy dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31007

            Bug ID: 31007
           Summary: strftime() crashes due to %Z in standards-compliant
                    mode
           Product: glibc
           Version: 2.37
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: time
          Assignee: unassigned at sourceware dot org
          Reporter: someplaceguy at wizy dot org
  Target Milestone: ---

The following code crashes quite often for me, even though as far as I can
tell, it should run fine when compiled in a standards-compliant mode (e.g.
-std=c99):

```
#include <locale.h>
#include <string.h>

#ifdef _BSD_SOURCE
# error "oh noes"
#elif defined(_DEFAULT_SOURCE)
# error "why god, why??"
#endif

#include <time.h>

int main()
{
    char buf[2048];
    struct tm time;

    // If you can't reproduce this bug, try uncommenting the following line
    //memset(&time, 0xcc, sizeof(struct tm));

    time.tm_year = 2023 - 1900;
    time.tm_mon = 9;
    time.tm_mday = 30;
    time.tm_hour = 0;
    time.tm_min = 0;
    time.tm_sec = 1;
    time.tm_wday = 1;
    time.tm_yday = 302;
    time.tm_isdst = 0;

    setlocale(LC_TIME, "en_GB.UTF-8");

    strftime(buf, 2048, "%c", &time);

    return 0;
}
```

$ gcc -o strftime strftime.c -Wall -std=c99
$ ./strftime
$ ./strftime
fish: Job 1, './strftime' terminated by signal SIGSEGV (Address boundary error)

As far as I can tell, this happens because "%c" expands to something containing
"%Z" in the "en_GB.UTF-8" locale.

When formatting "%Z", strftime() appears to try to access `struct tm`'s
`tm_zone` field, which should not even exist when <time.h> is included in a
standards-compliant mode (i.e. when _BSD_SOURCE and _DEFAULT_SOURCE aren't
defined).

As far as I can see, it then proceeds to segfault due to the `tm_zone` field
containing an invalid pointer.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug time/31007] strftime() crashes due to %Z in standards-compliant mode

[sam at gentoo dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31007

Sam James <sam at gentoo dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sam at gentoo dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug time/31007] strftime() crashes due to %Z in standards-compliant mode

[schwab@linux-m68k.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31007

Andreas Schwab <schwab@linux-m68k.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #1 from Andreas Schwab <schwab@linux-m68k.org> ---
You cannot pass a partially initialized struct tm to strftime.  The standard is
very clear that it can contain additional members beyond the standard ones.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug time/31007] strftime() crashes due to %Z in standards-compliant mode

[someplaceguy at wizy dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31007

--- Comment #2 from someplaceguy <someplaceguy at wizy dot org> ---
Hi Andreas,

Thank you for your quick feedback! Can you clarify the following 2 issues for
me, please:

1. The way I understood you, you're saying that a C standard-conforming program
is expected to properly initialize extra unknown fields in a struct without
knowing what those fields are, what they mean and therefore, without knowing
how to initialize them properly.

How is that even possible? How would you even write such a program that works
portably across all platforms?

Are you basically saying that the C standard expects all C standard-conforming
programs to _zero-initialize_ all structs that are allowed to contain extra
fields (i.e. beyond those specified in the standard) in order to be portable
across all systems? That is very surprising to me.

And even if so, I thought memset(&time, 0, sizeof(struct tm)) is not even
guaranteed to produce meaningful values for all fields in a struct, e.g. if the
struct tm contains a field of a floating point type (or perhaps a pointer where
NULL != 0 in some weird architecture)?

Anyway, if this is what you're saying, again, it is very surprising to me
(although admittedly I'm not a C standards expert). I thought those extra
fields could be added and used by implementations, but that portable
standards-conforming programs would be expected to ignore them (since they
can't even know what they are!).

2. Even if you are correct about the above, I would argue that at the very
least, the man pages need to be fixed (although I understand these man pages
are not part of glibc).

Specifically, the ctime(3) man page says the following:

       Broken-down time is stored in the structure tm, which is defined in
<time.h> as follows:

           struct tm {
               int tm_sec;    /* Seconds (0-60) */
               int tm_min;    /* Minutes (0-59) */
               int tm_hour;   /* Hours (0-23) */
               int tm_mday;   /* Day of the month (1-31) */
               int tm_mon;    /* Month (0-11) */
               int tm_year;   /* Year - 1900 */
               int tm_wday;   /* Day of the week (0-6, Sunday = 0) */
               int tm_yday;   /* Day in the year (0-365, 1 Jan = 0) */
               int tm_isdst;  /* Daylight saving time */
           };

Additionally, it says:

      The glibc version of struct tm has additional fields

           long tm_gmtoff;           /* Seconds east of UTC */
           const char *tm_zone;      /* Timezone abbreviation */

       defined when _BSD_SOURCE was set before including <time.h>.  This is a
BSD extension, present in 4.3BSD-Reno.

So according to the man page, it is very clear that if you don't define
_BSD_SOURCE then those two fields will not be defined as part of `struct tm`,
and instead, only the fields mentioned above will be defined.

Which means that the program I wrote in my initial comment should have worked
properly instead of crashing.

If you agree, should I report a bug in the kernel bugzilla?

Thank you!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug time/31007] strftime() crashes due to %Z in standards-compliant mode

[someplaceguy at wizy dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31007

--- Comment #3 from someplaceguy <someplaceguy at wizy dot org> ---
Sorry, please ignore the point I made about memset() (I wish I could edit
comments here). I guess you meant to initialize it like e.g. `struct tm time =
{0};`, which would initialize all fields to some meaningful default value
regardless of how `struct tm` is defined.

-- 
You are receiving this mail because:
You are on the CC list for the bug.