CVE-2018-8796 (C, published 2019-02-05T20:29Z)
Description: value: rdesktop versions up to and including v1.8.3 contain an Out-Of-Bounds Read in function process_bitmap_updates() that results in a Denial of Service (segfault).
Severity: MEDIUM (CVSS v3 base score: 7.5)
Weakness: CWE-125 \u2014 Out-of-bounds Read
The product reads data past the end, or before the beginning, of the intended buffer.
Repository: https://github.com/rdesktop/rdesktop
Files changed:
asn.c
bitmap.c
cliprdr.c
constants.h
cssp.c
lspci.c
mcs.c
orders.c
proto.h
rdp.c
rdpdr.c
rdpsnd.c
seamless.c
secure.c
stream.h
types.h
Commit message: Malicious RDP server security fixes

This commit includes fixes for a set of 21 vulnerabilities in
rdesktop when a malicious RDP server is used.

All vulnerabilities was identified and reported by Eyal Itkin.

 * Add rdp_protocol_error function that is used in several fixes
 * Refactor of process_bitmap_updates
 * Fix possible integer overflow in s_check_rem() on 32bit arch
 * Fix memory corruption in process_bitmap_data - CVE-2018-8794
 * Fix remote code execution in process_bitmap_data - CVE-2018-8795
 * Fix remote code execution in process_plane - CVE-2018-8797
 * Fix Denial of Service in mcs_recv_connect_response - CVE-2018-20175
 * Fix Denial of Service in mcs_parse_domain_params - CVE-2018-20175
 * Fix Denial of Service in sec_parse_crypt_info - CVE-2018-20176
 * Fix Denial of Service in sec_recv - CVE-2018-20176
 * Fix minor information leak in rdpdr_process - CVE-2018-8791
 * Fix Denial of Service in cssp_read_tsrequest - CVE-2018-8792
 * Fix remote code execution in cssp_read_tsrequest - CVE-2018-8793
 * Fix Denial of Service in process_bitmap_data - CVE-2018-8796
 * Fix minor information leak in rdpsnd_process_ping - CVE-2018-8798
 * Fix Denial of Service in process_secondary_order - CVE-2018-8799
 * Fix remote code execution in in ui_clip_handle_data - CVE-2018-8800
 * Fix major information leak in ui_clip_handle_data - CVE-2018-20174
 * Fix memory corruption in rdp_in_unistr - CVE-2018-20177
 * Fix Denial of Service in process_demand_active - CVE-2018-20178
 * Fix remote code execution in lspci_process - CVE-2018-20179
 * Fix remote code execution in rdpsnddbg_process - CVE-2018-20180
 * Fix remote code execution in seamless_process - CVE-2018-20181
 * Fix remote code execution in seamless_process_line - CVE-2018-20182


Vulnerable code:
ber_parse_header(STREAM s, int tagval, int *length)
				while (collen > 0)
				while (replen > 0)
				while (collen > 0)
				while (replen > 0)
		token->end = token->p = token->data;
   Copyright (C) 2005 Matrox Graphics Inc. 
	int length;
	int length;
