[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260330200018.GL1827@brightrain.aerifal.cx>
Date: Mon, 30 Mar 2026 16:00:18 -0400
From: Rich Felker <dalias@...c.org>
To: Hannu Nyman <hannu.nyman@....fi>
Cc: musl@...ts.openwall.com, Shiji Yang <yangshiji66@...look.com>
Subject: Re: strptime in 1.2.6 - is tzname[0/1] guaranteed to be set?
On Mon, Mar 30, 2026 at 03:44:12PM -0400, Rich Felker wrote:
> On Mon, Mar 30, 2026 at 10:34:07PM +0300, Hannu Nyman wrote:
> > Rich Felker kirjoitti 30.3.2026 klo 22.27:
> > > On Mon, Mar 30, 2026 at 10:05:33PM +0300, Hannu Nyman wrote:
> > > > Rich Felker kirjoitti 23.3.2026 klo 19.38:
> > > > > On Mon, Mar 23, 2026 at 06:38:07PM +0200, Hannu Nyman wrote:
> > > > > > Is a new patch version still coming, or was the last one ("the attached is
> > > > > > good") already fine?
> > > > > Here's a fixed one,
> > > > >
> > > > Hello Rich,
> > > >
> > > > the current patch version seems to be still somewhat problematic regarding
> > > > the fallback 'else' part.
> > > >
> > > > + } else {
> > > > + /* FIXME: is this supposed to be an error? */
> > > > + while ((**s|32)-'a' <= 'z'-'a') ++*s;
> > > > + }
> > > >
> > > > To my understanding the part is supposed to consume the alpha chars of the
> > > > passed timezone name string and then enable further parsing of the total
> > > > date stamp with other possible %-modifiers. Apparently the current check
> > > > fails to stop at null \0 and may continue further, causing segmentation
> > > > error at some point.
> > > Oh, it's a signedness error. Changing it to
> > >
> > > while ((**s|32)-'a' < 26U) ++*s;
> > >
> > > would fix it. But isalpha might be a nicer approach, yes, especially
> > > since we have the macro that does exactly that anyway.
> >
> > After further thought, "isalpha" might actually be wrong, as the quoted
> > POSIX timezone format like <-02>2 are also valid.
> >
> > (Reference to our discussion in 2016 ;-)
> > https://www.openwall.com/lists/musl/2016/03/31/9
> >
> > http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
> >
> > In the quoted form, the first character shall be the <less-than-sign> ( '<'
> > ) character and the last character shall be the <greater-than-sign> ( '>' )
> > character. All characters between these quoting characters shall be
> > alphanumeric characters from the portable character set in the current
> > locale, the <plus-sign> ( '+' ) character, or the <hyphen-minus> ( '-' )
> > character. The /std/ and /dst/ fields in this case shall not include the
> > quoting characters.
> >
> > Then the STD string might be "-02" and DST would be empty. So, isalpha
> > would wrongly fail on "-02" which could quite legitimately exist there.
>
> POSIX doesn't specify what %Z does in the case where the input does
> not match either of the tzname strings for the locale. Those quoting
> forms are for the TZ variable, and the quote marks <> are not
> considered part of the name, so they would not have been emitted by
> strftime %Z. Thus, I don't think there's any motivation to match them
> here. In theory maybe we should consume [A-Za-z0-9+-] that could have
> been expanded from a quoted TZ name; I'm not sure. But I don't think
> it matters a lot since this case is not specified. It's really a
> program error to be passing a string to strptime where the %Z field is
> not known to have been produced in a way strptime is well-defined to
> consume.
Fixed version attached.
Rich
View attachment "0001-fix-incorrect-access-to-tzname-by-strptime-Z-convers.patch" of type "text/plain" (3588 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
