diff options
author | Paul Olav Tvete <paul.tvete@qt.io> | 2024-02-27 16:18:33 +0100 |
---|---|---|
committer | Paul Olav Tvete <paul.tvete@qt.io> | 2024-03-01 18:00:33 +0000 |
commit | 997fd3b88ede8078af286da6ecc197e83a8cbb46 (patch) | |
tree | 0b7cc0d345b285c950797d47acc5666831c98793 /src/gui/text | |
parent | 7bde3f4c4c6b4c27ce4ce45b6735d61f405ea53b (diff) |
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
Pick-to: 6.7
Change-Id: I1cf7e1bc4c35276862f37aa6d01f37075fa11635
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
Diffstat (limited to 'src/gui/text')
-rw-r--r-- | src/gui/text/qtextengine.cpp | 23 | ||||
-rw-r--r-- | 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 b5e49e4307..febc364fb3 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 0048eb5e1e..7d5e2aa41e 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<QFixedPoint *>(address); - int offset = totalGlyphs * sizeof(QFixedPoint); + qsizetype offset = totalGlyphs * sizeof(QFixedPoint); glyphs = reinterpret_cast<glyph_t *>(address + offset); offset += totalGlyphs * sizeof(glyph_t); advances = reinterpret_cast<QFixed *>(address + offset); @@ -211,7 +211,7 @@ struct QGlyphLayout last = numGlyphs; if (first == 0 && last == numGlyphs && reinterpret_cast<char *>(offsets + numGlyphs) == reinterpret_cast<char *>(glyphs)) { - memset(static_cast<void *>(offsets), 0, (numGlyphs * SpaceNeeded)); + memset(static_cast<void *>(offsets), 0, qsizetype(numGlyphs) * SpaceNeeded); } else { const int num = last - first; memset(static_cast<void *>(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; |