From f432c08882ffebe5074ea28de871559a98a4d094 Mon Sep 17 00:00:00 2001

From: Lars Knoll <lars.knoll@qt.io>

Date: Wed, 26 Feb 2020 10:42:10 +0100

Subject: Add an expansion limit for entities

Recursively defined entities can easily exhaust all available

memory. Limit entity expansion to a default of 4096 characters to

avoid DoS attacks when a user loads untrusted content.

[ChangeLog][QtCore][QXmlStream] QXmlStreamReader does now

limit the expansion of entities to 4096 characters. Documents where

a single entity expands to more characters than the limit are not

considered well formed. The limit is there to avoid DoS attacks through

recursively expanding entities when loading untrusted content. Qt 5.15

will add methods that allow changing that limit.

Fixes: QTBUG-47417

Change-Id: I94387815d74fcf34783e136387ee57fac5ded0c9

Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

(cherry picked from commit fd4be84d23a0db4186cb42e736a9de3af722c7f7)

Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>

---

 src/corelib/serialization/qxmlstream.g             | 14 ++++++++++++-

 src/corelib/serialization/qxmlstream_p.h           | 14 ++++++++++++-

 .../serialization/qxmlstream/tst_qxmlstream.cpp    | 23 ++++++++++++++++++++--

 3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/src/corelib/serialization/qxmlstream.g b/src/corelib/serialization/qxmlstream.g

index 10bfcd491c..5726bafb26 100644

--- a/src/corelib/serialization/qxmlstream.g

+++ b/src/corelib/serialization/qxmlstream.g

@@ -277,9 +277,19 @@ public:

     QHash<QStringView, Entity> entityHash;

     QHash<QStringView, Entity> parameterEntityHash;

     QXmlStreamSimpleStack<Entity *>entityReferenceStack;

+    int entityExpansionLimit = 4096;

+    int entityLength = 0;

     inline bool referenceEntity(Entity &entity) {

         if (entity.isCurrentlyReferenced) {

-            raiseWellFormedError(QXmlStream::tr("Recursive entity detected."));

+            raiseWellFormedError(QXmlStream::tr("Self-referencing entity detected."));

+            return false;

+        }

+        // entityLength represents the amount of additional characters the

+        // entity expands into (can be negative for e.g. &). It's used to

+        // avoid DoS attacks through recursive entity expansions

+        entityLength += entity.value.size() - entity.name.size() - 2;

+        if (entityLength > entityExpansionLimit) {

+            raiseWellFormedError(QXmlStream::tr("Entity expands to more characters than the entity expansion limit."));

             return false;

         }

         entity.isCurrentlyReferenced = true;

@@ -830,6 +840,8 @@ entity_done ::= ENTITY_DONE;

 /.

         case $rule_number:

             entityReferenceStack.pop()->isCurrentlyReferenced = false;

+            if (entityReferenceStack.isEmpty())

+                entityLength = 0;

             clearSym();

         break;

 ./

diff --git a/src/corelib/serialization/qxmlstream_p.h b/src/corelib/serialization/qxmlstream_p.h

index 61f501f81b..31053f8e0b 100644

--- a/src/corelib/serialization/qxmlstream_p.h

+++ b/src/corelib/serialization/qxmlstream_p.h

@@ -774,9 +774,19 @@ public:

     QHash<QStringView, Entity> entityHash;

     QHash<QStringView, Entity> parameterEntityHash;

     QXmlStreamSimpleStack<Entity *>entityReferenceStack;

+    int entityExpansionLimit = 4096;

+    int entityLength = 0;

     inline bool referenceEntity(Entity &entity) {

         if (entity.isCurrentlyReferenced) {

-            raiseWellFormedError(QXmlStream::tr("Recursive entity detected."));

+            raiseWellFormedError(QXmlStream::tr("Self-referencing entity detected."));

+            return false;

+        }

+        // entityLength represents the amount of additional characters the

+        // entity expands into (can be negative for e.g. &). It's used to

+        // avoid DoS attacks through recursive entity expansions

+        entityLength += entity.value.size() - entity.name.size() - 2;

+        if (entityLength > entityExpansionLimit) {

+            raiseWellFormedError(QXmlStream::tr("Entity expands to more characters than the entity expansion limit."));

             return false;

         }

         entity.isCurrentlyReferenced = true;

@@ -1308,6 +1318,8 @@ bool QXmlStreamReaderPrivate::parse()

         case 10:

             entityReferenceStack.pop()->isCurrentlyReferenced = false;

+            if (entityReferenceStack.isEmpty())

+                entityLength = 0;

             clearSym();

         break;

diff --git a/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp b/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp

index 8fdf91b090..1f9a0d575d 100644

--- a/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp

+++ b/tests/auto/corelib/serialization/qxmlstream/tst_qxmlstream.cpp

@@ -393,8 +393,6 @@ public:

                 return true;

             }

-            QXmlStreamReader reader(&inputFile);

-

             /* See testcases.dtd which reads: 'Nonvalidating parsers

              * must also accept "invalid" testcases, but validating ones must reject them.' */

             if(type == QLatin1String("invalid") || type == QLatin1String("valid"))

@@ -580,6 +578,8 @@ private slots:

     void roundTrip() const;

     void roundTrip_data() const;

+    void entityExpansionLimit() const;

+

 private:

     static QByteArray readFile(const QString &filename);

@@ -1756,6 +1756,25 @@ void tst_QXmlStream::roundTrip_data() const

         "</root>\n";

 }

+void tst_QXmlStream::entityExpansionLimit() const

+{

+    QString xml = QStringLiteral(""

+                                 "

+                                 ""

+                                 ""

+                                 ""

+                                 ""

+                                 "]>"

+                                 "<foo>&d;&d;&d;</foo>");

+    {

+        QXmlStreamReader reader(xml);

+        do {

+            reader.readNext();

+        } while (!reader.atEnd());

+        QCOMPARE(reader.error(), QXmlStreamReader::NotWellFormedError);

+    }

+}

+

 void tst_QXmlStream::roundTrip() const

 {

     QFETCH(QString, in);

--

cgit v1.2.1