329 lines
11 KiB
Diff
329 lines
11 KiB
Diff
From fe6af80931c35fafc6a2cd0651b6de052d1bffae Mon Sep 17 00:00:00 2001
|
||
From: Philip Withnall <pwithnall@gnome.org>
|
||
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 <pwithnall@gnome.org>
|
||
---
|
||
glib/gdatetime.c | 12 ++++++++----
|
||
1 file changed, 8 insertions(+), 4 deletions(-)
|
||
|
||
diff --git a/glib/gdatetime.c b/glib/gdatetime.c
|
||
index ad9c190b6b..b33db2c20c 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),
|
||
--
|
||
GitLab
|
||
|
||
|
||
From 495c85278f9638fdf3ebf002c759e1bdccebaf2f Mon Sep 17 00:00:00 2001
|
||
From: Philip Withnall <pwithnall@gnome.org>
|
||
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 <pwithnall@gnome.org>
|
||
---
|
||
glib/gdatetime.c | 8 +++++---
|
||
1 file changed, 5 insertions(+), 3 deletions(-)
|
||
|
||
diff --git a/glib/gdatetime.c b/glib/gdatetime.c
|
||
index b33db2c20c..792c2ed15b 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) */
|
||
--
|
||
GitLab
|
||
|
||
|
||
From 5e8a3c19fcad2936dc5e070cf0767a5c5af907c5 Mon Sep 17 00:00:00 2001
|
||
From: Philip Withnall <pwithnall@gnome.org>
|
||
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 <pwithnall@gnome.org>
|
||
---
|
||
glib/gdatetime.c | 3 ++-
|
||
1 file changed, 2 insertions(+), 1 deletion(-)
|
||
|
||
diff --git a/glib/gdatetime.c b/glib/gdatetime.c
|
||
index 792c2ed15b..6335bcbe2d 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;
|
||
|
||
--
|
||
GitLab
|
||
|
||
|
||
From 804a3957720449dcfac601da96bd5f5db2b71ef1 Mon Sep 17 00:00:00 2001
|
||
From: Philip Withnall <pwithnall@gnome.org>
|
||
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 <pwithnall@gnome.org>
|
||
---
|
||
glib/gdatetime.c | 18 ++++++++++--------
|
||
1 file changed, 10 insertions(+), 8 deletions(-)
|
||
|
||
diff --git a/glib/gdatetime.c b/glib/gdatetime.c
|
||
index 6335bcbe2d..de5dd7af06 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)
|
||
--
|
||
GitLab
|
||
|
||
|
||
From 4c56ff80344e0d8796eb2307091f7b24ec198aa9 Mon Sep 17 00:00:00 2001
|
||
From: Philip Withnall <pwithnall@gnome.org>
|
||
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 <pwithnall@gnome.org>
|
||
---
|
||
glib/gdatetime.c | 13 ++++++-------
|
||
1 file changed, 6 insertions(+), 7 deletions(-)
|
||
|
||
diff --git a/glib/gdatetime.c b/glib/gdatetime.c
|
||
index de5dd7af06..2f8c864a1f 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] == ':')
|
||
--
|
||
GitLab
|
||
|
||
|
||
From 7f6d81130ec05406a8820bc753ed03859e88daea Mon Sep 17 00:00:00 2001
|
||
From: Philip Withnall <pwithnall@gnome.org>
|
||
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 <pwithnall@gnome.org>
|
||
---
|
||
glib/tests/gdatetime.c | 17 +++++++++++++++++
|
||
1 file changed, 17 insertions(+)
|
||
|
||
diff --git a/glib/tests/gdatetime.c b/glib/tests/gdatetime.c
|
||
index 9e1acd097b..94dd028a3a 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 {
|
||
--
|
||
GitLab
|
||
|