From: Alan T. DeKok <aland@freeradius.org>
Date: Sun, 24 May 2026 13:02:21 +0000 (-0400)
Subject: be more precise for time parsing
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1ecb05f7ea70ee747b5c5f421f53dde97cc4ca80;p=thirdparty%2Ffreeradius-server.git

be more precise for time parsing

* if there's a '.' but no subseconds, that's an error
* if there are subseconds, check that there's a trailing 'Z'
---

diff --git a/src/protocols/der/decode.c b/src/protocols/der/decode.c
index a0277f612b8..ac8752c43e1 100644
--- a/src/protocols/der/decode.c
+++ b/src/protocols/der/decode.c
@@ -1217,6 +1217,10 @@ static ssize_t fr_der_decode_utc_time(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_d
 	return fr_dbuff_set(in, &our_in);
 }
 
+static bool const sbuff_char_class_num[SBUFF_CHAR_CLASS] = {
+	SBUFF_CHAR_CLASS_NUM,
+};
+
 static ssize_t fr_der_decode_generalized_time(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent,
 					      fr_dbuff_t *in, UNUSED fr_der_decode_ctx_t *decode_ctx)
 {
@@ -1287,34 +1291,72 @@ static ssize_t fr_der_decode_generalized_time(TALLOC_CTX *ctx, fr_pair_list_t *o
 	}
 
 	/*
-	 *	Check if the fractional seconds are present
+	 *	Check if the fractional seconds are present.
 	 */
 	if (timestr[DER_GENERALIZED_TIME_LEN_MIN - 1] == '.') {
+		size_t sublen;
+
 		/*
-		 *	We only support subseconds up to 9 decimal places (nanoseconds)
+		 *	We only support subseconds up to 9 decimal places (nanoseconds).
 		 */
 		char subsecstring[DER_GENERALIZED_TIME_PRECISION_MAX + 1];
 
 		uint8_t precision = DER_GENERALIZED_TIME_PRECISION_MAX;
 
-		if (unlikely(fr_dbuff_remaining(&our_in) - 1 < DER_GENERALIZED_TIME_PRECISION_MAX)) {
-			precision = fr_dbuff_remaining(&our_in) - 1;
-		}
-
-		if (unlikely(precision == 0)) {
+		/*
+		 *	"." is invalid, as is ".Z", or even ".0"
+		 */
+		sublen = fr_dbuff_remaining(&our_in);
+		if (sublen <= 1) {
+		insufficient_data:
 			fr_strerror_const_push("Insufficient data for subseconds");
 			return -1;
 		}
 
-		FR_DBUFF_OUT_MEMCPY_RETURN((uint8_t *)subsecstring, &our_in, precision);
+		/*
+		 *	Ensure that the remaining characters are all decimal numbers.
+		 */
+		sublen = fr_sbuff_adv_past_allowed(&FR_SBUFF_IN((char const *) fr_dbuff_current(&our_in), sublen),
+						   SIZE_MAX, sbuff_char_class_num, NULL);
+		if (sublen == 0) goto insufficient_data;
 
-		if (memchr(subsecstring, '\0', precision) != NULL) {
-			fr_strerror_const_push("Generalized time contains null byte in subseconds");
-			return -1;
-		}
+		/*
+		 *	Limit precision to either what's there, or the maximum that we care about.
+		 */
+		precision = (sublen <= DER_GENERALIZED_TIME_PRECISION_MAX ?
+			     sublen : DER_GENERALIZED_TIME_PRECISION_MAX);
+
+		FR_DBUFF_OUT_MEMCPY_RETURN((uint8_t *)subsecstring, &our_in, precision);
 
 		subsecstring[precision] = '\0';
 
+		/*
+		 *	Skip the numbers, and see if we have a trailing 'Z'.
+		 */
+		if (precision < sublen) (void) fr_dbuff_advance(&our_in, sublen - precision);
+
+		sublen = fr_dbuff_remaining(&our_in);
+
+		/*
+		 *	Time zone can be missing.
+		 */
+		if (sublen > 0) {
+			FR_DBUFF_OUT_MEMCPY_RETURN((uint8_t *)subsecstring, &our_in, 1);
+
+			/*
+			 *	This is a special case for error messages.
+			 */
+			if (!subsecstring[0]) {
+				fr_strerror_const_push("Generalized time contains null byte in subseconds");
+				return -1;
+			}
+
+			if ((sublen > 1) || (subsecstring[0] != 'Z')) {
+				fr_strerror_const_push("Generalized time contains invalid time zone");
+				return -1;
+			}
+		}
+
 		/*
 		 *	Convert the subseconds to an unsigned long
 		 */
@@ -1337,7 +1379,7 @@ static ssize_t fr_der_decode_generalized_time(TALLOC_CTX *ctx, fr_pair_list_t *o
 			};
 			subseconds *= nsec_multiplier[precision];
 		}
-	}
+	} /* else the trailing character is 'Z' */
 
 	/*
 	 *	Make sure the timezone is UTC (Z)
@@ -1363,12 +1405,6 @@ static ssize_t fr_der_decode_generalized_time(TALLOC_CTX *ctx, fr_pair_list_t *o
 
 	fr_pair_append(out, vp);
 
-	/*
-	 *	Move to the end of the buffer
-	 *	This is necessary because the fractional seconds are being ignored
-	 */
-	fr_dbuff_advance(&our_in, fr_dbuff_remaining(&our_in));
-
 	return fr_dbuff_set(in, &our_in);
 }