Merge pull request #1557 from matt335672/hac425-2

Check for overflow before g_malloc() calls

common/ms-rdpbcgr.h

@@ -74,8 +74,11 @@
 #define CONNECTION_TYPE_LAN            0x06
 #define CONNECTION_TYPE_AUTODETECT     0x07
 
-/* Virtual channel options */
+/* Channel definition structure CHANNEL_DEF (2.2.1.3.4.1) */
-/* Channel Definition Structure: options (2.2.1.3.4.1) */
+/* This isn't explicitly named in MS-RDPBCGR */
+#define CHANNEL_NAME_LEN                7
+
+/* Oprions field */
 /* NOTE: XR_ prefixed to avoid conflict with FreeRDP */
 #define XR_CHANNEL_OPTION_INITIALIZED   0x80000000
 #define XR_CHANNEL_OPTION_ENCRYPT_RDP   0x40000000

sesman/chansrv/chansrv.c

@@ -39,6 +39,10 @@
 #include "xrdp_sockets.h"
 #include "audin.h"
 
+#include "ms-rdpbcgr.h"
+
+#define MAX_PATH 260
+
 static struct trans *g_lis_trans = 0;
 static struct trans *g_con_trans = 0;
 static struct trans *g_api_lis_trans = 0;
@@ -1042,9 +1046,13 @@ my_api_trans_data_in(struct trans *trans)
     int rv;
     int bytes;
     int ver;
-    int channel_name_bytes;
     struct chansrv_drdynvc_procs procs;
-    char *chan_name;
+    /*
+     * Name is limited to CHANNEL_NAME_LEN for an SVC, or MAX_PATH
+     * bytes for a DVC
+     */
+    char chan_name[MAX(CHANNEL_NAME_LEN, MAX_PATH) + 1];
+    unsigned int channel_name_bytes;
 
     //g_writeln("my_api_trans_data_in: extra_flags %d", trans->extra_flags);
     rv = 0;
@@ -1067,12 +1075,13 @@ my_api_trans_data_in(struct trans *trans)
         rv = 1;
         in_uint32_le(s, channel_name_bytes);
         //g_writeln("my_api_trans_data_in: channel_name_bytes %d", channel_name_bytes);
-        chan_name = g_new0(char, channel_name_bytes + 1);
+        if (channel_name_bytes > (sizeof(chan_name) - 1))
-        if (chan_name == NULL)
         {
             return 1;
         }
         in_uint8a(s, chan_name, channel_name_bytes);
+        chan_name[channel_name_bytes] = '\0';
+
         in_uint32_le(s, ad->chan_flags);
         //g_writeln("my_api_trans_data_in: chan_name %s chan_flags 0x%8.8x", chan_name, ad->chan_flags);
         if (ad->chan_flags == 0)
@@ -1132,7 +1141,6 @@ my_api_trans_data_in(struct trans *trans)
             //          "chan_id %d", rv, ad->chan_id);
             g_drdynvcs[ad->chan_id].xrdp_api_trans = trans;
         }
-        g_free(chan_name);
         init_stream(s, 0);
         trans->extra_flags = 2;
         trans->header_size = 0;

sesman/chansrv/smartcard_pcsc.c

@@ -586,9 +586,18 @@ int
 scard_process_list_readers(struct trans *con, struct stream *in_s)
 {
     int hContext;
-    int bytes_groups;
+    unsigned int bytes_groups;
     int cchReaders;
-    char *groups;
+    /*
+     * At the time of writing, the groups strings which can be sent
+     * over this interface are all small:-
+     *
+     * "SCard$AllReaders", "SCard$DefaultReaders", "SCard$LocalReaders" and
+     * "SCard$SystemReaders"
+     *
+     * We'll allow a bit extra in case the interface changes
+     */
+    char groups[256];
     struct pcsc_uds_client *uds_client;
     struct pcsc_context *lcontext;
     struct pcsc_list_readers *pcscListReaders;
@@ -597,8 +606,14 @@ scard_process_list_readers(struct trans *con, struct stream *in_s)
     uds_client = (struct pcsc_uds_client *) (con->callback_data);
     in_uint32_le(in_s, hContext);
     in_uint32_le(in_s, bytes_groups);
-    groups = (char *) g_malloc(bytes_groups + 1, 1);
+    if (bytes_groups > (sizeof(groups) - 1))
+    {
+        LLOGLN(0, ("scard_process_list_readers: Unreasonable string length %u",
+                   bytes_groups));
+        return 1;
+    }
     in_uint8a(in_s, groups, bytes_groups);
+    groups[bytes_groups] = '\0';
     in_uint32_le(in_s, cchReaders);
     LLOGLN(10, ("scard_process_list_readers: hContext 0x%8.8x cchReaders %d",
            hContext, cchReaders));
@@ -615,7 +630,6 @@ scard_process_list_readers(struct trans *con, struct stream *in_s)
     pcscListReaders->cchReaders = cchReaders;
     scard_send_list_readers(pcscListReaders, lcontext->context,
                             lcontext->context_bytes, groups, cchReaders, 1);
-    g_free(groups);
     return 0;
 }
 

xrdp/xrdp_bitmap.c

@@ -25,6 +25,8 @@
 #include <config_ac.h>
 #endif
 
+#include <limits.h>
+
 #include "xrdp.h"
 #include "log.h"
 
@@ -93,6 +95,30 @@ static const unsigned int g_crc_table[256] =
     (in_crc) = g_crc_table[((in_crc) ^ (in_pixel)) & 0xff] ^ ((in_crc) >> 8)
 #define CRC_END(in_crc) (in_crc) = ((in_crc) ^ 0xFFFFFFFF)
 
+/*****************************************************************************/
+/* Allocate bitmap for specified dimensions, checking for int overflow */
+static char *
+alloc_bitmap_data(int width, int height, int Bpp)
+{
+    char *result = NULL;
+    if (width > 0 && height > 0 && Bpp > 0)
+    {
+        int len = width;
+        /* g_malloc() currently takes an 'int' size */
+        if (len < INT_MAX / height)
+        {
+            len *= height;
+            if (len < INT_MAX / Bpp)
+            {
+                len *= Bpp;
+                result = (char *)malloc(len);
+            }
+        }
+    }
+
+    return result;
+}
+
 /*****************************************************************************/
 struct xrdp_bitmap *
 xrdp_bitmap_create(int width, int height, int bpp,
@@ -123,14 +149,28 @@ xrdp_bitmap_create(int width, int height, int bpp,
 
     if (self->type == WND_TYPE_BITMAP || self->type == WND_TYPE_IMAGE)
     {
-        self->data = (char *)g_malloc(width * height * Bpp, 0);
+        self->data = alloc_bitmap_data(width, height, Bpp);
+        if (self->data == NULL)
+        {
+            LLOGLN(0, ("xrdp_bitmap_create: size overflow %dx%dx%d",
+                       width, height, Bpp));
+            g_free(self);
+            return NULL;
+        }
     }
 
 #if defined(XRDP_PAINTER)
     if (self->type == WND_TYPE_SCREEN) /* noorders */
     {
         LLOGLN(0, ("xrdp_bitmap_create: noorders"));
-        self->data = (char *) g_malloc(width * height * Bpp, 0);
+        self->data = alloc_bitmap_data(width, height, Bpp);
+        if (self->data == NULL)
+        {
+            LLOGLN(0, ("xrdp_bitmap_create: size overflow %dx%dx%d",
+                       width, height, Bpp));
+            g_free(self);
+            return NULL;
+        }
     }
 #endif