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>
This commit is contained in:
Mårten Nordheim 2020-11-09 17:13:54 +01:00
parent 7aa68ee6f2
commit 43aaf74f60
3 changed files with 28 additions and 15 deletions

View File

@ -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();

View File

@ -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;
}

View File

@ -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();