diff mbox series

[uclibc-ng-devel] time: fix two wcsftime bugs, a use-after-free and a byte read as a character

Message ID 20260831080211.3552205-1-lordrasmus@gmail.com
State New
Headers show
Series [uclibc-ng-devel] time: fix two wcsftime bugs, a use-after-free and a byte read as a character | expand

Commit Message

Ramin Moussavi Aug. 31, 2026, 8:02 a.m. UTC 
From: ramin <lordrasmus@gmail.com>

tst_wcsftime in uclibc-ng-test covers this case: it failed on aarch64-be and
mips64-be-n64 with every date wrong.

First, the use-after-free.  A stacked conversion converts its narrow
replacement text to wide on the heap and points p at it, and OUTPUT frees the
top allocation at the end of every pass:

	if (allocno >= 0)
		free((void *)alloc[allocno--]);

That is right for a converted value, which OUTPUT has just copied out, and
wrong for a converted format string, which the loop keeps reading through p.
Both used fmt_to_wc(), so both landed on the same stack.  For %n and %t --
the only replacements that are literal text rather than a format -- the very
next thing the loop does is read *p out of the block it just freed.

The bug is not endian specific; what the read finds is.  On little-endian the
freed block happened to answer 0 and the loop ended correctly.  On big-endian
64-bit it answered 0x00120012, the wrong half of an allocator size field, and
wcsftime returned one character too many.  %D, %F, %R and %T survive because
their replacement contains %, so a value conversion sits on top of the
allocation stack and takes the free instead; their own later reads land deeper
in the freed block, past the header, and get away with it.

Values keep fmt_to_wc(); format strings get stacked_to_wc(), which stores the
buffer with its stack level and is freed when that level is popped.  The
truncation path releases what is still held, since it returns without popping.

Second, %c, %x, %X and %r returned "Sun" on big-endian:

	ccp = nl_langinfo(_NL_ITEM(LC_TIME, (int)(*((unsigned char *)p))) ...)

p is wchar_t* in this translation, so reading its first byte gives the high
one on a big-endian target, always 0 -- and item 0 of LC_TIME is ABDAY_1.
UCHAR_T is unsigned char in the narrow build and unsigned int in the wide one,
which is exactly the widening wanted, so (int)(UCHAR_T)(*p) is right for both.
Same for the ERA branch's p[4].

Measured on mips64-be-n64 before and after, against strftime on the same
target: %n 2 -> 1, %t 2 -> 1, %c 3 -> 24, %x 3 -> 8, %X 3 -> 8, %r 3 -> 11,
and the test's own format 24 -> 21.  tst_wcsftime passes there now.

For what it is worth, none of the three obvious references has this shape:
glibc's strftime_l.c contains no malloc at all and writes %n as
add (1, *p = L_('\n')), musl formats each conversion into a stack buffer, and
bionic converts the format to narrow once, calls strftime and converts back.
Allocating a buffer per expansion and freeing it in the output loop is ours
alone.

Signed-off-by: Ramin Moussavi <lordrasmus@gmail.com>
---
 libc/misc/time/time.c | 53 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/libc/misc/time/time.c b/libc/misc/time/time.c
index b6a5b097d..f7dd1313a 100644
--- a/libc/misc/time/time.c
+++ b/libc/misc/time/time.c
@@ -1022,12 +1022,23 @@  static wchar_t* fmt_to_wc_1(const char *src)
 	}
 	return dest;
 }
+/* For a converted value: consumed at OUTPUT, which frees it there. */
 # define fmt_to_wc(dest, src) \
 	dest = alloc[++allocno] = fmt_to_wc_1(src)
+/* For a converted format string: read through p until the level is popped, so
+ * it belongs to the level and not to OUTPUT.  Freeing it at OUTPUT is a
+ * use-after-free -- the loop then reads *p out of the freed block. */
+# define stacked_to_wc(dest, src) \
+	do { \
+		if (fmt_alloc[lvl - 1] != NULL) \
+			free((void *)fmt_alloc[lvl - 1]); \
+		dest = fmt_alloc[lvl - 1] = fmt_to_wc_1(src); \
+	} while (0)
 # define to_wc(dest, src) \
 	dest = fmt_to_wc_1(src)
 #else
 # define fmt_to_wc(dest, src) (dest) = (src)
