From 7447e2b337f12b4d04935d0f30fc673e4327d5a0 Mon Sep 17 00:00:00 2001
From: Shawn Rutledge <shawn.rutledge@qt.io>
Date: Mon, 24 Feb 2020 16:23:27 +0100
Subject: 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>
---
 src/gui/text/qtextmarkdownimporter.cpp             |  7 +++++--
 src/gui/text/qtextmarkdownimporter_p.h             |  2 +-
 .../text/qtextmarkdownimporter/data/fuzz20450.md   |  5 +++++
 .../text/qtextmarkdownimporter/data/fuzz20580.md   |  1 +
 .../qtextmarkdownimporter.pro                      |  2 ++
 .../tst_qtextmarkdownimporter.cpp                  | 24 ++++++++++++++++++++++
 6 files changed, 38 insertions(+), 3 deletions(-)
 create mode 100644 tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20450.md
 create mode 100644 tests/auto/gui/text/qtextmarkdownimporter/data/fuzz20580.md

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"
-- 
cgit v1.2.3