Avoid read-outside-array error by QStringRef over-reach

Constructing a QStringRef directly from the string, offset and a
length is UB if the offset + length exceeds the string's length.
Thanks to Robert Loehning and libFuzzer for finding this.
QString::midRef (as correctly used in both changed uses of QStringRef,
since 432d3b6962) takes care of that for us.  Changed one UB case and
a matching but correct case, for consistency.

In the process, deduplicate a QStringList look-up.
Added tests to exercise the code (but the one that exercises the
formerly UB case doesn't crash before the fix, so isn't very useful;
the invalid read is only outside the array it's scanning, not outside
allocated memory).

Change-Id: I7051bbbc0267dd7ec0a8f75eee2034d0b7eb75a2
Reviewed-by: Anton Kudryavtsev <antkudr@mail.ru>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Edward Welbourne 2019-02-07 17:04:49 +01:00
parent b611eb81c8
commit c066656aff
2 changed files with 19 additions and 5 deletions

View File

@ -1125,13 +1125,14 @@ QDateTimeParser::scanString(const QDateTime &defaultValue,
for (int index = 0; index < sectionNodesCount; ++index) { for (int index = 0; index < sectionNodesCount; ++index) {
Q_ASSERT(state != Invalid); Q_ASSERT(state != Invalid);
if (QStringRef(input, pos, separators.at(index).size()) != separators.at(index)) { const QString &separator = separators.at(index);
QDTPDEBUG << "invalid because" << input->midRef(pos, separators.at(index).size()) if (input->midRef(pos, separator.size()) != separator) {
<< "!=" << separators.at(index) QDTPDEBUG << "invalid because" << input->midRef(pos, separator.size())
<< "!=" << separator
<< index << pos << currentSectionIndex; << index << pos << currentSectionIndex;
return StateNode(); return StateNode();
} }
pos += separators.at(index).size(); pos += separator.size();
sectionNodes[index].pos = pos; sectionNodes[index].pos = pos;
int *current = 0; int *current = 0;
const SectionNode sn = sectionNodes.at(index); const SectionNode sn = sectionNodes.at(index);
@ -1227,7 +1228,7 @@ QDateTimeParser::scanString(const QDateTime &defaultValue,
isSet |= sn.type; isSet |= sn.type;
} }
if (QStringRef(input, pos, input->size() - pos) != separators.last()) { if (input->midRef(pos) != separators.last()) {
QDTPDEBUG << "invalid because" << input->midRef(pos) QDTPDEBUG << "invalid because" << input->midRef(pos)
<< "!=" << separators.last() << pos; << "!=" << separators.last() << pos;
return StateNode(); return StateNode();

View File

@ -2401,6 +2401,19 @@ void tst_QDateTime::fromStringStringFormat_data()
QTest::newRow("late") << QString("9999-12-31T23:59:59.999Z") QTest::newRow("late") << QString("9999-12-31T23:59:59.999Z")
<< QString("yyyy-MM-ddThh:mm:ss.zZ") << QString("yyyy-MM-ddThh:mm:ss.zZ")
<< QDateTime(QDate(9999, 12, 31), QTime(23, 59, 59, 999)); << QDateTime(QDate(9999, 12, 31), QTime(23, 59, 59, 999));
// Separators match /([^aAdhHMmstyz]*)/
QTest::newRow("oddly-separated") // To show broken-separator's format is valid.
<< QStringLiteral("2018 wilful long working block relief 12-19T21:09 cruel blurb encore flux")
<< QStringLiteral("yyyy wilful long working block relief MM-ddThh:mm cruel blurb encore flux")
<< QDateTime(QDate(2018, 12, 19), QTime(21, 9));
QTest::newRow("broken-separator")
<< QStringLiteral("2018 wilful")
<< QStringLiteral("yyyy wilful long working block relief MM-ddThh:mm cruel blurb encore flux")
<< invalidDateTime();
QTest::newRow("broken-terminator")
<< QStringLiteral("2018 wilful long working block relief 12-19T21:09 cruel")
<< QStringLiteral("yyyy wilful long working block relief MM-ddThh:mm cruel blurb encore flux")
<< invalidDateTime();
} }
void tst_QDateTime::fromStringStringFormat() void tst_QDateTime::fromStringStringFormat()