On 2023-11-10 10:44, Pedro Luis Castedo Cepeda wrote:
> El 10/11/2023 a las 11:16, Corinna Vinschen escribió:
>> On Nov  9 23:17, Brian Inglis wrote:
>>> On 2023-11-09 12:04, Pedro Luis Castedo Cepeda wrote:
>>>> - Prevent strftime to parsing format string beyond its end when
>>>>     it finish with "%E" or "%O".
>>>> ---
>>>>    newlib/libc/time/strftime.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/newlib/libc/time/strftime.c b/newlib/libc/time/strftime.c
>>>> index 56f227c5f..c4e9e45a9 100644
>>>> --- a/newlib/libc/time/strftime.c
>>>> +++ b/newlib/libc/time/strftime.c
>>>> @@ -754,6 +754,8 @@ __strftime (CHAR *s, size_t maxsize, const CHAR *format,
>>>>          switch (*format)
>>>>        {
>>>> +    case CQ('\0'):
>>>> +      break;
>>>>        case CQ('a'):
>>>>          _ctloc (wday[tim_p->tm_wday]);
>>>>          for (i = 0; i < ctloclen; i++)
>>>
>>> These cases appear to already be taken care of by setting and using
>>> (depending on the config parameters) the "alt" variable for those modifiers,
>>> and the default: return 0; for the format *character* (possibly wide) not
>>> matching following any modifiers.
>>>
>>> Patches to newlib should go to the newlib mailing list at sourceware dot org.
>>
>> Also, a simple reproducer would be nice.

> My first contribution. Sorry about posting to wrong mail list and, at best, 
> minimalistic patch motivation reasoning. First time with git send-mail, too.
> 
> I came across this newlib "feature" trying to update GLib port to 2.78.1. When 
> trying to find out why test_strftime (glib/test/date.c)
> was failing I discovered that one of the test format strings, "%E" was 
> triggering a loop in g_date_strftime (glib/gdate.c) requiring more and more 
> memory till it was stopped by a fortunate maximum size check in function.
> 
> The problem is that __strftime  (newlib/libc/time/strftime.c) doesn't check for 
> '\0' after a terminal "%E" and it continues parsing the format string. Finally 
> (not sure if intentionally), this triggers a direct return 0 from __strftime 
> instead breaking the loop, preventing it from add '\0' to the end of returned 
> string. Same for "%O", I think (not tested).
> 
> It seems that this trailing '\0' allows to differentiate returning an empty 
> string from needing more space (at least, in Glib).
> 
> So, is it a newlib bug? Not really, I think this format string is bad-formed (%E 
> should modify something, shouldn't it?) So undefined behaviour is OK. I could 
> patch-out these format strings from the port.
> 
> But... from Glib tests, it seems that, at least:
> 
> - If G_OS_WIN32, terminal "%E" & "%O" are silently discarded.
> - If __FreeBSD__ || __OpenBSD__ || __APPLE__ they are transformed to E & O, 
> respectively.
> - And if #else the same thing is expected.
> 
> So it seems that returning 0-terminated string is a common practice and I also 
> think that this is more deterministic and, potentially, safer. That's why I sent 
> the patch. It tries to be the shortest addition to check for end of string after 
> %E & %O modifiers and takes G_OS_WIN32 approach (only cause it's the simplest).

Not seeing any issue with any format - see attached source and log output, built 
under Cygwin.
[Derived from a bash script using printf %(...)T to do the same thing.]

-- 
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry