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.
#include <stdlib.h> #include <stdbool.h> #include <string.h> #include <time.h> #include <locale.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 }; setenv("LC_ALL", "en_US.utf-8", true); setlocale (LC_ALL, ""); char tmpbuf[128]; size_t tmplen; tmpbuf[0] = '\1'; tmplen = strftime(tmpbuf, sizeof(tmpbuf), "%E", &date); assert(tmpbuf[0] = '\0' || strncmp(tmpbuf, "E", 2)); tmplen = strftime(tmpbuf, sizeof(tmpbuf), "%O", &date); assert(tmpbuf[0] = '\0' || strncmp(tmpbuf, "O", 2)); return EXIT_SUCCESS; }
smime.p7s
Description: Firma criptográfica S/MIME