QXmlStreamReader: don't store pointers

They were pointing into the QHash structure which could
be invalidated on insert if growing was needed. This
caused some user-after-free issues.

Indexing into the QHash is quite fast, so let's just
store the name and a pointer to the QHash to do that.

Fixes: QTBUG-88246
Change-Id: If7f9b1c6ea7557c5bd0869b42b1b84aa824cc6ce
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

3 files changed, 28 insertions, 15 deletions

diff --git a/src/corelib/serialization/qxmlstream.g b/src/corelib/serialization/qxmlstream.g
index cf91fa8fc8..b6417b080c 100644
--- a/src/corelib/serialization/qxmlstream.g
+++ b/src/corelib/serialization/qxmlstream.g
@@ -524,12 +524,15 @@ prolog ::=;
 
 entity_done ::= ENTITY_DONE;
 /.
-        case $rule_number:
-            entityReferenceStack.pop()->isCurrentlyReferenced = false;
+        case $rule_number: {
+            auto reference = entityReferenceStack.pop();
+            auto it = reference.hash->find(reference.name);
+            Q_ASSERT(it != reference.hash->end());
+            it->isCurrentlyReferenced = false;
             if (entityReferenceStack.isEmpty())
                 entityLength = 0;
             clearSym();
-        break;
+        } break;
 ./
 
 
@@ -1331,7 +1334,7 @@ entity_ref ::= AMPERSAND name SEMICOLON;
                     }
                     if (entity.literal)
                         putStringLiteral(entity.value);
-                    else if (referenceEntity(entity))
+                    else if (referenceEntity(&entityHash, entity))
                         putReplacement(entity.value);
                     textBuffer.chop(2 + sym(2).len);
                     clearSym();
@@ -1368,7 +1371,7 @@ pereference ::= PERCENT name SEMICOLON;
                 if (entity.unparsed || entity.external) {
                     referenceToUnparsedEntityDetected = true;
                 } else {
-                    if (referenceEntity(entity))
+                    if (referenceEntity(&parameterEntityHash, entity))
                         putString(entity.value);
                     textBuffer.chop(2 + sym(2).len);
                     clearSym();
@@ -1405,7 +1408,7 @@ entity_ref_in_attribute_value ::= AMPERSAND name SEMICOLON;
                 }
                 if (entity.literal)
                     putStringLiteral(entity.value);
-                else if (referenceEntity(entity))
+                else if (referenceEntity(&entityHash, entity))
                     putReplacementInAttributeValue(entity.value);
                 textBuffer.chop(2 + sym(2).len);
                 clearSym();
diff --git a/src/corelib/serialization/qxmlstream_p.h b/src/corelib/serialization/qxmlstream_p.h
index 21229a949e..a601c24bd6 100644
--- a/src/corelib/serialization/qxmlstream_p.h
+++ b/src/corelib/serialization/qxmlstream_p.h
@@ -273,10 +273,17 @@ public:
     // are guaranteed to have the same lifetime as the referenced data:
     QHash<QStringView, Entity> entityHash;
     QHash<QStringView, Entity> parameterEntityHash;
-    QXmlStreamSimpleStack<Entity *>entityReferenceStack;
+    struct QEntityReference
+    {
+        QHash<QStringView, Entity> *hash;
+        QStringView name;
+    };
+    QXmlStreamSimpleStack<QEntityReference> entityReferenceStack;
     int entityExpansionLimit = 4096;
     int entityLength = 0;
-    inline bool referenceEntity(Entity &entity) {
+    inline bool referenceEntity(QHash<QStringView, Entity> *hash, Entity &entity)
+    {
+        Q_ASSERT(hash);
         if (entity.isCurrentlyReferenced) {
             raiseWellFormedError(QXmlStream::tr("Self-referencing entity detected."));
             return false;
@@ -290,7 +297,7 @@ public:
             return false;
         }
         entity.isCurrentlyReferenced = true;
-        entityReferenceStack.push() = &entity;
+        entityReferenceStack.push() = { hash, entity.name };
         injectToken(ENTITY_DONE);
         return true;
     }
diff --git a/src/corelib/serialization/qxmlstreamparser_p.h b/src/corelib/serialization/qxmlstreamparser_p.h
index 16c63cba2e..a5d8312390 100644
--- a/src/corelib/serialization/qxmlstreamparser_p.h
+++ b/src/corelib/serialization/qxmlstreamparser_p.h
@@ -360,12 +360,15 @@ bool QXmlStreamReaderPrivate::parse()
             }
         break;
 
-        case 10:
-            entityReferenceStack.pop()->isCurrentlyReferenced = false;
+        case 10: {
+            auto reference = entityReferenceStack.pop();
+            auto it = reference.hash->find(reference.name);
+            Q_ASSERT(it != reference.hash->end());
+            it->isCurrentlyReferenced = false;
             if (entityReferenceStack.isEmpty())
                 entityLength = 0;
             clearSym();
-        break;
+        } break;
 
         case 11:
             if (!scanString(spell[VERSION], VERSION, false) && atEnd) {
@@ -869,7 +872,7 @@ bool QXmlStreamReaderPrivate::parse()
                     }
                     if (entity.literal)
                         putStringLiteral(entity.value);
-                    else if (referenceEntity(entity))
+                    else if (referenceEntity(&entityHash, entity))
                         putReplacement(entity.value);
                     textBuffer.chop(2 + sym(2).len);
                     clearSym();
@@ -903,7 +906,7 @@ bool QXmlStreamReaderPrivate::parse()
                 if (entity.unparsed || entity.external) {
                     referenceToUnparsedEntityDetected = true;
                 } else {
-                    if (referenceEntity(entity))
+                    if (referenceEntity(&parameterEntityHash, entity))
                         putString(entity.value);
                     textBuffer.chop(2 + sym(2).len);
                     clearSym();
@@ -932,7 +935,7 @@ bool QXmlStreamReaderPrivate::parse()
                 }
                 if (entity.literal)
                     putStringLiteral(entity.value);
-                else if (referenceEntity(entity))
+                else if (referenceEntity(&entityHash, entity))
                     putReplacementInAttributeValue(entity.value);
                 textBuffer.chop(2 + sym(2).len);
                 clearSym();