diff --git a/common/ms-rdpbcgr.h b/common/ms-rdpbcgr.h index ad81d0df..03e03c39 100644 --- a/common/ms-rdpbcgr.h +++ b/common/ms-rdpbcgr.h @@ -74,8 +74,11 @@ #define CONNECTION_TYPE_LAN 0x06 #define CONNECTION_TYPE_AUTODETECT 0x07 -/* Virtual channel options */ -/* Channel Definition Structure: options (2.2.1.3.4.1) */ +/* Channel definition structure CHANNEL_DEF (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 diff --git a/sesman/chansrv/chansrv.c b/sesman/chansrv/chansrv.c index 6b1b9326..d7ef1869 100644 --- a/sesman/chansrv/chansrv.c +++ b/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 (chan_name == NULL) + if (channel_name_bytes > (sizeof(chan_name) - 1)) { 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; diff --git a/sesman/chansrv/smartcard_pcsc.c b/sesman/chansrv/smartcard_pcsc.c index 51409567..5f4516db 100644 --- a/sesman/chansrv/smartcard_pcsc.c +++ b/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; } diff --git a/xrdp/xrdp_bitmap.c b/xrdp/xrdp_bitmap.c index 1f5d010f..627f73c7 100644 --- a/xrdp/xrdp_bitmap.c +++ b/xrdp/xrdp_bitmap.c @@ -25,6 +25,8 @@ #include #endif +#include + #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