File xrdp-memory-allocation-security-fix.patch of Package xrdp.31863
From 7ef01f7b0cec6e75631a22906d67d8ff44cb1a8c Mon Sep 17 00:00:00 2001
From: matt335672 <30179339+matt335672@users.noreply.github.com>
Date: Sat, 18 Apr 2020 16:56:53 +0100
Subject: [PATCH] Address memory allocation overflow security issues
---
sesman/chansrv/chansrv.c | 12 ++++++++-
sesman/chansrv/smartcard_pcsc.c | 22 ++++++++++++++---
xrdp/xrdp_bitmap.c | 44 +++++++++++++++++++++++++++++++--
3 files changed, 71 insertions(+), 7 deletions(-)
diff --git a/sesman/chansrv/chansrv.c b/sesman/chansrv/chansrv.c
index 6b1b93267..d21d5b703 100644
--- a/sesman/chansrv/chansrv.c
+++ b/sesman/chansrv/chansrv.c
@@ -39,6 +39,9 @@
#include "xrdp_sockets.h"
#include "audin.h"
+#define CHANNEL_NAME_BYTES 7
+#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,7 +1045,7 @@ my_api_trans_data_in(struct trans *trans)
int rv;
int bytes;
int ver;
- int channel_name_bytes;
+ unsigned int channel_name_bytes;
struct chansrv_drdynvc_procs procs;
char *chan_name;
@@ -1067,6 +1070,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);
+ /*
+ * Name is limited to CHANNEL_NAME_BYTES for an SVC, or MAX_PATH
+ * bytes for a DVC */
+ if (channel_name_bytes > MAX(CHANNEL_NAME_BYTES, MAX_PATH))
+ {
+ return 1;
+ }
chan_name = g_new0(char, channel_name_bytes + 1);
if (chan_name == NULL)
{
diff --git a/sesman/chansrv/smartcard_pcsc.c b/sesman/chansrv/smartcard_pcsc.c
index 51409567a..5f4516db9 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 1f5d010fe..627f73c7d 100644
--- a/xrdp/xrdp_bitmap.c
+++ b/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