From 2c2cc9ed808ba4ff8cc2595e098c754e22862db7 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Tue, 13 Apr 2021 14:22:00 +0200 Subject: Make POSIX transition rule parser more robust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The POSIX rule parser used by QTzTimeZonePrivate recklessly assumed that, if splitting the rule on a dot produced more than one part, it necessarily produced at least three. That's true for well-formed POSIX rules, but we should catch the case of malformed rules. Likewise, when calculating the dates of transitions, splitting the date rule on dots might produce too few fragments; and the fragments might not parse as valid numbers, or might be out of range for their respective fields in a date. Check all these cases, too. Added a test that crashed previously. Changed QTimeZone::offsetFromUtc() so that its "return zero on invalid" applies also to the case where the backend returns invalid, in support of this. Fixes: QTBUG-92808 Change-Id: Ica383a7a987465483341bdef8dcfd42edb6b43d6 Reviewed-by: Andrei Golubev Reviewed-by: Thiago Macieira Reviewed-by: Robert Löhning (cherry picked from commit 964f91fd25a59654905c5a68d3cbccedab9ebb5a) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/time/qtimezoneprivate_tz.cpp | 43 +++++++++++++++------- .../auto/corelib/time/qtimezone/tst_qtimezone.cpp | 16 ++++++++ 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/corelib/time/qtimezoneprivate_tz.cpp b/src/corelib/time/qtimezoneprivate_tz.cpp index a18184b91e..65a28c749f 100644 --- a/src/corelib/time/qtimezoneprivate_tz.cpp +++ b/src/corelib/time/qtimezoneprivate_tz.cpp @@ -351,6 +351,11 @@ static QByteArray parseTzPosixRule(QDataStream &ds) static QDate calculateDowDate(int year, int month, int dayOfWeek, int week) { + if (dayOfWeek == 0) // Sunday; we represent it as 7, POSIX uses 0 + dayOfWeek = 7; + else if (dayOfWeek & ~7 || month < 1 || month > 12 || week < 1 || week > 5) + return QDate(); + QDate date(year, month, 1); int startDow = date.dayOfWeek(); if (startDow <= dayOfWeek) @@ -365,28 +370,34 @@ static QDate calculateDowDate(int year, int month, int dayOfWeek, int week) static QDate calculatePosixDate(const QByteArray &dateRule, int year) { + bool ok; // Can start with M, J, or a digit if (dateRule.at(0) == 'M') { // nth week in month format "Mmonth.week.dow" QList dateParts = dateRule.split('.'); - int month = dateParts.at(0).mid(1).toInt(); - int week = dateParts.at(1).toInt(); - int dow = dateParts.at(2).toInt(); - if (dow == 0) // Sunday; we represent it as 7 - dow = 7; - return calculateDowDate(year, month, dow, week); + if (dateParts.count() > 2) { + int month = dateParts.at(0).mid(1).toInt(&ok); + int week = ok ? dateParts.at(1).toInt(&ok) : 0; + int dow = ok ? dateParts.at(2).toInt(&ok) : 0; + if (ok) + return calculateDowDate(year, month, dow, week); + } } else if (dateRule.at(0) == 'J') { // Day of Year ignores Feb 29 - int doy = dateRule.mid(1).toInt(); - QDate date = QDate(year, 1, 1).addDays(doy - 1); - if (QDate::isLeapYear(date.year())) - date = date.addDays(-1); - return date; + int doy = dateRule.mid(1).toInt(&ok); + if (ok && doy > 0 && doy < 366) { + QDate date = QDate(year, 1, 1).addDays(doy - 1); + if (QDate::isLeapYear(date.year()) && date.month() > 2) + date = date.addDays(-1); + return date; + } } else { // Day of Year includes Feb 29 - int doy = dateRule.toInt(); - return QDate(year, 1, 1).addDays(doy - 1); + int doy = dateRule.toInt(&ok); + if (ok && doy > 0 && doy <= 366) + return QDate(year, 1, 1).addDays(doy - 1); } + return QDate(); } // returns the time in seconds, INT_MIN if we failed to parse @@ -576,7 +587,8 @@ static QList calculatePosixTransitions(const QByteArray result << data; return result; } - + if (parts.count() < 3 || parts.at(1).isEmpty() || parts.at(2).isEmpty()) + return result; // Malformed. // Get the std to dst transtion details QList dstParts = parts.at(1).split('/'); @@ -596,6 +608,9 @@ static QList calculatePosixTransitions(const QByteArray else stdTime = QTime(2, 0, 0); + if (dstDateRule.isEmpty() || stdDateRule.isEmpty() || !dstTime.isValid() || !stdTime.isValid()) + return result; // Malformed. + // Limit year to the range QDateTime can represent: const int minYear = int(QDateTime::YearRange::First); const int maxYear = int(QDateTime::YearRange::Last); diff --git a/tests/auto/corelib/time/qtimezone/tst_qtimezone.cpp b/tests/auto/corelib/time/qtimezone/tst_qtimezone.cpp index 1d09a84477..0c9afc7166 100644 --- a/tests/auto/corelib/time/qtimezone/tst_qtimezone.cpp +++ b/tests/auto/corelib/time/qtimezone/tst_qtimezone.cpp @@ -67,6 +67,7 @@ private Q_SLOTS: void isValidId_data(); void isValidId(); void serialize(); + void malformed(); // Backend tests void utcTest(); void icuTest(); @@ -961,6 +962,21 @@ void tst_QTimeZone::serialize() QSKIP("No serialization enabled"); } +void tst_QTimeZone::malformed() +{ + // Regression test for QTBUG-92808 + // Strings that look enough like a POSIX zone specifier that the constructor + // accepts them, but the specifier is invalid. + // Must not crash or trigger assertions when calling offsetFromUtc() + const QDateTime now = QDateTime::currentDateTime(); + QTimeZone barf("QUT4tCZ0 , /"); + if (barf.isValid()) + barf.offsetFromUtc(now); + barf = QTimeZone("QtC+09,,MA"); + if (barf.isValid()) + barf.offsetFromUtc(now); +} + void tst_QTimeZone::utcTest() { #ifdef QT_BUILD_INTERNAL -- cgit v1.2.3