Make sure that no integer overflow could occur during scaling

1 files changed, 22 insertions, 1 deletions

diff --git a/libvncserver/scale.c b/libvncserver/scale.c
index 6f91391..d5b3f8b 100644
--- a/libvncserver/scale.c
+++ b/libvncserver/scale.c
@@ -66,6 +66,12 @@
         (double) ((int) (x)) : (double) ((int) (x) + 1) )
 #define FLOOR(x) ( (double) ((int) (x)) )
 
+static inline int pad4(int value)
+{
+    int remainder = value & 3;
+    if (!remainder) return value;
+    return value + 4 - remainder;
+}
 
 int ScaleX(rfbScreenInfoPtr from, rfbScreenInfoPtr to, int x)
 {
@@ -281,14 +287,29 @@ rfbScreenInfoPtr rfbScaledScreenAllocate(rfbClientPtr cl, int width, int height)
     ptr = malloc(sizeof(rfbScreenInfo));
     if (ptr!=NULL)
     {
+        int allocSize;
+
         /* copy *everything* (we don't use most of it, but just in case) */
         memcpy(ptr, cl->screen, sizeof(rfbScreenInfo));
+
+        /* SECURITY: make sure that no integer overflow will occur afterwards.
+         * Note: this is defensive coding, as the check should have already been
+         * performed during initial, non-scaled screen setup.
+         */
+        allocSize = pad4(width * (ptr->bitsPerPixel/8)); /* per protocol, width<2**16 and bpp<256 */
+        if (height == 0 || allocSize >= SIZE_MAX / height)
+        {
+          free(ptr);
+          return NULL; /* malloc() will allocate an incorrect buffer size - early abort */
+        }
+
+        /* Resume copy everything */
         ptr->width = width;
         ptr->height = height;
         ptr->paddedWidthInBytes = (ptr->bitsPerPixel/8)*ptr->width;
 
         /* Need to by multiples of 4 for Sparc systems */
-        ptr->paddedWidthInBytes += (ptr->paddedWidthInBytes % 4);
+        ptr->paddedWidthInBytes = pad4(ptr->paddedWidthInBytes);
 
         /* Reset the reference count to 0! */
         ptr->scaledScreenRefCount = 0;