|
|
Message-ID: <a1a10f25-b63f-43b6-9367-5fe70f2f6117@gmail.com>
Date: Wed, 4 Nov 2026 01:48:08 -0800
From: Mahdi Rakhshandehroo <mahdi.rakhshandehroo@...il.com>
To: musl@...ts.openwall.com
Subject: [PATCH] Fix potentially unbounded recursion in
strftime/strftime_l/wcsftime when fed with a crafted locale
(Please CC me on replies)
__strftime_fmt_1() expands the %c, %x, %X and %r conversions by
re-entering __strftime_l() with a format string fetched from the
locale's LC_TIME data (D_T_FMT, D_FMT, T_FMT, T_FMT_AMPM) through
__nl_langinfo_l(). Unlike %D/%F/%R/%T, which expand into fixed literal
formats, these sub-formats come from the locale, so a catalog whose
format contains a conversion leading back to the same item (directly,
e.g. D_T_FMT = "%c", or through a cycle, e.g. D_T_FMT = "%x" with
D_FMT = "%c") closes the cycle:
__strftime_l() -> __strftime_fmt_1() -> __strftime_l()
which then recurses without bound. The 100-byte intermediate buffer does
not limit the recursion depth, since format expansion does not advance
the output length.
Recursion is triggered when the following criteria are met:
- A LC_TIME locale from a crafted file under MUSL_LOCPATH is used
- strftime(), strftime_l() or wcsftime() is invoked with n > 0 and a
format string with %c/%x/%X/%r
This patch bounds the recursion by threading a depth counter through the
call cycle. The following changes are made:
- strftime.c defines MAXIMUM_RECURSION_DEPTH with a value of 5. I
consider this a sensible upper bound, as no reasonable locale should
require that many levels of expansion.
- The body of __strftime_l() moves to strftime_l_helper().
__strftime_l() keeps its signature and simply forwards to
strftime_l_helper() with depth 0.
- __strftime_fmt_1() gains a depth parameter (the value belonging to
the strftime_l_helper() frame that called it) and passes depth+1 to
strftime_l_helper() at the recu_strftime site. wcsftime() starts at
depth 0 as well.
- Before recursing, __strftime_fmt_1() checks the depth against
MAXIMUM_RECURSION_DEPTH and returns 0 when the next level would
exceed it. This return value is propagated up the call stack, so
strftime()/strftime_l()/wcsftime() return 0 when the recursion limit
is reached.
Below is a minimal reproduction of the issue, written as a single self
contained shell script that runs in a fresh alpine-based system. The
repro can be run with Docker like so:
docker run --rm -i alpine /bin/sh < repro.sh
The script is as follows:
--------------------[repro.sh]--------------------
set -e
apk add --no-cache gcc musl-dev gettext > /dev/null
cat > repro.c << 'EOF'
int main(void)
{
struct tm tm = {0};
char buf[64];
setlocale(LC_ALL, "");
printf("%zu\n", strftime(buf, sizeof buf, "%c", &tm));
return 0;
}
EOF
cat > evil.po << 'EOF'
msgid ""
msgstr ""
"Content-Type: text/plain; charset=UTF-8\n"
msgid "%a %b %e %T %Y"
msgstr "%c"
EOF
gcc -static -o repro repro.c
msgfmt -o evil evil.po
set +e
MUSL_LOCPATH=$PWD LC_ALL=evil ./repro
STATUS=$?
echo "repro exit status: $STATUS"
if [ "$STATUS" -eq 139 ]; then
echo "strftime() died from unbounded recursion (SIGSEGV)"
else
echo "strftime() returned exit status $STATUS"
fi
--------------------------------------------------
With an unpatched version of musl, the program emits a segfault due to
stack overflow. With a version of musl that is compiled with the fix,
strftime returns 0 and the program prints 0.
---
src/time/strftime.c | 16 +++++++++++++---
src/time/time_impl.h | 2 +-
src/time/wcsftime.c | 2 +-
3 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/src/time/strftime.c b/src/time/strftime.c
index c40246d..b1268c0 100644
--- a/src/time/strftime.c
+++ b/src/time/strftime.c
@@ -9,6 +9,8 @@
#include "locale_impl.h"
#include "time_impl.h"
+#define MAXIMUM_RECURSION_DEPTH 5
+
static int is_leap(int y)
{
/* Avoid overflow */
@@ -43,7 +45,9 @@ static int week_num(const struct tm *tm)
return val;
}
-const char *__strftime_fmt_1(char (*s)[100], size_t *l, int f, const
struct tm *tm, locale_t loc, int pad)
+static size_t strftime_l_helper(char *restrict s, size_t n, const char
*restrict f, const struct tm *restrict tm, locale_t loc, size_t depth);
+
+const char *__strftime_fmt_1(char (*s)[100], size_t *l, int f, const
struct tm *tm, locale_t loc, int pad, size_t depth)
{
nl_item item;
long long val;
@@ -208,12 +212,18 @@ string:
nl_strftime:
fmt = __nl_langinfo_l(item, loc);
recu_strftime:
- *l = __strftime_l(*s, sizeof *s, fmt, tm, loc);
+ if (depth >= MAXIMUM_RECURSION_DEPTH) return 0;
+ *l = strftime_l_helper(*s, sizeof *s, fmt, tm, loc, depth+1);
if (!*l) return 0;
return *s;
}
size_t __strftime_l(char *restrict s, size_t n, const char *restrict
f, const struct tm *restrict tm, locale_t loc)
+{
+ return strftime_l_helper(s, n, f, tm, loc, 0);
+}
+
+static size_t strftime_l_helper(char *restrict s, size_t n, const char
*restrict f, const struct tm *restrict tm, locale_t loc, size_t depth)
{
size_t l, k;
char buf[100];
@@ -247,7 +257,7 @@ size_t __strftime_l(char *restrict s, size_t n,
const char *restrict f, const st
}
f = p;
if (*f == 'E' || *f == 'O') f++;
- t = __strftime_fmt_1(&buf, &k, *f, tm, loc, pad);
+ t = __strftime_fmt_1(&buf, &k, *f, tm, loc, pad, depth);
if (!t) break;
if (width) {
/* Trim off any sign and leading zeros, then
diff --git a/src/time/time_impl.h b/src/time/time_impl.h
index ffe5050..6e7a3b8 100644
--- a/src/time/time_impl.h
+++ b/src/time/time_impl.h
@@ -8,5 +8,5 @@ hidden const char *__tm_to_tzname(const struct tm *);
hidden int __tzname_to_isdst(const char *restrict *);
hidden int __secs_to_tm(long long, struct tm *);
hidden void __secs_to_zone(long long, int, int *, long *, long *,
const char **);
-hidden const char *__strftime_fmt_1(char (*)[100], size_t *, int, const
struct tm *, locale_t, int);
+hidden const char *__strftime_fmt_1(char (*)[100], size_t *, int, const
struct tm *, locale_t, int, size_t);
extern hidden const char __utc[];
diff --git a/src/time/wcsftime.c b/src/time/wcsftime.c
index 8e1437b..b60be84 100644
--- a/src/time/wcsftime.c
+++ b/src/time/wcsftime.c
@@ -35,7 +35,7 @@ size_t __wcsftime_l(wchar_t *restrict s, size_t n,
const wchar_t *restrict f, co
}
f = p;
if (*f == 'E' || *f == 'O') f++;
- t_mb = __strftime_fmt_1(&buf, &k, *f, tm, loc, pad);
+ t_mb = __strftime_fmt_1(&buf, &k, *f, tm, loc, pad, 0);
if (!t_mb) break;
k = mbstowcs(wbuf, t_mb, sizeof wbuf / sizeof *wbuf);
if (k == (size_t)-1) return 0;
--
2.56.0
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.