Fix possible crash in QImage::pixel()

QImage::pixel() assumed that the color table was valid
for the values in the bitmap. This was always wrong
for indexed images with explicit no color table set and
was wrong for mono images that were constructed from
preexisting data.

For mono images, we default to a black/white color table,
like we do when constructing with uninitialized data.

For indexed image, we always default to no color table,
but instead of crashing in pixel(), we warn and return
an undefined value.

[ChangeLog][QtGui][Image] Fixed possible crash in QImage::pixel()
for mono or indexed images.

Change-Id: Ieaf19c03984badddfd06e1855a7e287b862adc70
Task-number: QTBUG-50745
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@theqtcompany.com>
Reviewed-by: Gunnar Sletta <gunnar@sletta.org>

src/gui/image/qimage.cpp

@@ -98,35 +98,22 @@ QImageData::QImageData()
 {
 }
 
-/*! \fn QImageData * QImageData::create(const QSize &size, QImage::Format format, int numColors)
+/*! \fn QImageData * QImageData::create(const QSize &size, QImage::Format format)
 
     \internal
 
     Creates a new image data.
     Returns 0 if invalid parameters are give or anything else failed.
 */
-QImageData * QImageData::create(const QSize &size, QImage::Format format, int numColors)
+QImageData * QImageData::create(const QSize &size, QImage::Format format)
 {
-    if (!size.isValid() || numColors < 0 || format == QImage::Format_Invalid)
+    if (!size.isValid() || format == QImage::Format_Invalid)
         return 0;                                // invalid parameter(s)
 
     uint width = size.width();
     uint height = size.height();
     uint depth = qt_depthForFormat(format);
 
-    switch (format) {
-    case QImage::Format_Mono:
-    case QImage::Format_MonoLSB:
-        numColors = 2;
-        break;
-    case QImage::Format_Indexed8:
-        numColors = qBound(0, numColors, 256);
-        break;
-    default:
-        numColors = 0;
-        break;
-    }
-
     const int bytes_per_line = ((width * depth + 31) >> 5) << 2; // bytes per scanline (must be multiple of 4)
 
     // sanity check for potential overflows
@@ -138,13 +125,16 @@ QImageData * QImageData::create(const QSize &size, QImage::Format format, int nu
         return 0;
 
     QScopedPointer<QImageData> d(new QImageData);
-    d->colortable.resize(numColors);
-    if (depth == 1) {
+
+    switch (format) {
+    case QImage::Format_Mono:
+    case QImage::Format_MonoLSB:
+        d->colortable.resize(2);
         d->colortable[0] = QColor(Qt::black).rgba();
         d->colortable[1] = QColor(Qt::white).rgba();
-    } else {
-        for (int i = 0; i < numColors; ++i)
-            d->colortable[i] = 0;
+        break;
+    default:
+        break;
     }
 
     d->width = width;
@@ -773,7 +763,7 @@ QImage::QImage() Q_DECL_NOEXCEPT
 QImage::QImage(int width, int height, Format format)
     : QPaintDevice()
 {
-    d = QImageData::create(QSize(width, height), format, 0);
+    d = QImageData::create(QSize(width, height), format);
 }
 
 /*!
@@ -788,7 +778,7 @@ QImage::QImage(int width, int height, Format format)
 QImage::QImage(const QSize &size, Format format)
     : QPaintDevice()
 {
-    d = QImageData::create(size, format, 0);
+    d = QImageData::create(size, format);
 }
 
 
@@ -832,6 +822,17 @@ QImageData *QImageData::create(uchar *data, int width, int height,  int bpl, QIm
     d->cleanupFunction = cleanupFunction;
     d->cleanupInfo = cleanupInfo;
 
+    switch (format) {
+    case QImage::Format_Mono:
+    case QImage::Format_MonoLSB:
+        d->colortable.resize(2);
+        d->colortable[0] = QColor(Qt::black).rgba();
+        d->colortable[1] = QColor(Qt::white).rgba();
+        break;
+    default:
+        break;
+    }
+
     return d;
 }
 
@@ -2236,7 +2237,15 @@ QRgb QImage::pixel(int x, int y) const
     case Format_MonoLSB:
         return d->colortable.at((*(s + (x >> 3)) >> (x & 7)) & 1);
     case Format_Indexed8:
-        return d->colortable.at((int)s[x]);
+        {
+            int index = (int)s[x];
+            if (index < d->colortable.size()) {
+                return d->colortable.at(index);
+            } else {
+                qWarning("QImage::pixel: color table index %d out of range.", index);
+                return 0;
+            }
+        }
     case Format_RGB32:
         return 0xff000000 | reinterpret_cast<const QRgb *>(s)[x];
     case Format_ARGB32: // Keep old behaviour.

src/gui/image/qimage_p.h

@@ -57,7 +57,7 @@ class QImageWriter;
 struct Q_GUI_EXPORT QImageData {        // internal image data
     QImageData();
     ~QImageData();
-    static QImageData *create(const QSize &size, QImage::Format format, int numColors = 0);
+    static QImageData *create(const QSize &size, QImage::Format format);
     static QImageData *create(uchar *data, int w, int h,  int bpl, QImage::Format format, bool readOnly, QImageCleanupFunction cleanupFunction = 0, void *cleanupInfo = 0);
 
     QAtomicInt ref;

tests/auto/gui/image/qimage/tst_qimage.cpp

@@ -197,6 +197,7 @@ private slots:
     void metadataPassthrough();
 
     void pixelColor();
+    void pixel();
 
 private:
     const QString m_prefix;
@@ -3038,5 +3039,33 @@ void tst_QImage::pixelColor()
     QCOMPARE(t.pixel(0,0), argb32pm.pixel(0,0));
 }
 
+void tst_QImage::pixel()
+{
+    {
+        QImage mono(1, 1, QImage::Format_Mono);
+        QImage monolsb(1, 1, QImage::Format_MonoLSB);
+        QImage indexed(1, 1, QImage::Format_Indexed8);
+
+        mono.fill(0);
+        monolsb.fill(0);
+        indexed.fill(0);
+
+        QCOMPARE(QColor(mono.pixel(0, 0)), QColor(Qt::black));
+        QCOMPARE(QColor(monolsb.pixel(0, 0)), QColor(Qt::black));
+        indexed.pixel(0, 0); // Don't crash
+    }
+
+    {
+        uchar a = 0;
+        QImage mono(&a, 1, 1, QImage::Format_Mono);
+        QImage monolsb(&a, 1, 1, QImage::Format_MonoLSB);
+        QImage indexed(&a, 1, 1, QImage::Format_Indexed8);
+
+        QCOMPARE(QColor(mono.pixel(0, 0)), QColor(Qt::black));
+        QCOMPARE(QColor(monolsb.pixel(0, 0)), QColor(Qt::black));
+        indexed.pixel(0, 0); // Don't crash
+    }
+}
+
 QTEST_GUILESS_MAIN(tst_QImage)
 #include "tst_qimage.moc"