From 4eca2ac0eef187be01ee1ff32d0109ad45ab7104 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 3 Aug 2018 14:18:48 +0100 Subject: [PATCH 1/5] tests: Add an overflow test for bookmark file dates oss-fuzz#9673 Signed-off-by: Philip Withnall --- glib/tests/Makefile.am | 1 + glib/tests/bookmarks/fail-41.xbel | 1 + 2 files changed, 2 insertions(+) create mode 100644 glib/tests/bookmarks/fail-41.xbel diff --git a/glib/tests/Makefile.am b/glib/tests/Makefile.am index 2087f8332..3a11fd884 100644 --- a/glib/tests/Makefile.am +++ b/glib/tests/Makefile.am @@ -166,6 +166,7 @@ dist_test_data += \ bookmarks/fail-38.xbel \ bookmarks/fail-39.xbel \ bookmarks/fail-40.xbel \ + bookmarks/fail-41.xbel \ bookmarks/valid-01.xbel \ bookmarks/valid-02.xbel \ bookmarks/valid-03.xbel \ diff --git a/glib/tests/bookmarks/fail-41.xbel b/glib/tests/bookmarks/fail-41.xbel new file mode 100644 index 000000000..8ac0c5666 --- /dev/null +++ b/glib/tests/bookmarks/fail-41.xbel @@ -0,0 +1 @@ + From b1fffbffbfe46ba0c15911ea0193bb204bdc6e8b Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 3 Aug 2018 14:19:21 +0100 Subject: [PATCH 2/5] gtimer: Document that g_time_val_from_iso8601() drops whitespace Signed-off-by: Philip Withnall --- glib/gtimer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/glib/gtimer.c b/glib/gtimer.c index e95ac0ead..db2f1b565 100644 --- a/glib/gtimer.c +++ b/glib/gtimer.c @@ -345,6 +345,8 @@ mktime_utc (struct tm *tm) * zone indicator. (In the absence of any time zone indication, the * timestamp is assumed to be in local time.) * + * Any leading or trailing space in @iso_date is ignored. + * * Returns: %TRUE if the conversion was successful. * * Since: 2.12 From cefa66eb760055ed00e9a45c7e91979c5bb1609a Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 3 Aug 2018 14:20:29 +0100 Subject: [PATCH 3/5] gtimer: Add overflow checks to g_time_val_from_iso8601() The code was previously doing a few bits of arithmetic without checking whether they would overflow, and hence not validating the date/time it was parsing very strictly. The parser is now not 100% strict, but should avoid any arithmetic overflows which would cause an invalid result to be returned even though g_time_val_from_iso8601() had returned TRUE. oss-fuzz#9673 Signed-off-by: Philip Withnall --- glib/gtimer.c | 77 ++++++++++++++++++++++++++++++++++++---------- glib/tests/timer.c | 25 ++++++++++++++- 2 files changed, 84 insertions(+), 18 deletions(-) diff --git a/glib/gtimer.c b/glib/gtimer.c index db2f1b565..348b4befb 100644 --- a/glib/gtimer.c +++ b/glib/gtimer.c @@ -357,6 +357,8 @@ g_time_val_from_iso8601 (const gchar *iso_date, { struct tm tm = {0}; long val; + long mday, mon, year; + long hour, min, sec; g_return_val_if_fail (iso_date != NULL, FALSE); g_return_val_if_fail (time_ != NULL, FALSE); @@ -377,23 +379,35 @@ g_time_val_from_iso8601 (const gchar *iso_date, if (*iso_date == '-') { /* YYYY-MM-DD */ - tm.tm_year = val - 1900; + year = val; iso_date++; - tm.tm_mon = strtoul (iso_date, (char **)&iso_date, 10) - 1; - + + mon = strtoul (iso_date, (char **)&iso_date, 10); if (*iso_date++ != '-') return FALSE; - tm.tm_mday = strtoul (iso_date, (char **)&iso_date, 10); + mday = strtoul (iso_date, (char **)&iso_date, 10); } else { /* YYYYMMDD */ - tm.tm_mday = val % 100; - tm.tm_mon = (val % 10000) / 100 - 1; - tm.tm_year = val / 10000 - 1900; + mday = val % 100; + mon = (val % 10000) / 100; + year = val / 10000; } + /* Validation. */ + if (year < 1900 || year > G_MAXINT) + return FALSE; + if (mon < 1 || mon > 12) + return FALSE; + if (mday < 1 || mday > 31) + return FALSE; + + tm.tm_mday = mday; + tm.tm_mon = mon - 1; + tm.tm_year = year - 1900; + if (*iso_date != 'T') return FALSE; @@ -407,34 +421,50 @@ g_time_val_from_iso8601 (const gchar *iso_date, if (*iso_date == ':') { /* hh:mm:ss */ - tm.tm_hour = val; + hour = val; iso_date++; - tm.tm_min = strtoul (iso_date, (char **)&iso_date, 10); + min = strtoul (iso_date, (char **)&iso_date, 10); if (*iso_date++ != ':') return FALSE; - tm.tm_sec = strtoul (iso_date, (char **)&iso_date, 10); + sec = strtoul (iso_date, (char **)&iso_date, 10); } else { /* hhmmss */ - tm.tm_sec = val % 100; - tm.tm_min = (val % 10000) / 100; - tm.tm_hour = val / 10000; + sec = val % 100; + min = (val % 10000) / 100; + hour = val / 10000; } + /* Validation. Allow up to 2 leap seconds when validating @sec. */ + if (hour > 23) + return FALSE; + if (min > 59) + return FALSE; + if (sec > 61) + return FALSE; + + tm.tm_hour = hour; + tm.tm_min = min; + tm.tm_sec = sec; + time_->tv_usec = 0; if (*iso_date == ',' || *iso_date == '.') { glong mul = 100000; - while (g_ascii_isdigit (*++iso_date)) + while (mul >= 1 && g_ascii_isdigit (*++iso_date)) { time_->tv_usec += (*iso_date - '0') * mul; mul /= 10; } + + /* Skip any remaining digits after we’ve reached our limit of precision. */ + while (g_ascii_isdigit (*iso_date)) + iso_date++; } /* Now parse the offset and convert tm to a time_t */ @@ -450,11 +480,24 @@ g_time_val_from_iso8601 (const gchar *iso_date, val = strtoul (iso_date + 1, (char **)&iso_date, 10); if (*iso_date == ':') - val = 60 * val + strtoul (iso_date + 1, (char **)&iso_date, 10); + { + /* hh:mm */ + hour = val; + min = strtoul (iso_date + 1, (char **)&iso_date, 10); + } else - val = 60 * (val / 100) + (val % 100); + { + /* hhmm */ + hour = val / 100; + min = val % 100; + } - time_->tv_sec = mktime_utc (&tm) + (time_t) (60 * val * sign); + if (hour > 99) + return FALSE; + if (min > 59) + return FALSE; + + time_->tv_sec = mktime_utc (&tm) + (time_t) (60 * (60 * hour + min) * sign); } else { diff --git a/glib/tests/timer.c b/glib/tests/timer.c index cb9a2686c..dc08d4c7c 100644 --- a/glib/tests/timer.c +++ b/glib/tests/timer.c @@ -144,7 +144,30 @@ test_timeval_from_iso8601 (void) { FALSE, "2001-10-08Tx", { 0, 0 } }, { FALSE, "2001-10-08T10:11x", { 0, 0 } }, { FALSE, "Wed Dec 19 17:20:20 GMT 2007", { 0, 0 } }, - { FALSE, "1980-02-22T10:36:00Zulu", { 0, 0 } } + { FALSE, "1980-02-22T10:36:00Zulu", { 0, 0 } }, + { FALSE, "2T0+819855292164632335", { 0, 0 } }, + { TRUE, "2018-08-03T14:08:05.446178377+01:00", { 1533301685, 446178 } }, + { FALSE, "2147483648-08-03T14:08:05.446178377+01:00", { 0, 0 } }, + { FALSE, "2018-13-03T14:08:05.446178377+01:00", { 0, 0 } }, + { FALSE, "2018-00-03T14:08:05.446178377+01:00", { 0, 0 } }, + { FALSE, "2018-08-00T14:08:05.446178377+01:00", { 0, 0 } }, + { FALSE, "2018-08-32T14:08:05.446178377+01:00", { 0, 0 } }, + { FALSE, "2018-08-03T24:08:05.446178377+01:00", { 0, 0 } }, + { FALSE, "2018-08-03T14:60:05.446178377+01:00", { 0, 0 } }, + { FALSE, "2018-08-03T14:08:63.446178377+01:00", { 0, 0 } }, + { FALSE, "2018-08-03T14:08:05.446178377+100:00", { 0, 0 } }, + { FALSE, "2018-08-03T14:08:05.446178377+01:60", { 0, 0 } }, + { TRUE, "20180803T140805.446178377+0100", { 1533301685, 446178 } }, + { FALSE, "21474836480803T140805.446178377+0100", { 0, 0 } }, + { FALSE, "20181303T140805.446178377+0100", { 0, 0 } }, + { FALSE, "20180003T140805.446178377+0100", { 0, 0 } }, + { FALSE, "20180800T140805.446178377+0100", { 0, 0 } }, + { FALSE, "20180832T140805.446178377+0100", { 0, 0 } }, + { FALSE, "20180803T240805.446178377+0100", { 0, 0 } }, + { FALSE, "20180803T146005.446178377+0100", { 0, 0 } }, + { FALSE, "20180803T140863.446178377+0100", { 0, 0 } }, + { FALSE, "20180803T140805.446178377+10000", { 0, 0 } }, + { FALSE, "20180803T140805.446178377+0160", { 0, 0 } }, }; GTimeVal out; gboolean success; From fdccf5ff34b062c5b4a55fc10544ff0dc67e853b Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 6 Aug 2018 13:58:06 +0100 Subject: [PATCH 4/5] gtimer: Drop support for negative years from g_time_val_from_iso8601() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It never worked; we’ve always parsed the year with strtoul() (unsigned). While negative years are supported by the ISO 8601 standard, they can only be used by mutual agreement of the two parties interchanging data. Moreover, they are not supported by GTimeVal, which is what we’re filling with the results. Signed-off-by: Philip Withnall --- glib/gtimer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glib/gtimer.c b/glib/gtimer.c index 348b4befb..76da128c8 100644 --- a/glib/gtimer.c +++ b/glib/gtimer.c @@ -372,7 +372,7 @@ g_time_val_from_iso8601 (const gchar *iso_date, if (*iso_date == '\0') return FALSE; - if (!g_ascii_isdigit (*iso_date) && *iso_date != '-' && *iso_date != '+') + if (!g_ascii_isdigit (*iso_date) && *iso_date != '+') return FALSE; val = strtoul (iso_date, (char **)&iso_date, 10); From 2ab7fd2951fdec42c3ef6350c7c862ccdf32c2a1 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 6 Aug 2018 13:59:45 +0100 Subject: [PATCH 5/5] tests: Add more ISO 8601 parser tests These come from looking at the code coverage data. We should now have full branch coverage of the ISO 8601 parser. Signed-off-by: Philip Withnall --- glib/tests/timer.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/glib/tests/timer.c b/glib/tests/timer.c index dc08d4c7c..f40a278b4 100644 --- a/glib/tests/timer.c +++ b/glib/tests/timer.c @@ -168,6 +168,15 @@ test_timeval_from_iso8601 (void) { FALSE, "20180803T140863.446178377+0100", { 0, 0 } }, { FALSE, "20180803T140805.446178377+10000", { 0, 0 } }, { FALSE, "20180803T140805.446178377+0160", { 0, 0 } }, + { TRUE, "+1980-02-22T12:36:00+02:00", { 320063760, 0 } }, + { FALSE, "-0005-01-01T00:00:00Z", { 0, 0 } }, + { FALSE, "2018-08-06", { 0, 0 } }, + { FALSE, "2018-08-06 13:51:00Z", { 0, 0 } }, + { TRUE, "20180803T140805,446178377+0100", { 1533301685, 446178 } }, + { TRUE, "2018-08-03T14:08:05.446178377-01:00", { 1533308885, 446178 } }, + { FALSE, "2018-08-03T14:08:05.446178377 01:00", { 0, 0 } }, + { TRUE, "1990-11-01T10:21:17", { 657454877, 0 } }, + { TRUE, "1990-11-01T10:21:17 ", { 657454877, 0 } }, }; GTimeVal out; gboolean success;