RD_BOOL ber_parse_header(STREAM s, int tagval, int *length);
/* Process bitmap updates */
void
process_bitmap_updates(STREAM s)
	uint16 num_updates;
	uint16 cx, cy, bpp, Bpp, compress, bufsize, size;
	int i;

	in_uint16_le(s, num_updates);
	for (i = 0; i < num_updates; i++)
	{
		in_uint16_le(s, left);
		in_uint16_le(s, top);
		in_uint16_le(s, right);
		in_uint16_le(s, bottom);
		in_uint16_le(s, width);
		in_uint16_le(s, height);
		in_uint16_le(s, bpp);
		Bpp = (bpp + 7) / 8;
		in_uint16_le(s, compress);
		in_uint16_le(s, bufsize);

		cx = right - left + 1;
		cy = bottom - top + 1;

		logger(Graphics, Debug,
		       "process_bitmap_updates(), [%d,%d,%d,%d], [%d,%d], bpp=%d, compression=%d",
		       left, top, right, bottom, width, height, Bpp, compress);

		if (!compress)
		{
			int y;
			bmpdata = (uint8 *) xmalloc(width * height * Bpp);
			for (y = 0; y < height; y++)
			{
				in_uint8a(s, &bmpdata[(height - y - 1) * (width * Bpp)],
					  width * Bpp);
			}
			ui_paint_bitmap(left, top, cx, cy, width, height, bmpdata);
			xfree(bmpdata);
			continue;
		}
		if (compress & 0x400)
		{
			size = bufsize;
		}
		else
		{
			in_uint8s(s, 2);	/* pad */
			in_uint16_le(s, size);
			in_uint8s(s, 4);	/* line_size, final_size */
		}
		in_uint8p(s, data, size);
		if (bitmap_decompress(bmpdata, width, height, data, size, Bpp))
			ui_paint_bitmap(left, top, cx, cy, width, height, bmpdata);
		else
		{
			logger(Graphics, Warning,
			       "process_bitmap_updates(), failed to decompress bitmap");
		}

#define s_check_rem(s,n)	((s)->p + n <= (s)->end)

Fixed code:
ber_parse_header(STREAM s, int tagval, uint32 *length)
				while (indexw < width && collen > 0)
				while (indexw < width && replen > 0)
				while (indexw < width && collen > 0)
				while (indexw < width && replen > 0)
	struct stream packet = *s;
	if (!s_check_rem(s, length))
	{
		rdp_protocol_error("cliprdr_process(), consume of packet from stream would overrun", &packet);
	}

/* [MS-RDPBCGR], TS_BITMAP_DATA, flags */
#define BITMAP_COMPRESSION              0x0001

	struct stream packet;
	packet = *s;

	if (!s_check_rem(s, length))
	{
		 rdp_protocol_error("cssp_read_tsrequest(), consume of version from stream would overrun",
				    &packet);
	}
		if (!s_check_rem(s, length))
		{
			rdp_protocol_error("cssp_read_tsrequest(), consume of token from stream would overrun",
					   &packet);
		}

		s_realloc(token, length);
		s_reset(token);
   Copyright (C) 2005 Matrox Graphics Inc.
   Copyright 2018 Henrik Andersson <hean01@cendio.se> for Cendio AB
	struct stream packet = *s;

	if (!s_check(s))
	{
		rdp_protocol_error("lspci_process(), stream is in unstable state", &packet);
	}
	uint32 length;
	struct stream packet = *s;

	if (!s_check_rem(s, length))
	{
		rdp_protocol_error("mcs_parse_domain_params(), consume domain params from stream would overrun", &packet);
	}

	uint32 length;
	struct stream packet;
	
	packet = *s;

	if (!s_check_rem(s, length))
	{
		rdp_protocol_error("mcs_recv_connect_response(), consume connect id from stream would overrun", &packet);
	}

	struct stream packet = *s;
	if (!s_check_rem(s, length + 7))
	{
		rdp_protocol_error("process_secondary_order(), next order pointer would overrun stream", &packet);
	}

void rdp_protocol_error(const char *message, STREAM s);
RD_BOOL ber_parse_header(STREAM s, int tagval, uint32 *length);
	struct stream packet = *s;

	if ((in_len < 0) || ((uint32)in_len >= (RD_UINT32_MAX / 2)))
	{
		logger(Protocol, Error, "rdp_in_unistr(), length of unicode data is out of bounds.");
		abort();
	}

	if (!s_check_rem(s, in_len))
	{
		rdp_protocol_error("rdp_in_unistr(), consume of unicode data from stream would overrun", &packet);
	}

	struct stream packet = *s;

	if (!s_check_rem(s, len_src_descriptor))
	{
		rdp_protocol_error("rdp_demand_active(), consume of source descriptor from stream would overrun", &packet);
	}
/* Process TS_BITMAP_DATA */
static void
process_bitmap_data(STREAM s)
	uint16 cx, cy, bpp, Bpp, flags, bufsize, size;
	
	struct stream packet = *s;
	in_uint16_le(s, left); /* destLeft */
	in_uint16_le(s, top); /* destTop */
	in_uint16_le(s, right); /* destRight */
	in_uint16_le(s, bottom); /* destBottom */
	in_uint16_le(s, width); /* width */
	in_uint16_le(s, height); /* height */
	in_uint16_le(s, bpp); /*bitsPerPixel */
	Bpp = (bpp + 7) / 8;
	in_uint16_le(s, flags); /* flags */
	in_uint16_le(s, bufsize); /* bitmapLength */
	cx = right - left + 1;
	cy = bottom - top + 1;
	/* FIXME: There are a assumtion that we do not consider in
		this code. The value of bpp is not passed to
		ui_paint_bitmap() which relies on g_server_bpp for drawing
		the bitmap data.

		Does this means that we can sanity check bpp with g_server_bpp ?
	*/

	if (Bpp == 0 || width == 0 || height == 0)
	{
        logger(Protocol, Warning, "%s(), [%d,%d,%d,%d], [%d,%d], bpp=%d, flags=%x", __func__,
				left, top, right, bottom, width, height, bpp, flags);
		rdp_protocol_error("TS_BITMAP_DATA, unsafe size of bitmap data received from server", &packet);
	}

	if ((RD_UINT32_MAX / Bpp) <= (width * height))
	{
		logger(Protocol, Warning, "%s(), [%d,%d,%d,%d], [%d,%d], bpp=%d, flags=%x", __func__,
				left, top, right, bottom, width, height, bpp, flags);
		rdp_protocol_error("TS_BITMAP_DATA, unsafe size of bitmap data received from server", &packet);
	}
 
	if (flags == 0)
	{
		/* read uncompressed bitmap data */
		int y;
		for (y = 0; y < height; y++)
			in_uint8a(s, &bmpdata[(height - y - 1) * (width * Bpp)], width * Bpp);
		
		ui_paint_bitmap(left, top, cx, cy, width, height, bmpdata);
		return;
	}

	if (flags & NO_BITMAP_COMPRESSION_HDR)
	{
		size = bufsize;
	}
	else
	{
		/* Read TS_CD_HEADER */
		in_uint8s(s, 2);        /* skip cbCompFirstRowSize (must be 0x0000) */
		in_uint16_le(s, size);  /* cbCompMainBodySize */
		in_uint8s(s, 2);        /* skip cbScanWidth */
		in_uint8s(s, 2);        /* skip cbUncompressedSize */
	}

	/* read compressed bitmap data */
	if (!s_check_rem(s, size))
	{
		rdp_protocol_error("process_bitmap_data(), consume of bitmap data from stream would overrun", &packet);
	}
	in_uint8p(s, data, size);
	bmpdata = (uint8 *) xmalloc(width * height * Bpp);
	if (bitmap_decompress(bmpdata, width, height, data, size, Bpp))
	{
		ui_paint_bitmap(left, top, cx, cy, width, height, bmpdata);
	}
	else
	{
		logger(Protocol, Warning, "%s(), failed to decompress bitmap", __func__);
	}

	xfree(bmpdata);
}

