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>

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. &amp;). 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. &amp;). 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("<?xml version=\"1.0\"?>"
+                                 "<!DOCTYPE foo ["
+                                 "<!ENTITY a \"0123456789\" >"
+                                 "<!ENTITY b \"&a;&a;&a;&a;&a;&a;&a;&a;&a;&a;\" >"
+                                 "<!ENTITY c \"&b;&b;&b;&b;&b;&b;&b;&b;&b;&b;\" >"
+                                 "<!ENTITY d \"&c;&c;&c;&c;&c;&c;&c;&c;&c;&c;\" >"
+                                 "]>"
+                                 "<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);