CVE-2018-20182: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: rdesktop versions up to and including v1.8.3 contain a Buffer Overflow over the global variables in the function seamless_process_line() that results in memory corruption and probably even a remote code execution.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 9.8
Published: 2019-03-15T18:29Z
Repository: https://github.com/rdesktop/rdesktop
Language: C
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 date: 2019-01-16 12:28:44 +0100
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

Diff: diff --git a/asn.c b/asn.c
index 26b0e9b..1b9fea4 100644
--- a/asn.c
+++ b/asn.c
@@ -20,11 +20,11 @@
 #include "rdesktop.h"
 
 
 /* Parse an ASN.1 BER header */
 RD_BOOL
-ber_parse_header(STREAM s, int tagval, int *length)
+ber_parse_header(STREAM s, int tagval, uint32 *length)
 {
 	int tag, len;
 
 	if (tagval > 0xff)
 	{
diff --git a/bitmap.c b/bitmap.c
index 775622c..55c768c 100644
--- a/bitmap.c
+++ b/bitmap.c
@@ -792,19 +792,19 @@ process_plane(uint8 * in, int width, int height, uint8 * out, int size)
 				if ((revcode <= 47) && (revcode >= 16))
 				{
 					replen = revcode;
 					collen = 0;
 				}
-				while (collen > 0)
+				while (indexw < width && collen > 0)
 				{
 					color = CVAL(in);
 					*out = color;
 					out += 4;
 					indexw++;
 					collen--;
 				}
-				while (replen > 0)
+				while (indexw < width && replen > 0)
 				{
 					*out = color;
 					out += 4;
 					indexw++;
 					replen--;
@@ -822,11 +822,11 @@ process_plane(uint8 * in, int width, int height, uint8 * out, int size)
 				if ((revcode <= 47) && (revcode >= 16))
 				{
 					replen = revcode;
 					collen = 0;
 				}
-				while (collen > 0)
+				while (indexw < width && collen > 0)
 				{
 					x = CVAL(in);
 					if (x & 1)
 					{
 						x = x >> 1;
@@ -842,11 +842,11 @@ process_plane(uint8 * in, int width, int height, uint8 * out, int size)
 					*out = x;
 					out += 4;
 					indexw++;
 					collen--;
 				}
-				while (replen > 0)
+				while (indexw < width && replen > 0)
 				{
 					x = last_line[indexw * 4] + color;
 					*out = x;
 					out += 4;
 					indexw++;
diff --git a/cliprdr.c b/cliprdr.c
index 3f68213..76eb98d 100644
--- a/cliprdr.c
+++ b/cliprdr.c
@@ -116,19 +116,25 @@ static void
 cliprdr_process(STREAM s)
 {
 	uint16 type, status;
 	uint32 length, format;
 	uint8 *data;
+	struct stream packet = *s;
 
 	in_uint16_le(s, type);
 	in_uint16_le(s, status);
 	in_uint32_le(s, length);
 	data = s->p;
 
 	logger(Clipboard, Debug, "cliprdr_process(), type=%d, status=%d, length=%d", type, status,
 	       length);
 
+	if (!s_check_rem(s, length))
+	{
+		rdp_protocol_error("cliprdr_process(), consume of packet from stream would overrun", &packet);
+	}
+
 	if (status == CLIPRDR_ERROR)
 	{
 		switch (type)
 		{
 			case CLIPRDR_FORMAT_ACK:
diff --git a/constants.h b/constants.h
index 7dfc2e3..6301502 100644
--- a/constants.h
+++ b/constants.h
@@ -749,10 +749,13 @@ enum RDP_DESKTOP_ORIENTATION
 #define LONG_CREDENTIALS_SUPPORTED	0x0004
 #define AUTORECONNECT_SUPPORTED		0x0008
 #define ENC_SALTED_CHECKSUM		0x0010
 #define NO_BITMAP_COMPRESSION_HDR	0x0400
 
+/* [MS-RDPBCGR], TS_BITMAP_DATA, flags */
+#define BITMAP_COMPRESSION              0x0001
+
 /* orderFlags, [MS-RDPBCGR] 2.2.7.1.3 */
 #define NEGOTIATEORDERSUPPORT	0x0002
 #define ZEROBOUNDSDELTASSUPPORT 0x0008
 #define COLORINDEXSUPPORT	0x0020
 #define SOLIDPATTERNBRUSHONLY	0x0040
diff --git a/cssp.c b/cssp.c
index 33544c3..6b2d0f7 100644
--- a/cssp.c
+++ b/cssp.c
@@ -593,10 +593,11 @@ RD_BOOL
 cssp_read_tsrequest(STREAM token, STREAM pubkey)
 {
 	STREAM s;
 	int length;
 	int tagval;
+	struct stream packet;
 
 	s = tcp_recv(NULL, 4);
 
 	if (s == NULL)
 		return False;
@@ -620,20 +621,27 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey)
 	else
 		return False;
 
 	// receive the remainings of message
 	s = tcp_recv(s, length);
+	packet = *s;
 
 	// parse the response and into nego token
 	if (!ber_in_header(s, &tagval, &length) ||
 	    tagval != (BER_TAG_SEQUENCE | BER_TAG_CONSTRUCTED))
 		return False;
 
 	// version [0]
 	if (!ber_in_header(s, &tagval, &length) ||
 	    tagval != (BER_TAG_CTXT_SPECIFIC | BER_TAG_CONSTRUCTED | 0))
 		return False;
+
+	if (!s_check_rem(s, length))
+	{
+		 rdp_protocol_error("cssp_read_tsrequest(), consume of version from stream would overrun",
+				    &packet);
+	}
 	in_uint8s(s, length);
 
 	// negoToken [1]
 	if (token)
 	{
@@ -651,11 +659,18 @@ cssp_read_tsrequest(STREAM token, STREAM pubkey)
 			return False;
 
 		if (!ber_in_header(s, &tagval, &length) || tagval != BER_TAG_OCTET_STRING)
 			return False;
 
-		token->end = token->p = token->data;
+		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);
 		out_uint8p(token, s->p, length);
 		s_mark_end(token);
 	}
 
 	// pubKey [3]
diff --git a/lspci.c b/lspci.c
index c4c8ce6..2d17161 100644
--- a/lspci.c
+++ b/lspci.c
@@ -1,9 +1,10 @@
 /*  -*- c-basic-offset: 8 -*-
    rdesktop: A Remote Desktop Protocol client.
    Support for the Matrox "lspci" channel
-   Copyright (C) 2005 Matrox Graphics Inc. 
+   Copyright (C) 2005 Matrox Graphics Inc.
+   Copyright 2018 Henrik Andersson <hean01@cendio.se> for Cendio AB
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
    the Free Software Foundation, either version 3 of the License, or
    (at your option) any later version.
@@ -132,10 +133,16 @@ static void
 lspci_process(STREAM s)
 {
 	unsigned int pkglen;
 	static char *rest = NULL;
 	char *buf;
+	struct stream packet = *s;
+
+	if (!s_check(s))
+	{
+		rdp_protocol_error("lspci_process(), stream is in unstable state", &packet);
+	}
 
 	pkglen = s->end - s->p;
 	/* str_handle_lines requires null terminated strings */
 	buf = xmalloc(pkglen + 1);
 	STRNCPY(buf, (char *) s->p, pkglen + 1);
diff --git a/mcs.c b/mcs.c
index 10b73a1..0f2360e 100644
--- a/mcs.c
+++ b/mcs.c
@@ -43,13 +43,20 @@ mcs_out_domain_params(STREAM s, int max_channels, int max_users, int max_tokens,
 
 /* Parse a DOMAIN_PARAMS structure (ASN.1 BER) */
 static RD_BOOL
 mcs_parse_domain_params(STREAM s)
 {
-	int length;
+	uint32 length;
+	struct stream packet = *s;
 
 	ber_parse_header(s, MCS_TAG_DOMAIN_PARAMS, &length);
+
+	if (!s_check_rem(s, length))
+	{
+		rdp_protocol_error("mcs_parse_domain_params(), consume domain params from stream would overrun", &packet);
+	}
+
 	in_uint8s(s, length);
 
 	return s_check(s);
 }
 
@@ -87,20 +94,23 @@ mcs_send_connect_initial(STREAM mcs_data)
 static RD_BOOL
 mcs_recv_connect_response(STREAM mcs_data)
 {
 	UNUSED(mcs_data);
 	uint8 result;
-	int length;
+	uint32 length;
 	STREAM s;
+	struct stream packet;
 	RD_BOOL is_fastpath;
 	uint8 fastpath_hdr;
 
 	logger(Protocol, Debug, "%s()", __func__);
 	s = iso_recv(&is_fastpath, &fastpath_hdr);
 
 	if (s == NULL)
 		return False;
+	
+	packet = *s;
 
 	ber_parse_header(s, MCS_CONNECT_RESPONSE, &length);
 
 	ber_parse_header(s, BER_TAG_RESULT, &length);
 	in_uint8(s, result);
@@ -110,10 +120,16 @@ mcs_recv_connect_response(STREAM mcs_data)
 		return False;
 	}
 
 	ber_parse_header(s, BER_TAG_INTEGER, &length);
 	in_uint8s(s, length);	/* connect id */
+
+	if (!s_check_rem(s, length))
+	{
+		rdp_protocol_error("mcs_recv_connect_response(), consume connect id from stream would overrun", &packet);
+	}
+
 	mcs_parse_domain_params(s);
 
 	ber_parse_header(s, BER_TAG_OCTET_STRING, &length);
 
 	sec_process_mcs_data(s);
diff --git a/orders.c b/orders.c
index ba9cfa1..5e34770 100644
--- a/orders.c
+++ b/orders.c
@@ -1257,15 +1257,21 @@ process_secondary_order(STREAM s)
 	 * so a signed integer must be used. */
 	uint16 length;
 	uint16 flags;
 	uint8 type;
 	uint8 *next_order;
+	struct stream packet = *s;
 
 	in_uint16_le(s, length);
 	in_uint16_le(s, flags);	/* used by bmpcache2 */
 	in_uint8(s, type);
 
+	if (!s_check_rem(s, length + 7))
+	{
+		rdp_protocol_error("process_secondary_order(), next order pointer would overrun stream", &packet);
+	}
+
 	next_order = s->p + (sint16) length + 7;
 
 	switch (type)
 	{
 		case RDP_ORDER_RAW_BMPCACHE:
diff --git a/proto.h b/proto.h
index 73449dd..0d6a6c9 100644
--- a/proto.h
+++ b/proto.h
@@ -162,10 +162,11 @@ void rdp_main_loop(RD_BOOL * deactivated, uint32 * ext_disc_reason);
 RD_BOOL rdp_loop(RD_BOOL * deactivated, uint32 * ext_disc_reason);
 RD_BOOL rdp_connect(char *server, uint32 flags, char *domain, char *password, char *command,
 		    char *directory, RD_BOOL reconnect);
 void rdp_reset_state(void);
 void rdp_disconnect(void);
+void rdp_protocol_error(const char *message, STREAM s);
 /* rdpdr.c */
 int get_device_index(RD_NTHANDLE handle);
 void convert_to_unix_filename(char *filename);
 void rdpdr_send_completion(uint32 device, uint32 id, uint32 status, uint32 result, uint8 * buffer,
 			   uint32 length);
@@ -222,11 +223,11 @@ RD_BOOL tcp_tls_get_server_pubkey(STREAM s);
 void tcp_run_ui(RD_BOOL run);
 
 /* asn.c */
 RD_BOOL ber_in_header(STREAM s, int *tagval, int *length);
 void ber_out_header(STREAM s, int tagval, int length);
-RD_BOOL ber_parse_header(STREAM s, int tagval, int *length);
+RD_BOOL ber_parse_header(STREAM s, int tagval, uint32 *length);
 void ber_out_integer(STREAM s, int value);
 void ber_out_sequence(STREAM s, STREAM contents);
 
 /* xclip.c */
 void ui_clip_format_announce(uint8 * data, uint32 length);
diff --git a/rdp.c b/rdp.c
index f98f49c..1654e58 100644
--- a/rdp.c
+++ b/rdp.c
@@ -283,10 +283,23 @@ rdp_in_unistr(STREAM s, int in_len, char **string, uint32 * str_size)
 {
 	static iconv_t icv_utf16_to_local;
 	size_t ibl, obl;
 	char *pin, *pout;
 
+	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);
+	}
+
 	// if not already open
 	if (!icv_utf16_to_local)
 	{
 		icv_utf16_to_local = iconv_open(g_codepage, WINDOWS_CODEPAGE);
 		if (icv_utf16_to_local == (iconv_t) - 1)
@@ -1209,17 +1222,23 @@ rdp_process_server_caps(STREAM s, uint16 length)
 static void
 process_demand_active(STREAM s)
 {
 	uint8 type;
 	uint16 len_src_descriptor, len_combined_caps;
+	struct stream packet = *s;
 
 	/* at this point we need to ensure that we have ui created */
 	rd_create_ui();
 
 	in_uint32_le(s, g_rdp_shareid);
 	in_uint16_le(s, len_src_descriptor);
 	in_uint16_le(s, len_combined_caps);
+
+	if (!s_check_rem(s, len_src_descriptor))
+	{
+		rdp_protocol_error("rdp_demand_active(), consume of source descriptor from stream would overrun", &packet);
+	}
 	in_uint8s(s, len_src_descriptor);
 
 	logger(Protocol, Debug, "process_demand_active(), shareid=0x%x", g_rdp_shareid);
 
 	rdp_process_server_caps(s, len_combined_caps);
@@ -1388,82 +1407,117 @@ process_pointer_pdu(STREAM s)
 			logger(Protocol, Warning,
 			       "process_pointer_pdu(), unhandled message type 0x%x", message_type);
 	}
 }
 
-/* Process bitmap updates */
-void
-process_bitmap_updates(STREAM s)
+/* Process TS_BITMAP_DATA */
+static void
+process_bitmap_data(STREAM s)
 {
-	uint16 num_updates;
 	uint16 left, top, right, bottom, width, height;
-	uint16 cx, cy, bpp, Bpp, compress, bufsize, size;
+	uint16 cx, cy, bpp, Bpp, flags, bufsize, size;
 	uint8 *data, *bmpdata;
-	int i;
-
+	
 	logger(Protocol, Debug, "%s()", __func__);
 
-	in_uint16_le(s, num_updates);
+	struct stream packet = *s;
 
-	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;
-		}
+	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;
 
-		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);
+	/* 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;
 		bmpdata = (uint8 *) xmalloc(width * height * Bpp);
-		if (bitmap_decompress(bmpdata, width, height, data, size, Bpp))
+		for (y = 0; y < height; y++)
 		{
-			ui_paint_bitmap(left, top, cx, cy, width, height, bmpdata);
+			in_uint8a(s, &bmpdata[(height - y - 1) * (width * Bpp)], width * Bpp);
 		}
-		else
-		{
-			logger(Graphics, Warning,
-			       "process_bitmap_updates(), failed to decompress bitmap");
-		}
-
+		
+		ui_paint_bitmap(left, top, cx, cy, width, height, bmpdata);
 		xfree(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);
 	}
 }
 
 /* Process a palette update */
 void
@@ -2011,5 +2065,23 @@ void
 rdp_disconnect(void)
 {
 	logger(Protocol, Debug, "%s()", __func__);
 	sec_disconnect();
 }
+
+/* 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);
+}
diff --git a/rdpdr.c b/rdpdr.c
index e8f5431..0abcacd 100644
--- a/rdpdr.c
+++ b/rdpdr.c
@@ -852,10 +852,11 @@ rdpdr_process(STREAM s)
 {
 	uint32 handle;
 	uint16 vmin;
 	uint16 component;
 	uint16 pakid;
+	struct stream packet = *s;
 
 	logger(Protocol, Debug, "rdpdr_process()");
 	/* hexdump(s->p, s->end - s->p); */
 
 	in_uint16(s, component);
@@ -871,12 +872,22 @@ rdpdr_process(STREAM s)
 
 			case PAKID_CORE_SERVER_ANNOUNCE:
 				/* DR_CORE_SERVER_ANNOUNCE_REQ */
 				in_uint8s(s, 2);	/* skip versionMajor */
 				in_uint16_le(s, vmin);	/* VersionMinor */
+
 				in_uint32_le(s, g_client_id);	/* ClientID */
 
+				/* 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);
+				}
+
 				/* The RDP client is responsibility to provide a random client id
 				   if server version is < 12 */
 				if (vmin < 0x000c)
 					g_client_id = 0x815ed39d;	/* IP address (use 127.0.0.1) 0x815ed39d */
 				g_epoch++;
diff --git a/rdpsnd.c b/rdpsnd.c
index 71a3bf2..40881b7 100644
--- a/rdpsnd.c
+++ b/rdpsnd.c
@@ -267,10 +267,16 @@ static void
 rdpsnd_process_training(STREAM in)
 {
 	uint16 tick;
 	uint16 packsize;
 	STREAM out;
+	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);
+	}
 
 	in_uint16_le(in, tick);
 	in_uint16_le(in, packsize);
 
 	logger(Sound, Debug, "rdpsnd_process_training(), tick=0x%04x", (unsigned) tick);
@@ -443,10 +449,15 @@ rdpsnddbg_process(STREAM s)
 {
 	unsigned int pkglen;
 	static char *rest = NULL;
 	char *buf;
 
+	if (!s_check(s))
+	{
+		rdp_protocol_error("rdpsnddbg_process(), stream is in unstable state", s);
+	}
+
 	pkglen = s->end - s->p;
 	/* str_handle_lines requires null terminated strings */
 	buf = (char *) xmalloc(pkglen + 1);
 	STRNCPY(buf, (char *) s->p, pkglen + 1);
 
diff --git a/seamless.c b/seamless.c
index 8fde8dd..1935074 100644
--- a/seamless.c
+++ b/seamless.c
@@ -166,10 +166,16 @@ seamless_process_line(const char *line, void *data)
 			byte[1] = *tok8;
 			tok8++;
 
 			icon_buf[len] = strtol(byte, NULL, 16);
 			len++;
+
+			if ((size_t)len >= sizeof(icon_buf))
+			{
+				logger(Protocol, Warning, "seamless_process_line(), icon data would overrun icon_buf");
+				break;
+			}
 		}
 
 		ui_seamless_seticon(id, tok5, width, height, chunk, icon_buf, len);
 	}
 	else if (!strcmp("DELICON", tok1))
@@ -368,10 +374,16 @@ seamless_line_handler(const char *line, void *data)
 static void
 seamless_process(STREAM s)
 {
 	unsigned int pkglen;
 	char *buf;
+	struct stream packet = *s;
+
+	if (!s_check(s))
+	{
+		rdp_protocol_error("seamless_process(), stream is in unstable state", &packet);
+	}
 
 	pkglen = s->end - s->p;
 	/* str_handle_lines requires null terminated strings */
 	buf = xmalloc(pkglen + 1);
 	STRNCPY(buf, (char *) s->p, pkglen + 1);
diff --git a/secure.c b/secure.c
index 48d9d01..f548a4f 100644
--- a/secure.c
+++ b/secure.c
@@ -294,10 +294,13 @@ sec_encrypt(uint8 * data, int length)
 
 /* Decrypt data using RC4 */
 void
 sec_decrypt(uint8 * data, int length)
 {
+	if (length <= 0)
+		return;
+
 	if (g_sec_decrypt_use_count == 4096)
 	{
 		sec_update(g_sec_decrypt_key, g_sec_decrypt_update_key);
 		rdssl_rc4_set_key(&g_rc4_decrypt_key, g_sec_decrypt_key, g_rc4_key_len);
 		g_sec_decrypt_use_count = 0;
@@ -846,21 +849,27 @@ sec_recv(RD_BOOL * is_fastpath)
 {
 	uint8 fastpath_hdr, fastpath_flags;
 	uint16 sec_flags;
 	uint16 channel;
 	STREAM s;
+	struct stream packet;
 
 	while ((s = mcs_recv(&channel, is_fastpath, &fastpath_hdr)) != NULL)
 	{
+		packet = *s;
 		if (*is_fastpath == True)
 		{
 			/* If fastpath packet is encrypted, read data
 			   signature and decrypt */
 			/* FIXME: extracting flags from hdr could be made less obscure */
 			fastpath_flags = (fastpath_hdr & 0xC0) >> 6;
 			if (fastpath_flags & FASTPATH_OUTPUT_ENCRYPTED)
 			{
+				if (!s_check_rem(s, 8)) {
+					rdp_protocol_error("sec_recv(), consume fastpath signature from stream would overrun", &packet);
+				}
+
 				in_uint8s(s, 8);	/* signature */
 				sec_decrypt(s->p, s->end - s->p);
 			}
 			return s;
 		}
@@ -873,10 +882,14 @@ sec_recv(RD_BOOL * is_fastpath)
 
 			if (g_encryption)
 			{
 				if (sec_flags & SEC_ENCRYPT)
 				{
+					if (!s_check_rem(s, 8)) {
+						rdp_protocol_error("sec_recv(), consume encrypt signature from stream would overrun", &packet);
+					}
+
 					in_uint8s(s, 8);	/* signature */
 					sec_decrypt(s->p, s->end - s->p);
 				}
 
 				if (sec_flags & SEC_LICENSE_PKT)
@@ -887,10 +900,14 @@ sec_recv(RD_BOOL * is_fastpath)
 
 				if (sec_flags & SEC_REDIRECTION_PKT)
 				{
 					uint8 swapbyte;
 
+					if (!s_check_rem(s, 8)) {
+						rdp_protocol_error("sec_recv(), consume redirect signature from stream would overrun", &packet);
+					}
+
 					in_uint8s(s, 8);	/* signature */
 					sec_decrypt(s->p, s->end - s->p);
 
 					/* Check for a redirect packet, starts with 00 04 */
 					if (s->p[0] == 0 && s->p[1] == 4)
diff --git a/stream.h b/stream.h
index ccc125d..a9196e5 100644
--- a/stream.h
+++ b/stream.h
@@ -52,11 +52,11 @@ size_t in_ansi_string(STREAM s, char *string, size_t len);
 
 #define s_push_layer(s,h,n)	{ (s)->h = (s)->p; (s)->p += n; }
 #define s_pop_layer(s,h)	(s)->p = (s)->h;
 #define s_mark_end(s)		(s)->end = (s)->p;
 #define s_check(s)		((s)->p <= (s)->end)
-#define s_check_rem(s,n)	((s)->p + n <= (s)->end)
+#define s_check_rem(s,n) (s_check(s) && (n <= (s)->end - (s)->p))
 #define s_check_end(s)		((s)->p == (s)->end)
 #define s_length(s)		((s)->end - (s)->data)
 #define s_left(s)               ((s)->size - ((s)->p - (s)->data))
 
 #if defined(L_ENDIAN) && !defined(NEED_ALIGN)
diff --git a/types.h b/types.h
index b139b86..4556ab4 100644
--- a/types.h
+++ b/types.h
@@ -41,10 +41,12 @@ typedef signed char sint8;
 typedef unsigned short uint16;
 typedef signed short sint16;
 typedef unsigned int uint32;
 typedef signed int sint32;
 
+#define RD_UINT32_MAX (uint32)(-1)
+
 typedef void *RD_HBITMAP;
 typedef void *RD_HGLYPH;
 typedef void *RD_HCOLOURMAP;
 typedef void *RD_HCURSOR;
 

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)