Avoid out of bounds memory reads when scaling images

The calculation of the width/height required for the
scaling algorithm was prone to floating point rounding
issues, where the lower value got rounded down, the higher
one rounded up. This could lead to a situation where we
iterated over one more line/pixel in the line than we have
in the source image.

Correct this by passing the dimension of the source image into
the function and bounds checking the values before iterating.

Task-number: QTBUG-35927
Change-Id: If44b2235a479224660d508a0504fec40d724763a
Reviewed-by: Laszlo Agocs <laszlo.agocs@digia.com>
This commit is contained in:
Lars Knoll 2014-03-25 15:04:22 +01:00 committed by The Qt Project
parent 123ae472e2
commit c4d8734c50
8 changed files with 75 additions and 26 deletions

View File

@ -129,7 +129,7 @@ struct Blend_ARGB32_on_RGB16_SourceAndConstAlpha {
};
void qt_scale_image_rgb16_on_rgb16(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &sourceRect,
const QRect &clip,
@ -144,17 +144,17 @@ void qt_scale_image_rgb16_on_rgb16(uchar *destPixels, int dbpl,
#endif
if (const_alpha == 256) {
Blend_RGB16_on_RGB16_NoAlpha noAlpha;
qt_scale_image_16bit<quint16>(destPixels, dbpl, srcPixels, sbpl,
qt_scale_image_16bit<quint16>(destPixels, dbpl, srcPixels, sbpl, srch,
targetRect, sourceRect, clip, noAlpha);
} else {
Blend_RGB16_on_RGB16_ConstAlpha constAlpha(const_alpha);
qt_scale_image_16bit<quint16>(destPixels, dbpl, srcPixels, sbpl,
qt_scale_image_16bit<quint16>(destPixels, dbpl, srcPixels, sbpl, srch,
targetRect, sourceRect, clip, constAlpha);
}
}
void qt_scale_image_argb32_on_rgb16(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &sourceRect,
const QRect &clip,
@ -169,11 +169,11 @@ void qt_scale_image_argb32_on_rgb16(uchar *destPixels, int dbpl,
#endif
if (const_alpha == 256) {
Blend_ARGB32_on_RGB16_SourceAlpha noAlpha;
qt_scale_image_16bit<quint32>(destPixels, dbpl, srcPixels, sbpl,
qt_scale_image_16bit<quint32>(destPixels, dbpl, srcPixels, sbpl, srch,
targetRect, sourceRect, clip, noAlpha);
} else {
Blend_ARGB32_on_RGB16_SourceAndConstAlpha constAlpha(const_alpha);
qt_scale_image_16bit<quint32>(destPixels, dbpl, srcPixels, sbpl,
qt_scale_image_16bit<quint32>(destPixels, dbpl, srcPixels, sbpl, srch,
targetRect, sourceRect, clip, constAlpha);
}
}
@ -453,7 +453,7 @@ struct Blend_ARGB32_on_ARGB32_SourceAndConstAlpha {
};
void qt_scale_image_rgb32_on_rgb32(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &sourceRect,
const QRect &clip,
@ -468,17 +468,17 @@ void qt_scale_image_rgb32_on_rgb32(uchar *destPixels, int dbpl,
#endif
if (const_alpha == 256) {
Blend_RGB32_on_RGB32_NoAlpha noAlpha;
qt_scale_image_32bit(destPixels, dbpl, srcPixels, sbpl,
qt_scale_image_32bit(destPixels, dbpl, srcPixels, sbpl, srch,
targetRect, sourceRect, clip, noAlpha);
} else {
Blend_RGB32_on_RGB32_ConstAlpha constAlpha(const_alpha);
qt_scale_image_32bit(destPixels, dbpl, srcPixels, sbpl,
qt_scale_image_32bit(destPixels, dbpl, srcPixels, sbpl, srch,
targetRect, sourceRect, clip, constAlpha);
}
}
void qt_scale_image_argb32_on_argb32(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &sourceRect,
const QRect &clip,
@ -493,11 +493,11 @@ void qt_scale_image_argb32_on_argb32(uchar *destPixels, int dbpl,
#endif
if (const_alpha == 256) {
Blend_ARGB32_on_ARGB32_SourceAlpha sourceAlpha;
qt_scale_image_32bit(destPixels, dbpl, srcPixels, sbpl,
qt_scale_image_32bit(destPixels, dbpl, srcPixels, sbpl, srch,
targetRect, sourceRect, clip, sourceAlpha);
} else {
Blend_ARGB32_on_ARGB32_SourceAndConstAlpha constAlpha(const_alpha);
qt_scale_image_32bit(destPixels, dbpl, srcPixels, sbpl,
qt_scale_image_32bit(destPixels, dbpl, srcPixels, sbpl, srch,
targetRect, sourceRect, clip, constAlpha);
}
}

View File

@ -60,7 +60,7 @@ QT_BEGIN_NAMESPACE
template <typename SRC, typename T>
void qt_scale_image_16bit(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &srcRect,
const QRect &clip,
@ -136,6 +136,15 @@ void qt_scale_image_16bit(uchar *destPixels, int dbpl,
quint16 *dst = ((quint16 *) (destPixels + ty1 * dbpl)) + tx1;
// this bounds check here is required as floating point rounding above might in some cases lead to
// w/h values that are one pixel too large, falling outside of the valid image area.
int yend = (srcy + iy * (h - 1)) >> 16;
if (yend < 0 || yend >= srch)
--h;
int xend = (basex + ix * (w - 1)) >> 16;
if (xend < 0 || xend >= (int)(sbpl/sizeof(quint32)))
--w;
while (h--) {
const SRC *src = (const SRC *) (srcPixels + (srcy >> 16) * sbpl);
int srcx = basex;
@ -161,7 +170,7 @@ void qt_scale_image_16bit(uchar *destPixels, int dbpl,
}
template <typename T> void qt_scale_image_32bit(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &srcRect,
const QRect &clip,
@ -215,6 +224,8 @@ template <typename T> void qt_scale_image_32bit(uchar *destPixels, int dbpl,
int h = ty2 - ty1;
int w = tx2 - tx1;
if (!w || !h)
return;
quint32 basex;
quint32 srcy;
@ -236,6 +247,15 @@ template <typename T> void qt_scale_image_32bit(uchar *destPixels, int dbpl,
quint32 *dst = ((quint32 *) (destPixels + ty1 * dbpl)) + tx1;
// this bounds check here is required as floating point rounding above might in some cases lead to
// w/h values that are one pixel too large, falling outside of the valid image area.
int yend = (srcy + iy * (h - 1)) >> 16;
if (yend < 0 || yend >= srch)
--h;
int xend = (basex + ix * (w - 1)) >> 16;
if (xend < 0 || xend >= (int)(sbpl/sizeof(quint32)))
--w;
while (h--) {
const uint *src = (const quint32 *) (srcPixels + (srcy >> 16) * sbpl);
int srcx = basex;

View File

@ -6522,7 +6522,7 @@ void qInitDrawhelperAsm()
qDrawHelper[QImage::Format_RGBA8888_Premultiplied].bitmapBlit = qt_bitmapblit8888_sse2;
extern void qt_scale_image_argb32_on_argb32_sse2(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &sourceRect,
const QRect &clip,

View File

@ -539,7 +539,7 @@ Blend_on_RGB16_SourceAndConstAlpha_Neon_create(BlendFunc blender, int const_alph
}
void qt_scale_image_argb32_on_rgb16_neon(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &sourceRect,
const QRect &clip,
@ -548,19 +548,19 @@ void qt_scale_image_argb32_on_rgb16_neon(uchar *destPixels, int dbpl,
if (const_alpha == 0)
return;
qt_scale_image_16bit<quint32>(destPixels, dbpl, srcPixels, sbpl, targetRect, sourceRect, clip,
qt_scale_image_16bit<quint32>(destPixels, dbpl, srcPixels, sbpl, srch, targetRect, sourceRect, clip,
Blend_on_RGB16_SourceAndConstAlpha_Neon_create<quint32>(blend_8_pixels_argb32_on_rgb16_neon, const_alpha));
}
void qt_scale_image_rgb16_on_rgb16(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &sourceRect,
const QRect &clip,
int const_alpha);
void qt_scale_image_rgb16_on_rgb16_neon(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &sourceRect,
const QRect &clip,
@ -570,11 +570,11 @@ void qt_scale_image_rgb16_on_rgb16_neon(uchar *destPixels, int dbpl,
return;
if (const_alpha == 256) {
qt_scale_image_rgb16_on_rgb16(destPixels, dbpl, srcPixels, sbpl, targetRect, sourceRect, clip, const_alpha);
qt_scale_image_rgb16_on_rgb16(destPixels, dbpl, srcPixels, sbpl, srch, targetRect, sourceRect, clip, const_alpha);
return;
}
qt_scale_image_16bit<quint16>(destPixels, dbpl, srcPixels, sbpl, targetRect, sourceRect, clip,
qt_scale_image_16bit<quint16>(destPixels, dbpl, srcPixels, sbpl, srch, targetRect, sourceRect, clip,
Blend_on_RGB16_SourceAndConstAlpha_Neon_create<quint16>(blend_8_pixels_rgb16_on_rgb16_neon, const_alpha));
}

View File

@ -138,7 +138,7 @@ typedef void (*SrcOverBlendFunc)(uchar *destPixels, int dbpl,
int const_alpha);
typedef void (*SrcOverScaleFunc)(uchar *destPixels, int dbpl,
const uchar *src, int spbl,
const uchar *src, int spbl, int srch,
const QRectF &targetRect,
const QRectF &sourceRect,
const QRect &clipRect,

View File

@ -568,7 +568,7 @@ const uint * QT_FASTCALL qt_fetch_radial_gradient_sse2(uint *buffer, const Opera
}
void qt_scale_image_argb32_on_argb32_sse2(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &sourceRect,
const QRect &clip,
@ -577,12 +577,12 @@ void qt_scale_image_argb32_on_argb32_sse2(uchar *destPixels, int dbpl,
if (const_alpha != 256) {
// from qblendfunctions.cpp
extern void qt_scale_image_argb32_on_argb32(uchar *destPixels, int dbpl,
const uchar *srcPixels, int sbpl,
const uchar *srcPixels, int sbpl, int srch,
const QRectF &targetRect,
const QRectF &sourceRect,
const QRect &clip,
int const_alpha);
return qt_scale_image_argb32_on_argb32(destPixels, dbpl, srcPixels, sbpl, targetRect, sourceRect, clip, const_alpha);
return qt_scale_image_argb32_on_argb32(destPixels, dbpl, srcPixels, sbpl, srch, targetRect, sourceRect, clip, const_alpha);
}
qreal sx = targetRect.width() / (qreal) sourceRect.width();
@ -651,6 +651,14 @@ void qt_scale_image_argb32_on_argb32_sse2(uchar *destPixels, int dbpl,
const __m128i alphaMask = _mm_set1_epi32(0xff000000);
const __m128i ixVector = _mm_set1_epi32(4*ix);
// this bounds check here is required as floating point rounding above might in some cases lead to
// w/h values that are one pixel too large, falling outside of the valid image area.
int yend = (srcy + iy * (h - 1)) >> 16;
if (yend < 0 || yend >= srch)
--h;
int xend = (basex + ix * (w - 1)) >> 16;
if (xend < 0 || xend >= (int)(sbpl/sizeof(quint32)))
--w;
while (h--) {
const uint *src = (const quint32 *) (srcPixels + (srcy >> 16) * sbpl);

View File

@ -2377,7 +2377,7 @@ void QRasterPaintEngine::drawImage(const QRectF &r, const QImage &img, const QRe
SrcOverScaleFunc func = qScaleFunctions[d->rasterBuffer->format][img.format()];
if (func && (!clip || clip->hasRectClip)) {
func(d->rasterBuffer->buffer(), d->rasterBuffer->bytesPerLine(),
img.bits(), img.bytesPerLine(),
img.bits(), img.bytesPerLine(), img.height(),
qt_mapRect_non_normalizing(r, s->matrix), sr,
!clip ? d->deviceRect : clip->clipRect,
s->intOpacity);

View File

@ -168,6 +168,8 @@ private slots:
void convertOverUnPreMul();
void scaled_QTBUG35972();
void cleanupFunctions();
};
@ -2439,6 +2441,25 @@ void tst_QImage::convertOverUnPreMul()
}
}
void tst_QImage::scaled_QTBUG35972()
{
QImage src(532,519,QImage::Format_ARGB32_Premultiplied);
src.fill(QColor(Qt::white));
QImage dest(1000,1000,QImage::Format_ARGB32_Premultiplied);
dest.fill(QColor(Qt::white));
QPainter painter1(&dest);
const QTransform trf(1.25, 0,
0, 1.25,
/*dx */ 15.900000000000034, /* dy */ 72.749999999999986);
painter1.setTransform(trf);
painter1.drawImage(QRectF(-2.6, -2.6, 425.6, 415.20000000000005), src, QRectF(0,0,532,519));
const quint32 *pixels = reinterpret_cast<const quint32 *>(dest.constBits());
int size = dest.width()*dest.height();
for (int i = 0; i < size; ++i)
QCOMPARE(pixels[i], 0xffffffff);
}
static void cleanupFunction(void* info)
{
bool *called = static_cast<bool*>(info);