Limit size of cursor accepted by client.

Width and height of a cursor are received as U16 from network. Accepting full range of U16 values can cause integer overflows in multiple places.

The worst is probably VLA in CMsgReader::readSetXCursor:
  rdr::U8 buf[width*height*4];

The width*height*4 can be too big to fit on stack or it can overflow into negative numbers. Both cases are undefined behaviour. Following writes to buf can overwrite other data on stack.

diff --git a/common/rfb/CMsgReader.cxx b/common/rfb/CMsgReader.cxx
index 7233fbd76c9cd0f714ca0e740a8e48382ccb006e..9abe3f244ca471cf190638eafafb7b8f5f10e6f2 100644 (file)
--- a/common/rfb/CMsgReader.cxx
+++ b/common/rfb/CMsgReader.cxx
@@ -202,6 +202,9 @@ void CMsgReader::readRect(const Rect& r, int encoding)
 
 void CMsgReader::readSetXCursor(int width, int height, const Point& hotspot)
 {
+  if (width > maxCursorSize || height > maxCursorSize)
+    throw Exception("Too big cursor");
+
   rdr::U8 pr, pg, pb;
   rdr::U8 sr, sg, sb;
   int data_len = ((width+7)/8) * height;
@@ -257,6 +260,9 @@ void CMsgReader::readSetXCursor(int width, int height, const Point& hotspot)
 
 void CMsgReader::readSetCursor(int width, int height, const Point& hotspot)
 {
+  if (width > maxCursorSize || height > maxCursorSize)
+    throw Exception("Too big cursor");
+
   int data_len = width * height * (handler->cp.pf().bpp/8);
   int mask_len = ((width+7)/8) * height;
   rdr::U8Array data(data_len);
@@ -295,6 +301,9 @@ void CMsgReader::readSetCursor(int width, int height, const Point& hotspot)
 
 void CMsgReader::readSetCursorWithAlpha(int width, int height, const Point& hotspot)
 {
+  if (width > maxCursorSize || height > maxCursorSize)
+    throw Exception("Too big cursor");
+
   int encoding;
 
   const PixelFormat rgbaPF(32, 32, false, true, 255, 255, 255, 16, 8, 0);

diff --git a/common/rfb/CMsgReader.h b/common/rfb/CMsgReader.h
index ff73414ef2f07063d8a6eb18579453072167de9b..7b52033feedeff357318b40c6ae4ceb40571cee8 100644 (file)
--- a/common/rfb/CMsgReader.h
+++ b/common/rfb/CMsgReader.h
@@ -69,6 +69,8 @@ namespace rfb {
     CMsgHandler* handler;
     rdr::InStream* is;
     int nUpdateRectsLeft;
+
+    static const int maxCursorSize = 256;
   };
 }
 #endif