|
 |
549c89 |
From 996356b6c65ca165ee1ea46a571c32a1dc3c3821 Mon Sep 17 00:00:00 2001
|
|
|
549c89 |
From: Pierre Ossman <ossman@cendio.se>
|
|
|
549c89 |
Date: Tue, 10 Sep 2019 15:21:03 +0200
|
|
|
549c89 |
Subject: [PATCH] Restrict PixelBuffer dimensions to safe values
|
|
|
549c89 |
|
|
|
549c89 |
We do a lot of calculations based on pixel coordinates and we need
|
|
|
549c89 |
to make sure they do not overflow. Restrict the maximum dimensions
|
|
|
549c89 |
we support rather than try to switch over all calculations to use
|
|
|
549c89 |
64 bit integers.
|
|
|
549c89 |
|
|
|
549c89 |
This prevents attackers from from injecting code by specifying a
|
|
|
549c89 |
huge framebuffer size and relying on the values overflowing to
|
|
|
549c89 |
access invalid areas of the heap.
|
|
|
549c89 |
|
|
|
549c89 |
This primarily affects the client which gets both the screen
|
|
|
549c89 |
dimensions and the pixel contents from the remote side. But the
|
|
|
549c89 |
server might also be affected as a client can adjust the screen
|
|
|
549c89 |
dimensions, as can applications inside the session.
|
|
|
549c89 |
|
|
|
549c89 |
Issue found by Pavel Cheremushkin from Kaspersky Lab.
|
|
|
549c89 |
---
|
|
|
549c89 |
common/rfb/PixelBuffer.cxx | 22 ++++++++++++++++++++++
|
|
|
549c89 |
1 file changed, 22 insertions(+)
|
|
|
549c89 |
|
|
|
549c89 |
diff --git a/common/rfb/PixelBuffer.cxx b/common/rfb/PixelBuffer.cxx
|
|
|
549c89 |
index ad58324..18f41f8 100644
|
|
|
549c89 |
--- a/common/rfb/PixelBuffer.cxx
|
|
|
549c89 |
+++ b/common/rfb/PixelBuffer.cxx
|
|
|
549c89 |
@@ -31,6 +31,14 @@ using namespace rdr;
|
|
|
549c89 |
|
|
|
549c89 |
static LogWriter vlog("PixelBuffer");
|
|
|
549c89 |
|
|
|
549c89 |
+// We do a lot of byte offset calculations that assume the result fits
|
|
|
549c89 |
+// inside a signed 32 bit integer. Limit the maximum size of pixel
|
|
|
549c89 |
+// buffers so that these calculations never overflow.
|
|
|
549c89 |
+
|
|
|
549c89 |
+const int maxPixelBufferWidth = 16384;
|
|
|
549c89 |
+const int maxPixelBufferHeight = 16384;
|
|
|
549c89 |
+const int maxPixelBufferStride = 16384;
|
|
|
549c89 |
+
|
|
|
549c89 |
|
|
|
549c89 |
// -=- Generic pixel buffer class
|
|
|
549c89 |
|
|
|
549c89 |
@@ -108,6 +116,11 @@ void PixelBuffer::getImage(const PixelFormat& pf, void* imageBuf,
|
|
|
549c89 |
|
|
|
549c89 |
void PixelBuffer::setSize(int width, int height)
|
|
|
549c89 |
{
|
|
|
549c89 |
+ if ((width < 0) || (width > maxPixelBufferWidth))
|
|
|
549c89 |
+ throw rfb::Exception("Invalid PixelBuffer width of %d pixels requested", width);
|
|
|
549c89 |
+ if ((height < 0) || (height > maxPixelBufferHeight))
|
|
|
549c89 |
+ throw rfb::Exception("Invalid PixelBuffer height of %d pixels requested", height);
|
|
|
549c89 |
+
|
|
|
549c89 |
width_ = width;
|
|
|
549c89 |
height_ = height;
|
|
|
549c89 |
}
|
|
|
549c89 |
@@ -337,6 +350,15 @@ const rdr::U8* FullFramePixelBuffer::getBuffer(const Rect& r, int* stride_) cons
|
|
|
549c89 |
void FullFramePixelBuffer::setBuffer(int width, int height,
|
|
|
549c89 |
rdr::U8* data_, int stride_)
|
|
|
549c89 |
{
|
|
|
549c89 |
+ if ((width < 0) || (width > maxPixelBufferWidth))
|
|
|
549c89 |
+ throw rfb::Exception("Invalid PixelBuffer width of %d pixels requested", width);
|
|
|
549c89 |
+ if ((height < 0) || (height > maxPixelBufferHeight))
|
|
|
549c89 |
+ throw rfb::Exception("Invalid PixelBuffer height of %d pixels requested", height);
|
|
|
549c89 |
+ if ((stride_ < 0) || (stride_ > maxPixelBufferStride) || (stride_ < width))
|
|
|
549c89 |
+ throw rfb::Exception("Invalid PixelBuffer stride of %d pixels requested", stride_);
|
|
|
549c89 |
+ if ((width != 0) && (height != 0) && (data_ == NULL))
|
|
|
549c89 |
+ throw rfb::Exception("PixelBuffer requested without a valid memory area");
|
|
|
549c89 |
+
|
|
|
549c89 |
ModifiablePixelBuffer::setSize(width, height);
|
|
|
549c89 |
stride = stride_;
|
|
|
549c89 |
data = data_;
|