Details
-
Bug
-
Resolution: Fixed
-
P1: Critical
-
5.15.0, 6.3.1, 6.6.1
-
None
-
-
997fd3b88 (dev), 79ac8b110 (6.7), 7a84c58f5 (dev), c9b5b40d1 (6.7)
Description
Repro:
static_assert( (int)( 0x100000001 ) > 0 ); // 21 for QGlyphLayout::SpaceNeeded. Not sure where 1.5 comes from exactly, but inserting latin letters seems to result in 1.5x as many glyphs. static_assert( 0x100000001 / 1.5 / 21 + 1 <= INT_MAX ); QString buffer; buffer.fill( 'A', 0x100000001 / 1.5 / 21 + 1 ); QPlainTextEdit *plainTextEdit = new QPlainTextEdit( nullptr ); QTextCursor textCursor = plainTextEdit->textCursor(); textCursor.beginEditBlock(); textCursor.insertText( buffer ); // will crash due to unchecked integer overflow of space_glyphs in bool QTextEngine::LayoutData::reallocate(int totalGlyphs) textCursor.endEditBlock(); delete plainTextEdit;
Crash callstacks from Qt 5.15.0. I tested that this still occurs on Qt 6.3.1. And I can see in source code where the issue is still present in the latest dev version.
Access violation writing location 0x000001B5EBE42F18. > qwindowsd.dll!QWindowsFontEngine::getGlyphIndexes(const QChar * str, int numChars, QGlyphLayout * glyphs) Line 195 C++ qwindowsd.dll!QWindowsFontEngine::stringToCMap(const QChar * str, int len, QGlyphLayout * glyphs, int * nglyphs, QFlags<enum QFontEngine::ShaperFlag> flags) Line 324 C++ Qt5Guid.dll!QFontEngineMulti::stringToCMap(const QChar * str, int len, QGlyphLayout * glyphs, int * nglyphs, QFlags<enum QFontEngine::ShaperFlag> flags) Line 1891 C++ Qt5Guid.dll!QTextEngine::shapeText(int item) Line 1481 C++ Qt5Guid.dll!QTextEngine::shape(int item) Line 2030 C++ Qt5Guid.dll!QTextLine::layout_helper(int maxGlyphs) Line 1852 C++ Qt5Guid.dll!QTextLine::setLineWidth(double width) Line 1606 C++ Qt5Widgetsd.dll!QPlainTextDocumentLayout::layoutBlock(const QTextBlock & block) Line 391 C++ Qt5Widgetsd.dll!QPlainTextDocumentLayout::documentChanged(int from, int charsRemoved, int charsAdded) Line 305 C++ Qt5Guid.dll!QTextDocumentPrivate::finishEdit() Line 1224 C++ Qt5Guid.dll!QTextDocumentPrivate::endEditBlock() Line 1204 C++ Qt5Guid.dll!QTextCursor::endEditBlock() Line 2536 C++
Access violation writing location 0x000002CE1A207000. > vcruntime140d.dll!memset_repstos() Line 35 Unknown Qt5Guid.dll!QGlyphLayout::clear(int first, int last) Line 253 C++ Qt5Guid.dll!QGlyphLayout::grow(char * address, int totalGlyphs) Line 2947 C++ Qt5Guid.dll!QTextEngine::LayoutData::reallocate(int totalGlyphs) Line 2926 C++ Qt5Guid.dll!QTextEngine::ensureSpace(int nGlyphs) Line 520 C++ Qt5Guid.dll!QTextEngine::attributes() Line 1981 C++ Qt5Guid.dll!QTextLine::layout_helper(int maxGlyphs) Line 1834 C++ Qt5Guid.dll!QTextLine::setLineWidth(double width) Line 1606 C++ Qt5Widgetsd.dll!QPlainTextDocumentLayout::layoutBlock(const QTextBlock & block) Line 391 C++ Qt5Widgetsd.dll!QPlainTextDocumentLayout::documentChanged(int from, int charsRemoved, int charsAdded) Line 305 C++ Qt5Guid.dll!QTextDocumentPrivate::finishEdit() Line 1224 C++ Qt5Guid.dll!QTextDocumentPrivate::endEditBlock() Line 1204 C++ Qt5Guid.dll!QTextCursor::endEditBlock() Line 2536 C++
To fix, I suggest:
bool QTextEngine::LayoutData::reallocate(int totalGlyphs) { Q_ASSERT(totalGlyphs >= glyphLayout.numGlyphs); if (memory_on_stack && available_glyphs >= totalGlyphs) { glyphLayout.grow(glyphLayout.data(), totalGlyphs); return true; } const qsizetype space_charAttributes = sizeof(QCharAttributes) * string.size() / sizeof(void*) + 1; const qsizetype space_logClusters = sizeof(unsigned short) * string.size() / sizeof(void*) + 1; const qsizetype space_glyphs = (qsizetype)totalGlyphs * QGlyphLayout::SpaceNeeded / sizeof(void *) + 2; const qsizetype newAllocated = space_charAttributes + space_logClusters + space_glyphs; // If these values would cause integer overflow, // we can't layout such a long string all at once, so return false here to // indicate there is a failure if (space_charAttributes > INT_MAX || space_logClusters > INT_MAX || totalGlyphs < 0 || space_glyphs > INT_MAX || newAllocated > INT_MAX || newAllocated < allocated) { layoutState = LayoutFailed; return false; } void **newMem = (void **)::realloc(memory_on_stack ? nullptr : memory, newAllocated*sizeof(void *)); if (!newMem) { layoutState = LayoutFailed; return false; } if (memory_on_stack) memcpy(newMem, memory, allocated*sizeof(void *)); memory = newMem; memory_on_stack = false; void **m = memory; m += space_charAttributes; logClustersPtr = (unsigned short *) m; m += space_logClusters; const int space_preGlyphLayout = space_charAttributes + space_logClusters; if (allocated < space_preGlyphLayout) memset(memory + allocated, 0, (space_preGlyphLayout - allocated)*sizeof(void *)); glyphLayout.grow(reinterpret_cast<char *>(m), totalGlyphs); allocated = (int)newAllocated; return true; }
Attachments
For Gerrit Dashboard: QTBUG-119611 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
543612,5 | QTextEngine: Protect against integer overflow with huge texts | dev | qt/qtbase | Status: MERGED | +2 | 0 |
544689,2 | QTextEngine: Protect against integer overflow with huge texts | 6.7 | qt/qtbase | Status: MERGED | +2 | 0 |
548335,2 | Fix QTextEngine regression with large-ish texts | dev | qt/qtbase | Status: MERGED | +2 | 0 |
548393,2 | Fix QTextEngine regression with large-ish texts | 6.7 | qt/qtbase | Status: MERGED | +2 | 0 |