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 a0277f612b82f2bcf9db5a83b48db225413e85d7..ac8752c43e14e7db2f091a1c37c95098005760e5 100644 (file)
--- 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);
 }