From fe6af80931c35fafc6a2cd0651b6de052d1bffae Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 18 Feb 2025 16:44:58 +0000 Subject: [PATCH 1/6] gdatetime: Fix integer overflow when parsing very long ISO8601 inputs This will only happen with invalid (or maliciously invalid) potential ISO8601 strings, but `g_date_time_new_from_iso8601()` needs to be robust against that. Prevent `length` overflowing by correctly defining it as a `size_t`. Similarly for `date_length`, but additionally track its validity in a boolean rather than as its sign. Spotted by chamalsl as #YWH-PGM9867-43. Signed-off-by: Philip Withnall --- glib/gdatetime.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/glib/gdatetime.c b/glib/gdatetime.c index ad9c190b6..b33db2c20 100644 --- a/glib/gdatetime.c +++ b/glib/gdatetime.c @@ -1544,7 +1544,8 @@ parse_iso8601_time (const gchar *text, gsize length, GDateTime * g_date_time_new_from_iso8601 (const gchar *text, GTimeZone *default_tz) { - gint length, date_length = -1; + size_t length, date_length = 0; + gboolean date_length_set = FALSE; gint hour = 0, minute = 0; gdouble seconds = 0.0; GTimeZone *tz = NULL; @@ -1555,11 +1556,14 @@ g_date_time_new_from_iso8601 (const gchar *text, GTimeZone *default_tz) /* Count length of string and find date / time separator ('T', 't', or ' ') */ for (length = 0; text[length] != '\0'; length++) { - if (date_length < 0 && (text[length] == 'T' || text[length] == 't' || text[length] == ' ')) - date_length = length; + if (!date_length_set && (text[length] == 'T' || text[length] == 't' || text[length] == ' ')) + { + date_length = length; + date_length_set = TRUE; + } } - if (date_length < 0) + if (!date_length_set) return NULL; if (!parse_iso8601_time (text + date_length + 1, length - (date_length + 1), From 495c85278f9638fdf3ebf002c759e1bdccebaf2f Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 18 Feb 2025 16:51:36 +0000 Subject: [PATCH 2/6] gdatetime: Fix potential integer overflow in timezone offset handling This one is much harder to trigger than the one in the previous commit, but mixing `gssize` and `gsize` always runs the risk of the former overflowing for very (very very) long input strings. Avoid that possibility by not using the sign of the `tz_offset` to indicate its validity, and instead using the return value of the function. Signed-off-by: Philip Withnall --- glib/gdatetime.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/glib/gdatetime.c b/glib/gdatetime.c index b33db2c20..792c2ed15 100644 --- a/glib/gdatetime.c +++ b/glib/gdatetime.c @@ -1393,8 +1393,10 @@ parse_iso8601_date (const gchar *text, gsize length, return FALSE; } +/* Value returned in tz_offset is valid if and only if the function return value + * is non-NULL. */ static GTimeZone * -parse_iso8601_timezone (const gchar *text, gsize length, gssize *tz_offset) +parse_iso8601_timezone (const gchar *text, gsize length, size_t *tz_offset) { gint i, tz_length, offset_hours, offset_minutes; gint offset_sign = 1; @@ -1462,11 +1464,11 @@ static gboolean parse_iso8601_time (const gchar *text, gsize length, gint *hour, gint *minute, gdouble *seconds, GTimeZone **tz) { - gssize tz_offset = -1; + size_t tz_offset = 0; /* Check for timezone suffix */ *tz = parse_iso8601_timezone (text, length, &tz_offset); - if (tz_offset >= 0) + if (*tz != NULL) length = tz_offset; /* hh:mm:ss(.sss) */ From 5e8a3c19fcad2936dc5e070cf0767a5c5af907c5 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 18 Feb 2025 16:55:18 +0000 Subject: [PATCH 3/6] gdatetime: Track timezone length as an unsigned size_t MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s guaranteed to be in (0, length] by the calculations above. This avoids the possibility of integer overflow through `gssize` not being as big as `size_t`. Signed-off-by: Philip Withnall --- glib/gdatetime.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/glib/gdatetime.c b/glib/gdatetime.c index 792c2ed15..6335bcbe2 100644 --- a/glib/gdatetime.c +++ b/glib/gdatetime.c @@ -1398,7 +1398,8 @@ parse_iso8601_date (const gchar *text, gsize length, static GTimeZone * parse_iso8601_timezone (const gchar *text, gsize length, size_t *tz_offset) { - gint i, tz_length, offset_hours, offset_minutes; + size_t tz_length; + gint i, offset_hours, offset_minutes; gint offset_sign = 1; GTimeZone *tz; From 804a3957720449dcfac601da96bd5f5db2b71ef1 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 18 Feb 2025 17:07:24 +0000 Subject: [PATCH 4/6] gdatetime: Factor out some string pointer arithmetic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes the following code a little clearer, but doesn’t introduce any functional changes. Signed-off-by: Philip Withnall --- glib/gdatetime.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/glib/gdatetime.c b/glib/gdatetime.c index 6335bcbe2..de5dd7af0 100644 --- a/glib/gdatetime.c +++ b/glib/gdatetime.c @@ -1402,6 +1402,7 @@ parse_iso8601_timezone (const gchar *text, gsize length, size_t *tz_offset) gint i, offset_hours, offset_minutes; gint offset_sign = 1; GTimeZone *tz; + const char *tz_start; /* UTC uses Z suffix */ if (length > 0 && text[length - 1] == 'Z') @@ -1419,34 +1420,35 @@ parse_iso8601_timezone (const gchar *text, gsize length, size_t *tz_offset) } if (i < 0) return NULL; + tz_start = text + i; tz_length = length - i; /* +hh:mm or -hh:mm */ - if (tz_length == 6 && text[i+3] == ':') + if (tz_length == 6 && tz_start[3] == ':') { - if (!get_iso8601_int (text + i + 1, 2, &offset_hours) || - !get_iso8601_int (text + i + 4, 2, &offset_minutes)) + if (!get_iso8601_int (tz_start + 1, 2, &offset_hours) || + !get_iso8601_int (tz_start + 4, 2, &offset_minutes)) return NULL; } /* +hhmm or -hhmm */ else if (tz_length == 5) { - if (!get_iso8601_int (text + i + 1, 2, &offset_hours) || - !get_iso8601_int (text + i + 3, 2, &offset_minutes)) + if (!get_iso8601_int (tz_start + 1, 2, &offset_hours) || + !get_iso8601_int (tz_start + 3, 2, &offset_minutes)) return NULL; } /* +hh or -hh */ else if (tz_length == 3) { - if (!get_iso8601_int (text + i + 1, 2, &offset_hours)) + if (!get_iso8601_int (tz_start + 1, 2, &offset_hours)) return NULL; offset_minutes = 0; } else return NULL; - *tz_offset = i; - tz = g_time_zone_new_identifier (text + i); + *tz_offset = tz_start - text; + tz = g_time_zone_new_identifier (tz_start); /* Double-check that the GTimeZone matches our interpretation of the timezone. * This can fail because our interpretation is less strict than (for example) From 4c56ff80344e0d8796eb2307091f7b24ec198aa9 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 18 Feb 2025 17:28:33 +0000 Subject: [PATCH 5/6] gdatetime: Factor out an undersized variable For long input strings, it would have been possible for `i` to overflow. Avoid that problem by using the `tz_length` instead, so that we count up rather than down. This commit introduces no functional changes (outside of changing undefined behaviour), and can be verified using the identity `i === length - tz_length`. Signed-off-by: Philip Withnall --- glib/gdatetime.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/glib/gdatetime.c b/glib/gdatetime.c index de5dd7af0..2f8c864a1 100644 --- a/glib/gdatetime.c +++ b/glib/gdatetime.c @@ -1399,7 +1399,7 @@ static GTimeZone * parse_iso8601_timezone (const gchar *text, gsize length, size_t *tz_offset) { size_t tz_length; - gint i, offset_hours, offset_minutes; + gint offset_hours, offset_minutes; gint offset_sign = 1; GTimeZone *tz; const char *tz_start; @@ -1412,16 +1412,15 @@ parse_iso8601_timezone (const gchar *text, gsize length, size_t *tz_offset) } /* Look for '+' or '-' of offset */ - for (i = length - 1; i >= 0; i--) - if (text[i] == '+' || text[i] == '-') + for (tz_length = 1; tz_length <= length; tz_length++) + if (text[length - tz_length] == '+' || text[length - tz_length] == '-') { - offset_sign = text[i] == '-' ? -1 : 1; + offset_sign = text[length - tz_length] == '-' ? -1 : 1; break; } - if (i < 0) + if (tz_length > length) return NULL; - tz_start = text + i; - tz_length = length - i; + tz_start = text + length - tz_length; /* +hh:mm or -hh:mm */ if (tz_length == 6 && tz_start[3] == ':') From 7f6d81130ec05406a8820bc753ed03859e88daea Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 18 Feb 2025 18:20:56 +0000 Subject: [PATCH 6/6] tests: Add some missing GDateTime ISO8601 parsing tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This improves test coverage, adding coverage for some lines which I spotted were not covered while testing the preceding commits. It doesn’t directly test the preceding commits, though. Signed-off-by: Philip Withnall --- glib/tests/gdatetime.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/glib/tests/gdatetime.c b/glib/tests/gdatetime.c index 9e1acd097..94dd028a3 100644 --- a/glib/tests/gdatetime.c +++ b/glib/tests/gdatetime.c @@ -866,6 +866,23 @@ test_GDateTime_new_from_iso8601 (void) * NaN */ dt = g_date_time_new_from_iso8601 ("0005306 000001,666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666666600080000-00", NULL); g_assert_null (dt); + + /* Various invalid timezone offsets which look like they could be in + * `+hh:mm`, `-hh:mm`, `+hhmm`, `-hhmm`, `+hh` or `-hh` format */ + dt = g_date_time_new_from_iso8601 ("2025-02-18T18:14:00+01:xx", NULL); + g_assert_null (dt); + dt = g_date_time_new_from_iso8601 ("2025-02-18T18:14:00+xx:00", NULL); + g_assert_null (dt); + dt = g_date_time_new_from_iso8601 ("2025-02-18T18:14:00+xx:xx", NULL); + g_assert_null (dt); + dt = g_date_time_new_from_iso8601 ("2025-02-18T18:14:00+01xx", NULL); + g_assert_null (dt); + dt = g_date_time_new_from_iso8601 ("2025-02-18T18:14:00+xx00", NULL); + g_assert_null (dt); + dt = g_date_time_new_from_iso8601 ("2025-02-18T18:14:00+xxxx", NULL); + g_assert_null (dt); + dt = g_date_time_new_from_iso8601 ("2025-02-18T18:14:00+xx", NULL); + g_assert_null (dt); } typedef struct {