NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

lib/58041: strptime("%s") bug: undefined behaviour in 64-bit; accepts invalid values in 32-bit



>Number:         58041
>Category:       lib
>Synopsis:       strptime("%s") bug: undefined behaviour in 64-bit; accepts invalid values in 32-bit
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Mar 15 19:30:01 +0000 2024
>Originator:     emanuele6 (Emanuele Torre)
>Release:        src/lib/libc/time/strptime.c revision 1.63
>Organization:
jq
>Environment:
The version bundled in jq in builds for targets that don't have strptime
>Description:
The `} while ((sse * 10 <= TIME_MAX) &&' at line 366 of strptime.c is
always true because `sse * 10' is `time_t' (signed), and `TIME_MAX' is
its maximum value.

This makes the code, if the check is not optimised out, trigger a signed
integer overflow (undefined behaviour in standard C) for inputs to %s
that start with a sequence of characters between "922337203685477581"
and "999999999999999999" (inclusive, 64-bit time_t).

Additionally, in builds where there is `#define TIME_MAX INT32_MAX', and
`time_t' is `int32_t', if we assume that signed integer overflow works
like unsigned integer overflow, this makes %s accept, for example,
"9999999999" as a valid timestamp equivalent to
"1410065398" (Sat 20 Nov 17:46:39 UTC) instead of rejecting it.
This is thanks to the fact that UINT32_MAX and INT32_MAX (unlike their
64-bit counterparts) have the same number of digits in decimal, so
`rulim /= 10' can't reject the overflow in time.

>How-To-Repeat:

>Fix:
jq fix ported to src/lib/libc/time/strptime.c revision 1.63

--- strptime.c	2024-03-15 19:56:06.096430188 +0100
+++ strptime-patch.c	2024-03-15 19:57:22.722593741 +0100
@@ -351,7 +351,8 @@
 #endif
 		case 's':	/* seconds since the epoch */
 			{
-				time_t sse = 0;
+				time_t tsse;
+				uint64_t sse = 0;
 				uint64_t rulim = TIME_MAX;
 
 				if (*bp < '0' || *bp > '9') {
@@ -366,12 +367,13 @@
 				} while ((sse * 10 <= TIME_MAX) &&
 					 rulim && *bp >= '0' && *bp <= '9');
 
-				if (sse < 0 || (uint64_t)sse > TIME_MAX) {
+				if (sse > TIME_MAX) {
 					bp = NULL;
 					continue;
 				}
 
-				if (localtime_r(&sse, tm) == NULL)
+				tsse = sse;
+				if (localtime_r(&tsse, tm) == NULL)
 					bp = NULL;
 				else
 					state |= S_YDAY | S_WDAY |



Home | Main Index | Thread Index | Old Index