Skip to content

Commit

Permalink
Make POSIX transition rule parser more robust
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Thiago Macieira <[email protected]>
Reviewed-by: Robert Löhning <[email protected]>
(cherry picked from commit 964f91f)
Reviewed-by: Qt Cherry-pick Bot <[email protected]>
  • Loading branch information
ediosyncratic authored and Qt Cherry-pick Bot committed Apr 14, 2021
1 parent 6a4b3e8 commit fe0da2b
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 14 deletions.
43 changes: 29 additions & 14 deletions src/corelib/time/qtimezoneprivate_tz.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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<QByteArray> 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
Expand Down Expand Up @@ -576,7 +587,8 @@ static QList<QTimeZonePrivate::Data> 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<QByteArray> dstParts = parts.at(1).split('/');
Expand All @@ -596,6 +608,9 @@ static QList<QTimeZonePrivate::Data> 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);
Expand Down
16 changes: 16 additions & 0 deletions tests/auto/corelib/time/qtimezone/tst_qtimezone.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ private Q_SLOTS:
void isValidId_data();
void isValidId();
void serialize();
void malformed();
// Backend tests
void utcTest();
void icuTest();
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit fe0da2b

Please sign in to comment.