From 343df131f7207d65932c6505769aa2fb7fc04713 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Tue, 25 Mar 2014 15:55:34 +0100 Subject: 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. Backport of If44b2235a479224660d508a0504fec40d724763a from Qt 5 Task-number: QTBUG-35927 Change-Id: If145ee715a143b889538243f45227d8d78a0050f Reviewed-by: Laszlo Agocs --- src/gui/painting/qblendfunctions.cpp | 30 +++++++++++++++--------------- src/gui/painting/qblendfunctions_p.h | 22 ++++++++++++++++++++-- src/gui/painting/qdrawhelper_neon.cpp | 12 ++++++------ src/gui/painting/qdrawhelper_neon_p.h | 4 ++-- src/gui/painting/qdrawhelper_p.h | 2 +- src/gui/painting/qpaintengine_raster.cpp | 2 +- 6 files changed, 45 insertions(+), 27 deletions(-) diff --git a/src/gui/painting/qblendfunctions.cpp b/src/gui/painting/qblendfunctions.cpp index 6f0fe1e501..e11990f639 100644 --- a/src/gui/painting/qblendfunctions.cpp +++ b/src/gui/painting/qblendfunctions.cpp @@ -179,7 +179,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 sh, const QRectF &targetRect, const QRectF &sourceRect, const QRect &clip, @@ -194,17 +194,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(destPixels, dbpl, srcPixels, sbpl, + qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, sh, targetRect, sourceRect, clip, noAlpha); } else { Blend_RGB16_on_RGB16_ConstAlpha constAlpha(const_alpha); - qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, + qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, sh, targetRect, sourceRect, clip, constAlpha); } } void qt_scale_image_argb24_on_rgb16(uchar *destPixels, int dbpl, - const uchar *srcPixels, int sbpl, + const uchar *srcPixels, int sbpl, int sh, const QRectF &targetRect, const QRectF &sourceRect, const QRect &clip, @@ -219,18 +219,18 @@ void qt_scale_image_argb24_on_rgb16(uchar *destPixels, int dbpl, #endif if (const_alpha == 256) { Blend_ARGB24_on_RGB16_SourceAlpha noAlpha; - qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, + qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, sh, targetRect, sourceRect, clip, noAlpha); } else { Blend_ARGB24_on_RGB16_SourceAndConstAlpha constAlpha(const_alpha); - qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, + qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, sh, 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 sh, const QRectF &targetRect, const QRectF &sourceRect, const QRect &clip, @@ -245,11 +245,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(destPixels, dbpl, srcPixels, sbpl, + qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, sh, targetRect, sourceRect, clip, noAlpha); } else { Blend_ARGB32_on_RGB16_SourceAndConstAlpha constAlpha(const_alpha); - qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, + qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, sh, targetRect, sourceRect, clip, constAlpha); } } @@ -611,7 +611,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 sh, const QRectF &targetRect, const QRectF &sourceRect, const QRect &clip, @@ -626,17 +626,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, sh, 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, sh, 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 sh, const QRectF &targetRect, const QRectF &sourceRect, const QRect &clip, @@ -651,11 +651,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, sh, 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, sh, targetRect, sourceRect, clip, constAlpha); } } diff --git a/src/gui/painting/qblendfunctions_p.h b/src/gui/painting/qblendfunctions_p.h index ef5aca08d2..5d20ceaa81 100644 --- a/src/gui/painting/qblendfunctions_p.h +++ b/src/gui/painting/qblendfunctions_p.h @@ -60,7 +60,7 @@ QT_BEGIN_NAMESPACE template void qt_scale_image_16bit(uchar *destPixels, int dbpl, - const uchar *srcPixels, int sbpl, + const uchar *srcPixels, int sbpl, int sh, 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 >= sh) + --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 void qt_scale_image_32bit(uchar *destPixels, int dbpl, - const uchar *srcPixels, int sbpl, + const uchar *srcPixels, int sbpl, int sh, const QRectF &targetRect, const QRectF &srcRect, const QRect &clip, @@ -236,6 +245,15 @@ template 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 >= sh) + --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; diff --git a/src/gui/painting/qdrawhelper_neon.cpp b/src/gui/painting/qdrawhelper_neon.cpp index 48853f1a43..e3e74d0e1f 100644 --- a/src/gui/painting/qdrawhelper_neon.cpp +++ b/src/gui/painting/qdrawhelper_neon.cpp @@ -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 sh, 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(destPixels, dbpl, srcPixels, sbpl, targetRect, sourceRect, clip, + qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, sh, targetRect, sourceRect, clip, Blend_on_RGB16_SourceAndConstAlpha_Neon_create(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 sh, 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 sh, 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, sh, targetRect, sourceRect, clip, const_alpha); return; } - qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, targetRect, sourceRect, clip, + qt_scale_image_16bit(destPixels, dbpl, srcPixels, sbpl, sh, targetRect, sourceRect, clip, Blend_on_RGB16_SourceAndConstAlpha_Neon_create(blend_8_pixels_rgb16_on_rgb16_neon, const_alpha)); } diff --git a/src/gui/painting/qdrawhelper_neon_p.h b/src/gui/painting/qdrawhelper_neon_p.h index 83be25ee10..360b5ce363 100644 --- a/src/gui/painting/qdrawhelper_neon_p.h +++ b/src/gui/painting/qdrawhelper_neon_p.h @@ -96,14 +96,14 @@ void qt_alphamapblit_quint16_neon(QRasterBuffer *rasterBuffer, const QClipData *clip); void qt_scale_image_argb32_on_rgb16_neon(uchar *destPixels, int dbpl, - const uchar *srcPixels, int sbpl, + const uchar *srcPixels, int sbpl, int sh, 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 sh, const QRectF &targetRect, const QRectF &sourceRect, const QRect &clip, diff --git a/src/gui/painting/qdrawhelper_p.h b/src/gui/painting/qdrawhelper_p.h index 2e346d1446..f80bfcb185 100644 --- a/src/gui/painting/qdrawhelper_p.h +++ b/src/gui/painting/qdrawhelper_p.h @@ -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, diff --git a/src/gui/painting/qpaintengine_raster.cpp b/src/gui/painting/qpaintengine_raster.cpp index 6cbf99554d..f715c142ee 100644 --- a/src/gui/painting/qpaintengine_raster.cpp +++ b/src/gui/painting/qpaintengine_raster.cpp @@ -2369,7 +2369,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); -- cgit v1.2.3