72 lines
2.9 KiB
Diff
72 lines
2.9 KiB
Diff
From 996356b6c65ca165ee1ea46a571c32a1dc3c3821 Mon Sep 17 00:00:00 2001
|
|
From: Pierre Ossman <ossman@cendio.se>
|
|
Date: Tue, 10 Sep 2019 15:21:03 +0200
|
|
Subject: [PATCH] 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.
|
|
---
|
|
common/rfb/PixelBuffer.cxx | 22 ++++++++++++++++++++++
|
|
1 file changed, 22 insertions(+)
|
|
|
|
diff --git a/common/rfb/PixelBuffer.cxx b/common/rfb/PixelBuffer.cxx
|
|
index ad58324..18f41f8 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;
|
|
}
|
|
@@ -337,6 +350,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_;
|