/* Process TS_UPDATE_BITMAP_DATA */
void
process_bitmap_updates(STREAM s)
{
	int i;
	uint16 num_updates;
	
	in_uint16_le(s, num_updates);   /* rectangles */

	for (i = 0; i < num_updates; i++)
	{
		process_bitmap_data(s);

/* Abort rdesktop upon protocol error

   A protocol error is defined as:

    - A value is outside specified range for example;
      bpp for a bitmap is not allowed to be greater than the
      value 32 but is represented by a byte in protocol.

*/
void
rdp_protocol_error(const char *message, STREAM s)
{
	logger(Protocol, Error, "%s(), %s", __func__, message);
	if (s)
		hexdump(s->p, s_length(s));
	exit(0);
}
	struct stream packet = *s;

				/* g_client_id is sent back to server,
				   so lets check that we actually got
				   valid data from stream to prevent
				   that we leak back data to server */
				if (!s_check(s))
				{
					rdp_protocol_error("rdpdr_process(), consume of g_client_id from stream did overrun", &packet);
				}

	struct stream packet = *in;

	if (!s_check_rem(in, 4))
	{
		rdp_protocol_error("rdpsnd_process_training(), consume of training data from stream would overrun", &packet);
	}
	if (!s_check(s))
	{
		rdp_protocol_error("rdpsnddbg_process(), stream is in unstable state", s);
	}


			if ((size_t)len >= sizeof(icon_buf))
			{
				logger(Protocol, Warning, "seamless_process_line(), icon data would overrun icon_buf");
				break;
			}
	struct stream packet = *s;

	if (!s_check(s))
	{
		rdp_protocol_error("seamless_process(), stream is in unstable state", &packet);
	}
	if (length <= 0)
		return;

	struct stream packet;
		packet = *s;
				if (!s_check_rem(s, 8)) {
					rdp_protocol_error("sec_recv(), consume fastpath signature from stream would overrun", &packet);
				}

					if (!s_check_rem(s, 8)) {
						rdp_protocol_error("sec_recv(), consume encrypt signature from stream would overrun", &packet);
					}

					if (!s_check_rem(s, 8)) {
						rdp_protocol_error("sec_recv(), consume redirect signature from stream would overrun", &packet);
					}

#define s_check_rem(s,n) (s_check(s) && (n <= (s)->end - (s)->p))
#define RD_UINT32_MAX (uint32)(-1)