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.
> 
> 
> Thanks,
> Corinna

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).

Best regards.