summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShawn Rutledge <shawn.rutledge@qt.io>2020-02-24 16:23:27 +0100
committerShawn Rutledge <shawn.rutledge@qt.io>2020-02-28 09:31:59 +0100
commit7447e2b337f12b4d04935d0f30fc673e4327d5a0 (patch)
treee6dfaac556c0e2ccb745bd9d13145b2ed3690f1d
parenteaf7f572bfbcb33b106097923f4e0efdd9c683fc (diff)
QTextMarkdownImporter: fix use after free; add fuzz-generated tests
It was possible to end up with a dangling pointer in m_listStack. This is now avoided by using QPointer and doing nullptr checks before accessing any QTextList pointer stored there. We have 2 specimens of garbage that caused crashes before; now they don't. But only fuzz20450 triggered the dangling pointer in the list stack. The crash caused by fuzz20580 was fixed by updating md4c from upstream: 4b0fc030777cd541604f5ebaaad47a2b76d61ff9 Change-Id: I8e1eca23b281256a03aea0f55e9ae20f1bdd2a38 Reviewed-by: Robert Loehning <robert.loehning@qt.io>
-rw-r--r--src/gui/text/qtextmarkdownimporter.cpp7
-rw-r--r--src/gui/text/qtextmarkdownimporter_p.h2
-rw-r--r--tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20450.md5
-rw-r--r--tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20580.md1
-rw-r--r--tests/auto/gui/text/qtextmarkdownimporter/qtextmarkdownimporter.pro2
-rw-r--r--tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp24
6 files changed, 38 insertions, 3 deletions
diff --git a/src/gui/text/qtextmarkdownimporter.cpp b/src/gui/text/qtextmarkdownimporter.cpp
index 7e18a10895..5e75e7816b 100644
--- a/src/gui/text/qtextmarkdownimporter.cpp
+++ b/src/gui/text/qtextmarkdownimporter.cpp
@@ -577,7 +577,10 @@ void QTextMarkdownImporter::insertBlock()
QTextBlockFormat blockFormat;
if (!m_listStack.isEmpty() && !m_needsInsertList && m_listItem) {
QTextList *list = m_listStack.top();
- blockFormat = list->item(list->count() - 1).blockFormat();
+ if (list)
+ blockFormat = list->item(list->count() - 1).blockFormat();
+ else
+ qWarning() << "attempted to insert into a list that no longer exists";
}
if (m_blockQuoteDepth) {
blockFormat.setProperty(QTextFormat::BlockQuoteLevel, m_blockQuoteDepth);
@@ -607,7 +610,7 @@ void QTextMarkdownImporter::insertBlock()
}
if (m_needsInsertList) {
m_listStack.push(m_cursor->createList(m_listFormat));
- } else if (!m_listStack.isEmpty() && m_listItem) {
+ } else if (!m_listStack.isEmpty() && m_listItem && m_listStack.top()) {
m_listStack.top()->add(m_cursor->block());
}
m_needsInsertList = false;
diff --git a/src/gui/text/qtextmarkdownimporter_p.h b/src/gui/text/qtextmarkdownimporter_p.h
index f450da5eb3..e3b4bcd0f2 100644
--- a/src/gui/text/qtextmarkdownimporter_p.h
+++ b/src/gui/text/qtextmarkdownimporter_p.h
@@ -113,7 +113,7 @@ private:
#endif
QString m_blockCodeLanguage;
QVector<int> m_nonEmptyTableCells; // in the current row
- QStack<QTextList *> m_listStack;
+ QStack<QPointer<QTextList>> m_listStack;
QStack<QTextCharFormat> m_spanFormatStack;
QFont m_monoFont;
QPalette m_palette;
diff --git a/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20450.md b/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20450.md
new file mode 100644
index 0000000000..d7005cb01e
--- /dev/null
+++ b/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20450.md
@@ -0,0 +1,5 @@
+<t>ÿ
+* ÿ
+
+ ÿ
+* ÿ \ No newline at end of file
diff --git a/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20580.md b/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20580.md
new file mode 100644
index 0000000000..22006f5876
--- /dev/null
+++ b/tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20580.md
@@ -0,0 +1 @@
+| --:| <?`?><?|` \ No newline at end of file
diff --git a/tests/auto/gui/text/qtextmarkdownimporter/qtextmarkdownimporter.pro b/tests/auto/gui/text/qtextmarkdownimporter/qtextmarkdownimporter.pro
index 7b7fb61244..f3818efbf7 100644
--- a/tests/auto/gui/text/qtextmarkdownimporter/qtextmarkdownimporter.pro
+++ b/tests/auto/gui/text/qtextmarkdownimporter/qtextmarkdownimporter.pro
@@ -5,5 +5,7 @@ SOURCES += tst_qtextmarkdownimporter.cpp
TESTDATA += \
data/thematicBreaks.md \
data/headingBulletsContinuations.md \
+ data/fuzz20450.md \
+ data/fuzz20580.md \
DEFINES += SRCDIR=\\\"$$PWD\\\"
diff --git a/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp b/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp
index 39a1370f6f..5eb04af696 100644
--- a/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp
+++ b/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp
@@ -57,6 +57,8 @@ private slots:
void lists();
void avoidBlankLineAtBeginning_data();
void avoidBlankLineAtBeginning();
+ void pathological_data();
+ void pathological();
};
void tst_QTextMarkdownImporter::headingBulletsContinuations()
@@ -256,5 +258,27 @@ void tst_QTextMarkdownImporter::avoidBlankLineAtBeginning() // QTBUG-81060
QCOMPARE(i, expectedNumberOfParagraphs);
}
+void tst_QTextMarkdownImporter::pathological_data()
+{
+ QTest::addColumn<QString>("warning");
+ QTest::newRow("fuzz20450") << "attempted to insert into a list that no longer exists";
+ QTest::newRow("fuzz20580") << "";
+}
+
+void tst_QTextMarkdownImporter::pathological() // avoid crashing on crazy input
+{
+ QFETCH(QString, warning);
+ QString filename = QLatin1String("data/") + QTest::currentDataTag() + QLatin1String(".md");
+ QFile f(QFINDTESTDATA(filename));
+ QVERIFY(f.open(QFile::ReadOnly));
+#ifdef QT_NO_DEBUG
+ Q_UNUSED(warning)
+#else
+ if (!warning.isEmpty())
+ QTest::ignoreMessage(QtWarningMsg, warning.toLatin1());
+#endif
+ QTextDocument().setMarkdown(f.readAll());
+}
+
QTEST_MAIN(tst_QTextMarkdownImporter)
#include "tst_qtextmarkdownimporter.moc"