From 79ac8b110ae998ad2545238536cb254197540b57 Mon Sep 17 00:00:00 2001 From: Paul Olav Tvete Date: Tue, 27 Feb 2024 16:18:33 +0100 Subject: [PATCH] QTextEngine: Protect against integer overflow with huge texts QPlainTextEdit would crash when adding a string of 136 348 169 characters, due to the integer overflow checks being done with int variables. Perform intermediate calculations and size checks with qsizetype instead of int. This commit contains a slight modification of the fix contributed by Adam Clarke in the bug report. Note that the size check casts to size_t to cover the 32-bit case where qsizetype is qint32. Fixes: QTBUG-119611 Change-Id: I1cf7e1bc4c35276862f37aa6d01f37075fa11635 Reviewed-by: Eskil Abrahamsen Blomfeldt (cherry picked from commit 997fd3b88ede8078af286da6ecc197e83a8cbb46) Reviewed-by: Qt Cherry-pick Bot --- src/gui/text/qtextengine.cpp | 23 ++++++++++++----------- src/gui/text/qtextengine_p.h | 10 +++++----- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp index b5e49e43074..febc364fb36 100644 --- a/src/gui/text/qtextengine.cpp +++ b/src/gui/text/qtextengine.cpp @@ -2653,14 +2653,14 @@ QTextEngine::LayoutData::LayoutData() currentMaxWidth = 0; } -QTextEngine::LayoutData::LayoutData(const QString &str, void **stack_memory, int _allocated) +QTextEngine::LayoutData::LayoutData(const QString &str, void **stack_memory, qsizetype _allocated) : string(str) { allocated = _allocated; - int space_charAttributes = int(sizeof(QCharAttributes) * string.size() / sizeof(void*) + 1); - int space_logClusters = int(sizeof(unsigned short) * string.size() / sizeof(void*) + 1); - available_glyphs = ((int)allocated - space_charAttributes - space_logClusters)*(int)sizeof(void*)/(int)QGlyphLayout::SpaceNeeded; + qsizetype space_charAttributes = sizeof(QCharAttributes) * string.size() / sizeof(void*) + 1; + qsizetype space_logClusters = sizeof(unsigned short) * string.size() / sizeof(void*) + 1; + available_glyphs = (allocated - space_charAttributes - space_logClusters) * sizeof(void*) / QGlyphLayout::SpaceNeeded; if (available_glyphs < str.size()) { // need to allocate on the heap @@ -2701,15 +2701,16 @@ bool QTextEngine::LayoutData::reallocate(int totalGlyphs) return true; } - int space_charAttributes = int(sizeof(QCharAttributes) * string.size() / sizeof(void*) + 1); - int space_logClusters = int(sizeof(unsigned short) * string.size() / sizeof(void*) + 1); - int space_glyphs = (totalGlyphs * QGlyphLayout::SpaceNeeded) / sizeof(void *) + 2; + 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; - int newAllocated = space_charAttributes + space_glyphs + space_logClusters; - // These values can be negative if the length of string/glyphs causes overflow, + const qsizetype newAllocated = space_charAttributes + space_glyphs + space_logClusters; + // Check if the length of string/glyphs causes int 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 < 0 || space_logClusters < 0 || space_glyphs < 0 || newAllocated < allocated) { + if (size_t(space_charAttributes) > INT_MAX || size_t(space_logClusters) > INT_MAX || totalGlyphs < 0 + || size_t(space_glyphs) > INT_MAX || size_t(newAllocated) > INT_MAX || newAllocated < allocated) { layoutState = LayoutFailed; return false; } @@ -2729,7 +2730,7 @@ bool QTextEngine::LayoutData::reallocate(int totalGlyphs) logClustersPtr = (unsigned short *) m; m += space_logClusters; - const int space_preGlyphLayout = space_charAttributes + space_logClusters; + const qsizetype space_preGlyphLayout = space_charAttributes + space_logClusters; if (allocated < space_preGlyphLayout) memset(memory + allocated, 0, (space_preGlyphLayout - allocated)*sizeof(void *)); diff --git a/src/gui/text/qtextengine_p.h b/src/gui/text/qtextengine_p.h index 0048eb5e1ee..7d5e2aa41ec 100644 --- a/src/gui/text/qtextengine_p.h +++ b/src/gui/text/qtextengine_p.h @@ -178,7 +178,7 @@ struct QGlyphLayout inline explicit QGlyphLayout(char *address, int totalGlyphs) { offsets = reinterpret_cast(address); - int offset = totalGlyphs * sizeof(QFixedPoint); + qsizetype offset = totalGlyphs * sizeof(QFixedPoint); glyphs = reinterpret_cast(address + offset); offset += totalGlyphs * sizeof(glyph_t); advances = reinterpret_cast(address + offset); @@ -211,7 +211,7 @@ struct QGlyphLayout last = numGlyphs; if (first == 0 && last == numGlyphs && reinterpret_cast(offsets + numGlyphs) == reinterpret_cast(glyphs)) { - memset(static_cast(offsets), 0, (numGlyphs * SpaceNeeded)); + memset(static_cast(offsets), 0, qsizetype(numGlyphs) * SpaceNeeded); } else { const int num = last - first; memset(static_cast(offsets + first), 0, num * sizeof(QFixedPoint)); @@ -372,12 +372,12 @@ public: LayoutFailed }; struct Q_GUI_EXPORT LayoutData { - LayoutData(const QString &str, void **stack_memory, int mem_size); + LayoutData(const QString &str, void **stack_memory, qsizetype mem_size); LayoutData(); ~LayoutData(); mutable QScriptItemArray items; - int allocated; - int available_glyphs; + qsizetype allocated; + qsizetype available_glyphs; void **memory; unsigned short *logClustersPtr; QGlyphLayout glyphLayout;