File U_tigervnc-prevent-invalid-PixelBuffer-accesses.patch of Package tigervnc.openSUSE_Leap_42.2_Update
Git-commit: d5ab3e376f00e4237a90732f1dd0b5c71e0693de
Patch-Mainline: Upstream
References: bnc#1019274
Author: Pierre Ossman <ossman@cendio.se>
Subject: [PATCH] Prevent invalid PixelBuffer accesses
Signed-off-by: Michal Srb <msrb@suse.com>
There has been multiple attempts at tricking decoders to exceed
the boundaries of the active pixel buffer. Add extra checks to
prevent such invalid access.
diff --git a/common/rfb/PixelBuffer.cxx b/common/rfb/PixelBuffer.cxx
index 7f3df6c..f444aa0 100644
--- a/common/rfb/PixelBuffer.cxx
+++ b/common/rfb/PixelBuffer.cxx
@@ -42,17 +42,32 @@ PixelBuffer::~PixelBuffer() {}
void
-PixelBuffer::getImage(void* imageBuf, const Rect& r, int outStride) const {
+PixelBuffer::getImage(void* imageBuf, const Rect& r, int outStride) const
+{
int inStride;
- const U8* data = getBuffer(r, &inStride);
- // We assume that the specified rectangle is pre-clipped to the buffer
- int bytesPerPixel = format.bpp/8;
- int inBytesPerRow = inStride * bytesPerPixel;
- if (!outStride) outStride = r.width();
- int outBytesPerRow = outStride * bytesPerPixel;
- int bytesPerMemCpy = r.width() * bytesPerPixel;
- U8* imageBufPos = (U8*)imageBuf;
- const U8* end = data + (inBytesPerRow * r.height());
+ const U8* data;
+ int bytesPerPixel, inBytesPerRow, outBytesPerRow, bytesPerMemCpy;
+ U8* imageBufPos;
+ const U8* end;
+
+ if (!r.enclosed_by(getRect()))
+ throw rfb::Exception("Source rect %dx%d at %d,%d exceeds framebuffer %dx%d",
+ r.width(), r.height(),
+ r.tl.x, r.tl.y, width_, height_);
+
+ data = getBuffer(r, &inStride);
+
+ bytesPerPixel = format.bpp/8;
+ inBytesPerRow = inStride * bytesPerPixel;
+
+ if (!outStride)
+ outStride = r.width();
+ outBytesPerRow = outStride * bytesPerPixel;
+ bytesPerMemCpy = r.width() * bytesPerPixel;
+
+ imageBufPos = (U8*)imageBuf;
+ end = data + (inBytesPerRow * r.height());
+
while (data < end) {
memcpy(imageBufPos, data, bytesPerMemCpy);
imageBufPos += outBytesPerRow;
@@ -71,6 +86,11 @@ void PixelBuffer::getImage(const PixelFormat& pf, void* imageBuf,
return;
}
+ if (!r.enclosed_by(getRect()))
+ throw rfb::Exception("Source rect %dx%d at %d,%d exceeds framebuffer %dx%d",
+ r.width(), r.height(),
+ r.tl.x, r.tl.y, width_, height_);
+
if (stride == 0)
stride = r.width();
@@ -101,26 +121,19 @@ void ModifiablePixelBuffer::fillRect(const Rect& r, const void* pix)
int stride;
U8 *buf;
int w, h, b;
- Rect drect;
-
- drect = r;
- if (!drect.enclosed_by(getRect())) {
- vlog.error("Destination rect %dx%d at %d,%d exceeds framebuffer %dx%d",
- drect.width(), drect.height(), drect.tl.x, drect.tl.y, width_, height_);
- drect = drect.intersect(getRect());
- }
- if (drect.is_empty())
- return;
+ if (!r.enclosed_by(getRect()))
+ throw rfb::Exception("Destination rect %dx%d at %d,%d exceeds framebuffer %dx%d",
+ r.width(), r.height(), r.tl.x, r.tl.y, width_, height_);
- w = drect.width();
- h = drect.height();
+ w = r.width();
+ h = r.height();
b = format.bpp/8;
if (h == 0)
return;
- buf = getBufferRW(drect, &stride);
+ buf = getBufferRW(r, &stride);
if (b == 1) {
while (h--) {
@@ -147,44 +160,70 @@ void ModifiablePixelBuffer::fillRect(const Rect& r, const void* pix)
}
}
- commitBufferRW(drect);
+ commitBufferRW(r);
}
void ModifiablePixelBuffer::imageRect(const Rect& r,
const void* pixels, int srcStride)
{
- int bytesPerPixel = getPF().bpp/8;
+ U8* dest;
int destStride;
- U8* dest = getBufferRW(r, &destStride);
- int bytesPerDestRow = bytesPerPixel * destStride;
- if (!srcStride) srcStride = r.width();
- int bytesPerSrcRow = bytesPerPixel * srcStride;
- int bytesPerFill = bytesPerPixel * r.width();
- const U8* src = (const U8*)pixels;
- U8* end = dest + (bytesPerDestRow * r.height());
+ int bytesPerPixel, bytesPerDestRow, bytesPerSrcRow, bytesPerFill;
+ const U8* src;
+ U8* end;
+
+ if (!r.enclosed_by(getRect()))
+ throw rfb::Exception("Destination rect %dx%d at %d,%d exceeds framebuffer %dx%d",
+ r.width(), r.height(),
+ r.tl.x, r.tl.y, width_, height_);
+
+ bytesPerPixel = getPF().bpp/8;
+
+ dest = getBufferRW(r, &destStride);
+
+ bytesPerDestRow = bytesPerPixel * destStride;
+
+ if (!srcStride)
+ srcStride = r.width();
+ bytesPerSrcRow = bytesPerPixel * srcStride;
+ bytesPerFill = bytesPerPixel * r.width();
+
+ src = (const U8*)pixels;
+ end = dest + (bytesPerDestRow * r.height());
+
while (dest < end) {
memcpy(dest, src, bytesPerFill);
dest += bytesPerDestRow;
src += bytesPerSrcRow;
}
+
commitBufferRW(r);
}
void ModifiablePixelBuffer::maskRect(const Rect& r,
const void* pixels, const void* mask_)
{
- Rect cr = getRect().intersect(r);
- if (cr.is_empty()) return;
int stride;
- U8* data = getBufferRW(cr, &stride);
- U8* mask = (U8*) mask_;
- int w = cr.width();
- int h = cr.height();
- int bpp = getPF().bpp;
- int pixelStride = r.width();
- int maskStride = (r.width() + 7) / 8;
-
- Point offset = Point(cr.tl.x-r.tl.x, cr.tl.y-r.tl.y);
+ U8* data;
+ U8* mask;
+ int w, h, bpp, pixelStride, maskStride;
+ Point offset;
+
+ if (!r.enclosed_by(getRect()))
+ throw rfb::Exception("Destination rect %dx%d at %d,%d exceeds framebuffer %dx%d",
+ r.width(), r.height(),
+ r.tl.x, r.tl.y, width_, height_);
+
+ data = getBufferRW(r, &stride);
+ mask = (U8*) mask_;
+
+ w = r.width();
+ h = r.height();
+ bpp = getPF().bpp;
+ pixelStride = r.width();
+ maskStride = (r.width() + 7) / 8;
+
+ offset = Point(r.tl.x-r.tl.x, r.tl.y-r.tl.y);
mask += offset.y * maskStride;
for (int y = 0; y < h; y++) {
int cy = offset.y + y;
@@ -209,23 +248,32 @@ void ModifiablePixelBuffer::maskRect(const Rect& r,
mask += maskStride;
}
- commitBufferRW(cr);
+ commitBufferRW(r);
}
void ModifiablePixelBuffer::maskRect(const Rect& r,
Pixel pixel, const void* mask_)
{
- Rect cr = getRect().intersect(r);
- if (cr.is_empty()) return;
int stride;
- U8* data = getBufferRW(cr, &stride);
- U8* mask = (U8*) mask_;
- int w = cr.width();
- int h = cr.height();
- int bpp = getPF().bpp;
- int maskStride = (r.width() + 7) / 8;
-
- Point offset = Point(cr.tl.x-r.tl.x, cr.tl.y-r.tl.y);
+ U8* data;
+ U8* mask;
+ int w, h, bpp, maskStride;
+ Point offset;
+
+ if (!r.enclosed_by(getRect()))
+ throw rfb::Exception("Destination rect %dx%d at %d,%d exceeds framebuffer %dx%d",
+ r.width(), r.height(),
+ r.tl.x, r.tl.y, width_, height_);
+
+ data = getBufferRW(r, &stride);
+ mask = (U8*) mask_;
+
+ w = r.width();
+ h = r.height();
+ bpp = getPF().bpp;
+ maskStride = (r.width() + 7) / 8;
+
+ offset = Point(r.tl.x-r.tl.x, r.tl.y-r.tl.y);
mask += offset.y * maskStride;
for (int y = 0; y < h; y++) {
for (int x = 0; x < w; x++) {
@@ -249,7 +297,7 @@ void ModifiablePixelBuffer::maskRect(const Rect& r,
mask += maskStride;
}
- commitBufferRW(cr);
+ commitBufferRW(r);
}
void ModifiablePixelBuffer::copyRect(const Rect &rect,
@@ -262,26 +310,16 @@ void ModifiablePixelBuffer::copyRect(const Rect &rect,
Rect drect, srect;
drect = rect;
- if (!drect.enclosed_by(getRect())) {
- vlog.error("Destination rect %dx%d at %d,%d exceeds framebuffer %dx%d",
- drect.width(), drect.height(), drect.tl.x, drect.tl.y, width_, height_);
- drect = drect.intersect(getRect());
- }
-
- if (drect.is_empty())
- return;
+ if (!drect.enclosed_by(getRect()))
+ throw rfb::Exception("Destination rect %dx%d at %d,%d exceeds framebuffer %dx%d",
+ drect.width(), drect.height(),
+ drect.tl.x, drect.tl.y, width_, height_);
srect = drect.translate(move_by_delta.negate());
- if (!srect.enclosed_by(getRect())) {
- vlog.error("Source rect %dx%d at %d,%d exceeds framebuffer %dx%d",
- srect.width(), srect.height(), srect.tl.x, srect.tl.y, width_, height_);
- srect = srect.intersect(getRect());
- // Need to readjust the destination now that the area has changed
- drect = srect.translate(move_by_delta);
- }
-
- if (srect.is_empty())
- return;
+ if (!srect.enclosed_by(getRect()))
+ throw rfb::Exception("Source rect %dx%d at %d,%d exceeds framebuffer %dx%d",
+ srect.width(), srect.height(),
+ srect.tl.x, srect.tl.y, width_, height_);
srcData = getBuffer(srect, &srcStride);
dstData = getBufferRW(drect, &dstStride);
@@ -331,6 +369,11 @@ void ModifiablePixelBuffer::imageRect(const PixelFormat& pf, const Rect &dest,
rdr::U8* dstBuffer;
int dstStride;
+ if (!dest.enclosed_by(getRect()))
+ throw rfb::Exception("Destination rect %dx%d at %d,%d exceeds framebuffer %dx%d",
+ dest.width(), dest.height(),
+ dest.tl.x, dest.tl.y, width_, height_);
+
if (stride == 0)
stride = dest.width();
@@ -355,6 +398,11 @@ FullFramePixelBuffer::~FullFramePixelBuffer() {}
rdr::U8* FullFramePixelBuffer::getBufferRW(const Rect& r, int* stride_)
{
+ if (!r.enclosed_by(getRect()))
+ throw rfb::Exception("Pixel buffer request %dx%d at %d,%d exceeds framebuffer %dx%d",
+ r.width(), r.height(),
+ r.tl.x, r.tl.y, width_, height_);
+
*stride_ = stride;
return &data[(r.tl.x + (r.tl.y * stride)) * format.bpp/8];
}
@@ -365,6 +413,11 @@ void FullFramePixelBuffer::commitBufferRW(const Rect& r)
const rdr::U8* FullFramePixelBuffer::getBuffer(const Rect& r, int* stride_) const
{
+ if (!r.enclosed_by(getRect()))
+ throw rfb::Exception("Pixel buffer request %dx%d at %d,%d exceeds framebuffer %dx%d",
+ r.width(), r.height(),
+ r.tl.x, r.tl.y, width_, height_);
+
*stride_ = stride;
return &data[(r.tl.x + (r.tl.y * stride)) * format.bpp/8];
}
