From f81148c43a25d4c69e635b6ad13eab674b473aca Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Wed, 25 Jul 2018 20:44:32 +0200 Subject: [PATCH] Avoid integer overflows in pixel size calculations --- common/rfb/EncodeManager.cxx | 4 ++-- common/rfb/PixelBuffer.cxx | 29 ++++++++++++++++------------- common/rfb/RawDecoder.cxx | 4 ++-- common/rfb/SMsgWriter.cxx | 2 +- common/rfb/TightDecoder.cxx | 2 +- common/rfb/ZRLEEncoder.cxx | 2 +- 6 files changed, 23 insertions(+), 20 deletions(-) diff --git a/common/rfb/EncodeManager.cxx b/common/rfb/EncodeManager.cxx index 0ce611e9..53e0365d 100644 --- a/common/rfb/EncodeManager.cxx +++ b/common/rfb/EncodeManager.cxx @@ -519,7 +519,7 @@ Encoder *EncodeManager::startRect(const Rect& rect, int type) stats[klass][activeType].rects++; stats[klass][activeType].pixels += rect.area(); - equiv = 12 + rect.area() * conn->cp.pf().bpp/8; + equiv = 12 + rect.area() * (conn->cp.pf().bpp/8); stats[klass][activeType].equivalent += equiv; encoder = encoders[klass]; @@ -561,7 +561,7 @@ void EncodeManager::writeCopyRects(const Region& copied, const Point& delta) copyStats.rects++; copyStats.pixels += rect->area(); - equiv = 12 + rect->area() * conn->cp.pf().bpp/8; + equiv = 12 + rect->area() * (conn->cp.pf().bpp/8); copyStats.equivalent += equiv; conn->writer()->writeCopyRect(*rect, rect->tl.x - delta.x, diff --git a/common/rfb/PixelBuffer.cxx b/common/rfb/PixelBuffer.cxx index 007b6c84..7f4c1ad3 100644 --- a/common/rfb/PixelBuffer.cxx +++ b/common/rfb/PixelBuffer.cxx @@ -204,6 +204,7 @@ void ModifiablePixelBuffer::copyRect(const Rect &rect, const Point &move_by_delta) { int srcStride, dstStride; + int bytesPerPixel; const U8* srcData; U8* dstData; @@ -221,6 +222,8 @@ void ModifiablePixelBuffer::copyRect(const Rect &rect, srect.width(), srect.height(), srect.tl.x, srect.tl.y, width_, height_); + bytesPerPixel = format.bpp/8; + srcData = getBuffer(srect, &srcStride); dstData = getBufferRW(drect, &dstStride); @@ -228,27 +231,27 @@ void ModifiablePixelBuffer::copyRect(const Rect &rect, // Possible overlap. Be careful and use memmove(). int h = drect.height(); while (h--) { - memmove(dstData, srcData, drect.width() * format.bpp/8); - dstData += dstStride * format.bpp/8; - srcData += srcStride * format.bpp/8; + memmove(dstData, srcData, drect.width() * bytesPerPixel); + dstData += dstStride * bytesPerPixel; + srcData += srcStride * bytesPerPixel; } } else if (move_by_delta.y < 0) { // The data shifted upwards. Copy from top to bottom. int h = drect.height(); while (h--) { - memcpy(dstData, srcData, drect.width() * format.bpp/8); - dstData += dstStride * format.bpp/8; - srcData += srcStride * format.bpp/8; + memcpy(dstData, srcData, drect.width() * bytesPerPixel); + dstData += dstStride * bytesPerPixel; + srcData += srcStride * bytesPerPixel; } } else { // The data shifted downwards. Copy from bottom to top. int h = drect.height(); - dstData += (h-1) * dstStride * format.bpp/8; - srcData += (h-1) * srcStride * format.bpp/8; + dstData += (h-1) * dstStride * bytesPerPixel; + srcData += (h-1) * srcStride * bytesPerPixel; while (h--) { - memcpy(dstData, srcData, drect.width() * format.bpp/8); - dstData -= dstStride * format.bpp/8; - srcData -= srcStride * format.bpp/8; + memcpy(dstData, srcData, drect.width() * bytesPerPixel); + dstData -= dstStride * bytesPerPixel; + srcData -= srcStride * bytesPerPixel; } } @@ -304,7 +307,7 @@ rdr::U8* FullFramePixelBuffer::getBufferRW(const Rect& r, int* stride_) r.tl.x, r.tl.y, width_, height_); *stride_ = stride; - return &data[(r.tl.x + (r.tl.y * stride)) * format.bpp/8]; + return &data[(r.tl.x + (r.tl.y * stride)) * (format.bpp/8)]; } void FullFramePixelBuffer::commitBufferRW(const Rect& r) @@ -319,7 +322,7 @@ const rdr::U8* FullFramePixelBuffer::getBuffer(const Rect& r, int* stride_) cons r.tl.x, r.tl.y, width_, height_); *stride_ = stride; - return &data[(r.tl.x + (r.tl.y * stride)) * format.bpp/8]; + return &data[(r.tl.x + (r.tl.y * stride)) * (format.bpp/8)]; } // -=- Managed pixel buffer class diff --git a/common/rfb/RawDecoder.cxx b/common/rfb/RawDecoder.cxx index 786f1545..ec0c68ee 100644 --- a/common/rfb/RawDecoder.cxx +++ b/common/rfb/RawDecoder.cxx @@ -36,13 +36,13 @@ RawDecoder::~RawDecoder() void RawDecoder::readRect(const Rect& r, rdr::InStream* is, const ConnParams& cp, rdr::OutStream* os) { - os->copyBytes(is, r.area() * cp.pf().bpp/8); + os->copyBytes(is, r.area() * (cp.pf().bpp/8)); } void RawDecoder::decodeRect(const Rect& r, const void* buffer, size_t buflen, const ConnParams& cp, ModifiablePixelBuffer* pb) { - assert(buflen >= (size_t)r.area() * cp.pf().bpp/8); + assert(buflen >= (size_t)r.area() * (cp.pf().bpp/8)); pb->imageRect(cp.pf(), r, buffer); } diff --git a/common/rfb/SMsgWriter.cxx b/common/rfb/SMsgWriter.cxx index 6ef7692e..3da9413f 100644 --- a/common/rfb/SMsgWriter.cxx +++ b/common/rfb/SMsgWriter.cxx @@ -350,7 +350,7 @@ void SMsgWriter::writePseudoRects() if (needSetCursor) { const Cursor& cursor = cp->cursor(); - rdr::U8Array data(cursor.width()*cursor.height() * cp->pf().bpp/8); + rdr::U8Array data(cursor.width()*cursor.height() * (cp->pf().bpp/8)); rdr::U8Array mask(cursor.getMask()); const rdr::U8* in; diff --git a/common/rfb/TightDecoder.cxx b/common/rfb/TightDecoder.cxx index 3a1254a2..cc786f5b 100644 --- a/common/rfb/TightDecoder.cxx +++ b/common/rfb/TightDecoder.cxx @@ -364,7 +364,7 @@ void TightDecoder::decodeRect(const Rect& r, const void* buffer, if (directDecode) outbuf = pb->getBufferRW(r, &stride); else { - outbuf = new rdr::U8[r.area() * pf.bpp/8]; + outbuf = new rdr::U8[r.area() * (pf.bpp/8)]; stride = r.width(); } diff --git a/common/rfb/ZRLEEncoder.cxx b/common/rfb/ZRLEEncoder.cxx index d3afe747..8917d8ff 100644 --- a/common/rfb/ZRLEEncoder.cxx +++ b/common/rfb/ZRLEEncoder.cxx @@ -223,7 +223,7 @@ void ZRLEEncoder::writePixels(const rdr::U8* buffer, const PixelFormat& pf, pf.bufferFromPixel(pixBuf, maxPixel); if ((pf.bpp != 32) || ((pixBuf[0] != 0) && (pixBuf[3] != 0))) { - zos.writeBytes(buffer, count * pf.bpp/8); + zos.writeBytes(buffer, count * (pf.bpp/8)); return; } -- 2.39.5