aboutsummaryrefslogtreecommitdiffstats
path: root/common/rfb/PixelBuffer.cxx
diff options
context:
space:
mode:
authorPierre Ossman <ossman@cendio.se>2019-09-10 15:21:03 +0200
committerPierre Ossman <ossman@cendio.se>2019-11-15 10:53:41 +0100
commit996356b6c65ca165ee1ea46a571c32a1dc3c3821 (patch)
treef19ab03c6537cae21dfb551fccac4350cbcc68fd /common/rfb/PixelBuffer.cxx
parent53f913a76196c7357d4858bfbf2c33caa9181bae (diff)
downloadtigervnc-996356b6c65ca165ee1ea46a571c32a1dc3c3821.tar.gz
tigervnc-996356b6c65ca165ee1ea46a571c32a1dc3c3821.zip
Restrict PixelBuffer dimensions to safe values
We do a lot of calculations based on pixel coordinates and we need to make sure they do not overflow. Restrict the maximum dimensions we support rather than try to switch over all calculations to use 64 bit integers. This prevents attackers from from injecting code by specifying a huge framebuffer size and relying on the values overflowing to access invalid areas of the heap. This primarily affects the client which gets both the screen dimensions and the pixel contents from the remote side. But the server might also be affected as a client can adjust the screen dimensions, as can applications inside the session. Issue found by Pavel Cheremushkin from Kaspersky Lab.
Diffstat (limited to 'common/rfb/PixelBuffer.cxx')
-rw-r--r--common/rfb/PixelBuffer.cxx22
1 files changed, 22 insertions, 0 deletions
diff --git a/common/rfb/PixelBuffer.cxx b/common/rfb/PixelBuffer.cxx
index 0aa67744..fe406b96 100644
--- a/common/rfb/PixelBuffer.cxx
+++ b/common/rfb/PixelBuffer.cxx
@@ -31,6 +31,14 @@ using namespace rdr;
static LogWriter vlog("PixelBuffer");
+// We do a lot of byte offset calculations that assume the result fits
+// inside a signed 32 bit integer. Limit the maximum size of pixel
+// buffers so that these calculations never overflow.
+
+const int maxPixelBufferWidth = 16384;
+const int maxPixelBufferHeight = 16384;
+const int maxPixelBufferStride = 16384;
+
// -=- Generic pixel buffer class
@@ -108,6 +116,11 @@ void PixelBuffer::getImage(const PixelFormat& pf, void* imageBuf,
void PixelBuffer::setSize(int width, int height)
{
+ if ((width < 0) || (width > maxPixelBufferWidth))
+ throw rfb::Exception("Invalid PixelBuffer width of %d pixels requested", width);
+ if ((height < 0) || (height > maxPixelBufferHeight))
+ throw rfb::Exception("Invalid PixelBuffer height of %d pixels requested", height);
+
width_ = width;
height_ = height;
}
@@ -340,6 +353,15 @@ const rdr::U8* FullFramePixelBuffer::getBuffer(const Rect& r, int* stride_) cons
void FullFramePixelBuffer::setBuffer(int width, int height,
rdr::U8* data_, int stride_)
{
+ if ((width < 0) || (width > maxPixelBufferWidth))
+ throw rfb::Exception("Invalid PixelBuffer width of %d pixels requested", width);
+ if ((height < 0) || (height > maxPixelBufferHeight))
+ throw rfb::Exception("Invalid PixelBuffer height of %d pixels requested", height);
+ if ((stride_ < 0) || (stride_ > maxPixelBufferStride) || (stride_ < width))
+ throw rfb::Exception("Invalid PixelBuffer stride of %d pixels requested", stride_);
+ if ((width != 0) && (height != 0) && (data_ == NULL))
+ throw rfb::Exception("PixelBuffer requested without a valid memory area");
+
ModifiablePixelBuffer::setSize(width, height);
stride = stride_;
data = data_;