CVE-2018-20176: Out-of-bounds Read (CWE-125)
value: rdesktop versions up to and including v1.8.3 contain several Out-Of- Bounds Reads in the file secure.c that result in a Denial of Service (segfault).
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 7.5
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)