Bounds-check time-zone offsets when parsing

Parsing of time-zone offsets should check the offset string conforms
to the expected format and has valid values in its fields. The
QDateTime parser, fromOffsetString(), neglected the bounds check on
hours; the QTzTimeZonePrivate parser, parsePosixTime(), neglected all
upper bounds checks, only checking against negative valus.

Drive-by - refined phrasing of a comment.

Fixes: QTBUG-88656
Change-Id: If04cdbe65064108eaa87c42310527783ad21b4c0
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 380d97e1bd15e753907c378a070bdf7f1c1cf06e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Edward Welbourne 2020-11-24 12:45:11 +01:00 committed by Qt Cherry-pick Bot
parent 5938396011
commit a754477b73
2 changed files with 17 additions and 12 deletions

View File

@ -319,7 +319,7 @@ static int fromOffsetString(QStringView offsetString, bool *valid) noexcept
const QStringView hhRef = time.first(qMin(hhLen, time.size())); const QStringView hhRef = time.first(qMin(hhLen, time.size()));
bool ok = false; bool ok = false;
const int hour = hhRef.toInt(&ok); const int hour = hhRef.toInt(&ok);
if (!ok) if (!ok || hour > 23) // More generous than QTimeZone::MaxUtcOffsetSecs
return 0; return 0;
const QStringView mmRef = time.sliced(qMin(mmIndex, time.size())); const QStringView mmRef = time.sliced(qMin(mmIndex, time.size()));

View File

@ -395,29 +395,34 @@ static int parsePosixTime(const char *begin, const char *end)
// Format "hh[:mm[:ss]]" // Format "hh[:mm[:ss]]"
int hour, min = 0, sec = 0; int hour, min = 0, sec = 0;
// Note that the calls to qstrtoll do *not* check the end pointer, which // Note that the calls to qstrtoll do *not* check against the end pointer,
// means they proceed until they find a non-digit. We check that we're // which means they proceed until they find a non-digit. We check that we're
// still in range at the end, but we may have read from past end. It's the // still in range at the end, but we may have read past end. It's the
// caller's responsibility to ensure that begin is part of a // caller's responsibility to ensure that begin is part of a null-terminated
// null-terminated string. // string.
const int maxHour = QTimeZone::MaxUtcOffsetSecs / 3600;
bool ok = false; bool ok = false;
hour = qstrtoll(begin, &begin, 10, &ok); const char *cut = begin;
if (!ok || hour < 0) hour = qstrtoll(begin, &cut, 10, &ok);
if (!ok || hour < 0 || hour > maxHour || cut > begin + 2)
return INT_MIN; return INT_MIN;
begin = cut;
if (begin < end && *begin == ':') { if (begin < end && *begin == ':') {
// minutes // minutes
++begin; ++begin;
min = qstrtoll(begin, &begin, 10, &ok); min = qstrtoll(begin, &cut, 10, &ok);
if (!ok || min < 0) if (!ok || min < 0 || min > 59 || cut > begin + 2)
return INT_MIN; return INT_MIN;
begin = cut;
if (begin < end && *begin == ':') { if (begin < end && *begin == ':') {
// seconds // seconds
++begin; ++begin;
sec = qstrtoll(begin, &begin, 10, &ok); sec = qstrtoll(begin, &cut, 10, &ok);
if (!ok || sec < 0) if (!ok || sec < 0 || sec > 59 || cut > begin + 2)
return INT_MIN; return INT_MIN;
begin = cut;
} }
} }