CVE-2018-8794 (C, published 2019-02-05T20:29Z)
Description: value: rdesktop versions up to and including v1.8.3 contain an Integer Overflow that leads to an Out-Of-Bounds Write in function process_bitmap_updates() and results in a memory corruption and possibly even a remote code execution.
Severity: HIGH (CVSS v3 base score: 9.8)
Weakness: CWE-190 \u2014 Integer Overflow or Wraparound
The product performs a calculation that can
         produce an integer overflow or wraparound when the logic
         assumes that the resulting value will always be larger than
         the original value. This occurs when an integer value is
         incremented to a value that is too large to store in the
         associated representation. When this occurs, the value may
         become a very small or negative number.
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)