From a5cded843f495b4276a8289b1324778d97bed5ba Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Mon, 11 Feb 2019 10:52:06 +0100 Subject: [PATCH] Avoid creating wide images with negative bytesPerLine The QImage API can not handle images with more bytes per line than what an integer can hold. Fixes: QTBUG-73731 Fixes: QTBUG-73732 Change-Id: Ieed6fec7645661fd58d8d25335f806faaa1bb3e9 Reviewed-by: Thiago Macieira --- src/gui/image/qimage.cpp | 11 +++++++++-- src/gui/image/qimage.h | 4 ++++ src/gui/image/qimage_p.h | 6 ++++++ tests/auto/gui/image/qimage/tst_qimage.cpp | 20 ++++++++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/gui/image/qimage.cpp b/src/gui/image/qimage.cpp index 9897c3aa6f..3e18ca6528 100644 --- a/src/gui/image/qimage.cpp +++ b/src/gui/image/qimage.cpp @@ -124,7 +124,7 @@ QImageData * QImageData::create(const QSize &size, QImage::Format format) int height = size.height(); int depth = qt_depthForFormat(format); auto params = calculateImageParameters(width, height, depth); - if (params.bytesPerLine < 0) + if (!params.isValid()) return nullptr; QScopedPointer d(new QImageData); @@ -781,7 +781,7 @@ QImageData *QImageData::create(uchar *data, int width, int height, int bpl, QIm const int depth = qt_depthForFormat(format); auto params = calculateImageParameters(width, height, depth); - if (params.totalSize < 0) + if (!params.isValid()) return nullptr; if (bpl > 0) { @@ -1484,10 +1484,17 @@ qsizetype QImage::sizeInBytes() const \sa scanLine() */ +#if QT_VERSION >= QT_VERSION_CHECK(6,0,0) +qsizetype QImage::bytesPerLine() const +{ + return d ? d->bytes_per_line : 0; +} +#else int QImage::bytesPerLine() const { return d ? d->bytes_per_line : 0; } +#endif /*! diff --git a/src/gui/image/qimage.h b/src/gui/image/qimage.h index 4b7a3b1ead..6505fd5845 100644 --- a/src/gui/image/qimage.h +++ b/src/gui/image/qimage.h @@ -227,7 +227,11 @@ public: uchar *scanLine(int); const uchar *scanLine(int) const; const uchar *constScanLine(int) const; +#if QT_VERSION >= QT_VERSION_CHECK(6,0,0) + qsizetype bytesPerLine() const; +#else int bytesPerLine() const; +#endif bool valid(int x, int y) const; bool valid(const QPoint &pt) const; diff --git a/src/gui/image/qimage_p.h b/src/gui/image/qimage_p.h index e3a6c53833..a0a3b5406e 100644 --- a/src/gui/image/qimage_p.h +++ b/src/gui/image/qimage_p.h @@ -109,6 +109,7 @@ struct Q_GUI_EXPORT QImageData { // internal image data struct ImageSizeParameters { qsizetype bytesPerLine; qsizetype totalSize; + bool isValid() const { return bytesPerLine > 0 && totalSize > 0; } }; static ImageSizeParameters calculateImageParameters(qsizetype width, qsizetype height, qsizetype depth); }; @@ -135,6 +136,11 @@ QImageData::calculateImageParameters(qsizetype width, qsizetype height, qsizetyp qsizetype dummy; if (mul_overflow(height, qsizetype(sizeof(uchar *)), &dummy)) return invalid; // why is this here? +#if QT_VERSION < QT_VERSION_CHECK(6,0,0) + // Disallow images where width * depth calculations might overflow + if (width > (INT_MAX - 31) / depth) + return invalid; +#endif return { bytes_per_line, total_size }; } diff --git a/tests/auto/gui/image/qimage/tst_qimage.cpp b/tests/auto/gui/image/qimage/tst_qimage.cpp index eded206d37..6bc27a6e16 100644 --- a/tests/auto/gui/image/qimage/tst_qimage.cpp +++ b/tests/auto/gui/image/qimage/tst_qimage.cpp @@ -230,6 +230,8 @@ private slots: void convertColorTable(); + void wideImage(); + #if defined(Q_OS_WIN) && !defined(Q_OS_WINRT) void toWinHBITMAP_data(); void toWinHBITMAP(); @@ -3535,6 +3537,24 @@ void tst_QImage::convertColorTable() QCOMPARE(rgb32.pixel(0,0), 0xffffffff); } +void tst_QImage::wideImage() +{ + // QTBUG-73731 and QTBUG-73732 + QImage i(538994187, 2, QImage::Format_ARGB32); + QImage i2(32, 32, QImage::Format_ARGB32); + i2.fill(Qt::white); + + // Test that it doesn't crash: + QPainter painter(&i); + // With the composition mode is SourceOver out it's an invalid write + // With the composition mode is Source it's an invalid read + painter.drawImage(0, 0, i2); + painter.setCompositionMode(QPainter::CompositionMode_Source); + painter.drawImage(0, 0, i2); + + // Qt6: Test that it actually works on 64bit architectures. +} + #if defined(Q_OS_WIN) && !defined(Q_OS_WINRT) QT_BEGIN_NAMESPACE Q_GUI_EXPORT HBITMAP qt_imageToWinHBITMAP(const QImage &p, int hbitmapFormat = 0);