Fix possible crash in QTextLayout for glyphless items

Change e1915815bc5ef86b3844608bba46769da5173363 moved part of the
right bearing check out of the "non-whitespace-or-object" block of the
layout, which could potentially cause crashes for layouts that contained
items that were line separators or tabs etc. because we would access
the logical clusters array based on the position of e.g. the tab even
though it didn't have an entry. This could potentially give us an
arbitrary index which might cause an out of bounds when accessing the
glyphs array.

Task-number: QTBUG-11427
Reviewed-by: Simon Hausmann

2 files changed, 30 insertions, 3 deletions

diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp
index c5dd854..34272cc 100644
--- a/src/gui/text/qtextlayout.cpp
+++ b/src/gui/text/qtextlayout.cpp
@@ -1646,7 +1646,7 @@ namespace {
     {
         LineBreakHelper()
             : glyphCount(0), maxGlyphs(0), currentPosition(0), fontEngine(0), logClusters(0),
-              manualWrap(false)
+              manualWrap(false), whiteSpaceOrObject(true)
         {
         }
 
@@ -1669,6 +1669,7 @@ namespace {
         const unsigned short *logClusters;
 
         bool manualWrap;
+        bool whiteSpaceOrObject;
 
         bool checkFullOtherwiseExtend(QScriptLine &line);
 
@@ -1678,8 +1679,10 @@ namespace {
         }
 
         inline glyph_t currentGlyph() const
-        {
+        {            
             Q_ASSERT(currentPosition > 0);
+            Q_ASSERT(logClusters[currentPosition - 1] < glyphs.numGlyphs);
+
             return glyphs.glyphs[logClusters[currentPosition - 1]];
         }
 
@@ -1814,6 +1817,7 @@ void QTextLine::layout_helper(int maxGlyphs)
         lbh.tmpData.descent = qMax(lbh.tmpData.descent, current.descent);
 
         if (current.analysis.flags == QScriptAnalysis::Tab && (alignment & (Qt::AlignLeft | Qt::AlignRight | Qt::AlignCenter | Qt::AlignJustify))) {
+            lbh.whiteSpaceOrObject = true;
             if (lbh.checkFullOtherwiseExtend(line))
                 goto found;
 
@@ -1830,6 +1834,7 @@ void QTextLine::layout_helper(int maxGlyphs)
             if (lbh.checkFullOtherwiseExtend(line))
                 goto found;
         } else if (current.analysis.flags == QScriptAnalysis::LineOrParagraphSeparator) {
+            lbh.whiteSpaceOrObject = true;
             // if the line consists only of the line separator make sure
             // we have a sane height
             if (!line.length && !lbh.tmpData.length)
@@ -1843,6 +1848,7 @@ void QTextLine::layout_helper(int maxGlyphs)
             line += lbh.tmpData;
             goto found;
         } else if (current.analysis.flags == QScriptAnalysis::Object) {
+            lbh.whiteSpaceOrObject = true;
             lbh.tmpData.length++;
 
             QTextFormat format = eng->formats()->format(eng->formatIndex(&eng->layoutData->items[item]));
@@ -1856,6 +1862,7 @@ void QTextLine::layout_helper(int maxGlyphs)
             if (lbh.checkFullOtherwiseExtend(line))
                 goto found;
         } else if (attributes[lbh.currentPosition].whiteSpace) {
+            lbh.whiteSpaceOrObject = true;
             while (lbh.currentPosition < end && attributes[lbh.currentPosition].whiteSpace)
                 addNextCluster(lbh.currentPosition, end, lbh.spaceData, lbh.glyphCount,
                                current, lbh.logClusters, lbh.glyphs);
@@ -1865,6 +1872,7 @@ void QTextLine::layout_helper(int maxGlyphs)
                 goto found;
             }
         } else {
+            lbh.whiteSpaceOrObject = false;
             bool sb_or_ws = false;
             do {
                 addNextCluster(lbh.currentPosition, end, lbh.tmpData, lbh.glyphCount,
@@ -1926,7 +1934,7 @@ void QTextLine::layout_helper(int maxGlyphs)
     LB_DEBUG("reached end of line");
     lbh.checkFullOtherwiseExtend(line);
 found:       
-    if (lbh.rightBearing > 0) // If right bearing has not yet been adjusted
+    if (lbh.rightBearing > 0 && !lbh.whiteSpaceOrObject) // If right bearing has not yet been adjusted
         lbh.adjustRightBearing();
     line.textAdvance = line.textWidth;
     line.textWidth -= qMin(QFixed(), lbh.rightBearing);
diff --git a/tests/auto/qtextlayout/tst_qtextlayout.cpp b/tests/auto/qtextlayout/tst_qtextlayout.cpp
index caf9bd3..6d27ef2 100644
--- a/tests/auto/qtextlayout/tst_qtextlayout.cpp
+++ b/tests/auto/qtextlayout/tst_qtextlayout.cpp
@@ -110,6 +110,7 @@ private slots:
     void longText();
     void widthOfTabs();
     void columnWrapWithTabs();
+    void glyphLessItems();
 
     // QTextLine stuff
     void setNumColumnsWrapAtWordBoundaryOrAnywhere();
@@ -1319,6 +1320,24 @@ void tst_QTextLayout::lineWidthFromBOM()
     // Don't spin into an infinite loop
  }
 
+void tst_QTextLayout::glyphLessItems()
+{
+    {
+        QTextLayout layout;
+        layout.setText("\t\t");
+        layout.beginLayout();
+        layout.createLine();
+        layout.endLayout();
+    }
+
+    {
+        QTextLayout layout;
+        layout.setText(QString::fromLatin1("AA") + QChar(QChar::LineSeparator));
+        layout.beginLayout();
+        layout.createLine();
+        layout.endLayout();
+    }
+}
 
 QTEST_MAIN(tst_QTextLayout)
 #include "tst_qtextlayout.moc"