El 11/11/2023 a las 6:57, Brian Inglis escribió:
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.]
OK. It's not a newlib problem but a GLib one as it is relaying on common
but non-standard strftime implementation details.
I attach a short program more focused in g_date_strftime implementation
so it can be evaluated if it worths addressing this corner case.
Thanks.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <assert.h>
int main(int argc, char *argv[])
{
const struct tm date = {
.tm_sec = 0, .tm_min = 0, .tm_hour = 0, .tm_mday = 1, .tm_mon = 0,
.tm_year = -1899, .tm_wday = 1, .tm_yday = 0, .tm_isdst = -1,
.tm_gmtoff = 0, .tm_zone = 0x0
};
const char *fmt = "%E";
const size_t maxsz = 65536u;
size_t bufsz = 128;
char *buf = NULL;
// Mimic g_date_strftime (glib/gdate.c) approach
do {
buf = (char *)realloc(buf, bufsz);
assert(buf);
buf[0] = '\1'; // Mark to guess if empty or not enough space
size_t len = strftime(buf, bufsz, fmt, &date);
if (len != 0 || buf[0] == '\0')
break; // OK, done.
bufsz *= 2; // Assume more space needed
} while (bufsz <= maxsz); // .. up to a limit
int rc;
if (bufsz <= maxsz)
{
printf("Date: %s\n", buf);
rc = EXIT_SUCCESS;
}
else
{
puts("Date: longest date ever :-\\");
rc = EXIT_FAILURE;
}
free(buf);
return rc;
}