+# define stacked_to_wc(dest, src) (dest) = (src)
 # define to_wc(dest, src) (dest) = (src)
 #endif
 
@@ -1048,6 +1059,10 @@  size_t __XL_NPP(strftime)(CHAR_T *__restrict s, size_t maxsize,
 #if defined __UCLIBC_HAS_WCHAR__ && (defined L_wcsftime || defined L_wcsftime_l)
 	const CHAR_T *alloc[MAX_PUSH];
 	int allocno = -1;
+	/* A converted format string is read through p for as long as its stack
+	 * level lives, so it cannot be freed at OUTPUT like a converted value.
+	 * One slot per level, freed when the level is popped. */
+	const CHAR_T *fmt_alloc[MAX_PUSH];
 #endif
 	size_t count;
 	size_t o_count;
@@ -1067,6 +1082,14 @@  size_t __XL_NPP(strftime)(CHAR_T *__restrict s, size_t maxsize,
 
 LOOP:
 	if (!count) {
+#if defined __UCLIBC_HAS_WCHAR__ && (defined L_wcsftime || defined L_wcsftime_l)
+		/* Returning from inside an expansion, so the levels are never
+		 * popped and have to be released here. */
+		while (lvl > 0) {
+			if (fmt_alloc[--lvl] != NULL)
+				free((void *)fmt_alloc[lvl]);
+		}
+#endif
 		return 0;
 	}
 	if (!*p) {
@@ -1075,6 +1098,12 @@  LOOP:
 			return maxsize - count;
 		}
 		p = stack[--lvl];
+#if defined __UCLIBC_HAS_WCHAR__ && (defined L_wcsftime || defined L_wcsftime_l)
+		if (fmt_alloc[lvl] != NULL) {
+			free((void *)fmt_alloc[lvl]);
+			fmt_alloc[lvl] = NULL;
+		}
+#endif
 		goto LOOP;
 	}
 
@@ -1120,31 +1149,41 @@  LOOP:
 			if (lvl == MAX_PUSH) {
 				goto OUTPUT;	/* Stack full so treat as illegal spec. */
 			}
-			stack[lvl++] = ++p;
+			stack[lvl] = ++p;
+#if defined __UCLIBC_HAS_WCHAR__ && (defined L_wcsftime || defined L_wcsftime_l)
+			fmt_alloc[lvl] = NULL;
+#endif
+			++lvl;
 			if ((code &= 0xf) < 8) {
 				ccp = (const char *)(spec + STACKED_STRINGS_START + code);
 				ccp += *ccp;
-				fmt_to_wc(p, ccp);
+				stacked_to_wc(p, ccp);
 				goto LOOP;
 			}
 			ccp = (const char *)spec + STACKED_STRINGS_NL_ITEM_START + (code & 7);
-			fmt_to_wc(p, ccp);
+			stacked_to_wc(p, ccp);
 #ifdef ENABLE_ERA_CODE
 			if ((mod & NO_E_MOD) /* Actually, this means E modifier present. */
 				&& (*(ccp = __XL_NPP(nl_langinfo)(_NL_ITEM(LC_TIME,
-							(int)(((unsigned char *)p)[4]))
+							(int)(UCHAR_T)(p[4]))
 							__LOCALE_ARG
 							)))
 				) {
-				fmt_to_wc(p, ccp);
+				stacked_to_wc(p, ccp);
 				goto LOOP;
 			}
 #endif
+			/* The index is the value of the character, not its first
+			 * byte: for wcsftime p is wchar_t*, so reading a byte gives
+			 * the high one on a big-endian target -- always 0, which is
+			 * ABDAY_1, and %c %x %X %r all returned "Sun".  UCHAR_T is
+			 * unsigned char in the narrow build and unsigned int in the
+			 * wide one, which is exactly the widening wanted here. */
 			ccp = __XL_NPP(nl_langinfo)(_NL_ITEM(LC_TIME,
-							(int)(*((unsigned char *)p)))
+							(int)(UCHAR_T)(*p))
 							__LOCALE_ARG
 							);
-			fmt_to_wc(p, ccp);
+			stacked_to_wc(p, ccp);
 			goto LOOP;
 		}