core: handle disconnect pdu better, avoid reading past end of buffer

3 files changed, 88 insertions, 35 deletions

diff --git a/libfreerdp-core/mcs.c b/libfreerdp-core/mcs.c
index 7837d6d..a786641 100644
--- a/libfreerdp-core/mcs.c
+++ b/libfreerdp-core/mcs.c
@@ -8,7 +8,7 @@
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *     http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -175,30 +175,46 @@ static const char* const mcs_result_enumerated[] =
 */
 
 /**
+ * Read a DomainMCSPDU header and do check.
+ * @param s stream
+ * @param domainMCSPDU DomainMCSPDU type
+ * @param length TPKT length
+ * @return
+ */
+
+static tbool mcs_read_domain_mcspdu_header_check(STREAM* s, enum DomainMCSPDU* domainMCSPDU, uint16* length)
+{
+	enum DomainMCSPDU MCSPDU;
+
+	MCSPDU = *domainMCSPDU;
+	if (!mcs_read_domain_mcspdu_header(s, domainMCSPDU, length))
+		return false;
+	if (*domainMCSPDU != MCSPDU)
+		return false;
+	return true;
+}
+
+/**
  * Read a DomainMCSPDU header.
  * @param s stream
  * @param domainMCSPDU DomainMCSPDU type
  * @param length TPKT length
  * @return
+ * do not return false on MCSPDU
  */
 
 tbool mcs_read_domain_mcspdu_header(STREAM* s, enum DomainMCSPDU* domainMCSPDU, uint16* length)
 {
 	uint8 choice;
-	enum DomainMCSPDU MCSPDU;
 
 	*length = tpkt_read_header(s);
 
 	if (tpdu_read_data(s) == 0)
 		return false;
 
-	MCSPDU = *domainMCSPDU;
 	per_read_choice(s, &choice);
 	*domainMCSPDU = (choice >> 2);
 
-	if (*domainMCSPDU != MCSPDU)
-		return false;
-
 	return true;
 }
 
@@ -562,7 +578,7 @@ tbool mcs_recv_erect_domain_request(rdpMcs* mcs, STREAM* s)
 	enum DomainMCSPDU MCSPDU;
 
 	MCSPDU = DomainMCSPDU_ErectDomainRequest;
-	if (!mcs_read_domain_mcspdu_header(s, &MCSPDU, &length))
+	if (!mcs_read_domain_mcspdu_header_check(s, &MCSPDU, &length))
 		return false;
 
 	return true;
@@ -604,7 +620,7 @@ tbool mcs_recv_attach_user_request(rdpMcs* mcs, STREAM* s)
 	enum DomainMCSPDU MCSPDU;
 
 	MCSPDU = DomainMCSPDU_AttachUserRequest;
-	if (!mcs_read_domain_mcspdu_header(s, &MCSPDU, &length))
+	if (!mcs_read_domain_mcspdu_header_check(s, &MCSPDU, &length))
 		return false;
 
 	return true;
@@ -643,7 +659,7 @@ tbool mcs_recv_attach_user_confirm(rdpMcs* mcs, STREAM* s)
 	enum DomainMCSPDU MCSPDU;
 
 	MCSPDU = DomainMCSPDU_AttachUserConfirm;
-	if (!mcs_read_domain_mcspdu_header(s, &MCSPDU, &length))
+	if (!mcs_read_domain_mcspdu_header_check(s, &MCSPDU, &length))
 		return false;
 
 	per_read_enumerated(s, &result, MCS_Result_enum_length); /* result */
@@ -690,7 +706,7 @@ tbool mcs_recv_channel_join_request(rdpMcs* mcs, STREAM* s, uint16* channel_id)
 	uint16 user_id;
 
 	MCSPDU = DomainMCSPDU_ChannelJoinRequest;
-	if (!mcs_read_domain_mcspdu_header(s, &MCSPDU, &length))
+	if (!mcs_read_domain_mcspdu_header_check(s, &MCSPDU, &length))
 		return false;
 
 	if (!per_read_integer16(s, &user_id, MCS_BASE_CHANNEL_ID))
@@ -742,7 +758,7 @@ tbool mcs_recv_channel_join_confirm(rdpMcs* mcs, STREAM* s, uint16* channel_id)
 	enum DomainMCSPDU MCSPDU;
 
 	MCSPDU = DomainMCSPDU_ChannelJoinConfirm;
-	if (!mcs_read_domain_mcspdu_header(s, &MCSPDU, &length))
+	if (!mcs_read_domain_mcspdu_header_check(s, &MCSPDU, &length))
 		return false;
 
 	per_read_enumerated(s, &result, MCS_Result_enum_length); /* result */
diff --git a/libfreerdp-core/per.c b/libfreerdp-core/per.c
index 4365cb2..f85514c 100644
--- a/libfreerdp-core/per.c
+++ b/libfreerdp-core/per.c
@@ -8,7 +8,7 @@
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *     http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -30,10 +30,18 @@ tbool per_read_length(STREAM* s, uint16* length)
 {
 	uint8 byte;
 
+	if (stream_get_left(s) < 1)
+	{
+		return false;
+	}
 	stream_read_uint8(s, byte);
 
 	if (byte & 0x80)
 	{
+		if (stream_get_left(s) < 1)
+		{
+			return false;
+		}
 		byte &= ~(0x80);
 		*length = (byte << 8);
 		stream_read_uint8(s, byte);
@@ -250,12 +258,11 @@ void per_write_integer16(STREAM* s, uint16 integer, uint16 min)
 
 tbool per_read_enumerated(STREAM* s, uint8* enumerated, uint8 count)
 {
-	stream_read_uint8(s, *enumerated);
-
-	/* check that enumerated value falls within expected range */
-	if (*enumerated + 1 > count)
+	if (stream_get_left(s) < 1)
+	{
 		return false;
-
+	}
+	stream_read_uint8(s, *enumerated);
 	return true;
 }
 
diff --git a/libfreerdp-core/rdp.c b/libfreerdp-core/rdp.c
index 621abf3..8a5f88b 100644
--- a/libfreerdp-core/rdp.c
+++ b/libfreerdp-core/rdp.c
@@ -8,7 +8,7 @@
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *     http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -23,6 +23,10 @@
 #include "per.h"
 #include "redirection.h"
 
+#define LLOG_LEVEL 1
+#define LLOGLN(_level, _args) \
+  do { if (_level < LLOG_LEVEL) { printf _args ; printf("\n"); } } while (0)
+
 static const char* const DATA_PDU_TYPE_STRINGS[] =
 {
 		"", "", /* 0x00 - 0x01 */
@@ -219,34 +223,50 @@ STREAM* rdp_data_pdu_init(rdpRdp* rdp)
 
 tbool rdp_read_header(rdpRdp* rdp, STREAM* s, uint16* length, uint16* channel_id)
 {
+	uint8 reason;
 	uint16 initiator;
 	enum DomainMCSPDU MCSPDU;
 
 	MCSPDU = (rdp->settings->server_mode) ? DomainMCSPDU_SendDataRequest : DomainMCSPDU_SendDataIndication;
-	mcs_read_domain_mcspdu_header(s, &MCSPDU, length);
-
+	if (!mcs_read_domain_mcspdu_header(s, &MCSPDU, length))
+	{
+		LLOGLN(0, ("rdp_read_header: mcs_read_domain_mcspdu_header failed"));
+		return false;
+	}
 	if (*length - 8 > stream_get_left(s))
+	{
+		LLOGLN(0, ("rdp_read_header: parse error"));
 		return false;
-
+	}
 	if (MCSPDU == DomainMCSPDU_DisconnectProviderUltimatum)
 	{
-		uint8 reason;
-
-		(void) per_read_enumerated(s, &reason, 0);
-
+		if (!per_read_enumerated(s, &reason, 0))
+		{
+			LLOGLN(0, ("rdp_read_header: per_read_enumerated failed"));
+			return false;
+		}
 		rdp->disconnect = true;
-
+		*channel_id = MCS_GLOBAL_CHANNEL_ID;
 		return true;
 	}
-
+	if (stream_get_left(s) < 5)
+	{
+		LLOGLN(0, ("rdp_read_header: parse error"));
+		return false;
+	}
 	per_read_integer16(s, &initiator, MCS_BASE_CHANNEL_ID); /* initiator (UserId) */
 	per_read_integer16(s, channel_id, 0); /* channelId */
 	stream_seek(s, 1); /* dataPriority + Segmentation (0x70) */
-	per_read_length(s, length); /* userData (OCTET_STRING) */
-
+	if (!per_read_length(s, length)) /* userData (OCTET_STRING) */
+	{
+		LLOGLN(0, ("rdp_read_header: per_read_length failed"));
+		return false;
+	}
 	if (*length > stream_get_left(s))
+	{
 		return false;
-
+		LLOGLN(0, ("rdp_read_header: parse error"));
+	}
 	return true;
 }
 
@@ -695,23 +715,33 @@ static tbool rdp_recv_tpkt_pdu(rdpRdp* rdp, STREAM* s)
 
 	if (!rdp_read_header(rdp, s, &length, &channelId))
 	{
-		printf("Incorrect RDP header.\n");
+		LLOGLN(0, ("Incorrect RDP header."));
+		return false;
+	}
+
+	LLOGLN(10, ("rdp_recv_tpkt_pdu length %d", length));
+
+	if (rdp->disconnect)
+	{
+		LLOGLN(0, ("rdp_recv_tpkt_pdu: disconnect"));
 		return false;
 	}
 
 	if (rdp->settings->encryption)
 	{
 		rdp_read_security_header(s, &securityFlags);
-		if (securityFlags & (SEC_ENCRYPT|SEC_REDIRECTION_PKT))
+		LLOGLN(10, ("rdp_recv_tpkt_pdu: securityFlags 0x%8.8x", securityFlags));
+		if (securityFlags & (SEC_ENCRYPT | SEC_REDIRECTION_PKT))
 		{
 			if (!rdp_decrypt(rdp, s, length - 4, securityFlags))
 			{
-				printf("rdp_decrypt failed\n");
+				LLOGLN(0, ("rdp_decrypt failed"));
 				return false;
 			}
 		}
 		if (securityFlags & SEC_REDIRECTION_PKT)
 		{
+			LLOGLN(0, ("rdp_recv_tpkt_pdu: got SEC_REDIRECTION_PKT securityFlags 0x%8.8x", securityFlags));
 			/*
 			 * [MS-RDPBCGR] 2.2.13.2.1
 			 *  - no share control header, nor the 2 byte pad
@@ -741,7 +771,7 @@ static tbool rdp_recv_tpkt_pdu(rdpRdp* rdp, STREAM* s)
 				case PDU_TYPE_DATA:
 					if (!rdp_recv_data_pdu(rdp, s))
 					{
-						printf("rdp_recv_data_pdu failed\n");
+						LLOGLN(0, ("rdp_recv_data_pdu failed"));
 						return false;
 					}
 					break;
@@ -756,7 +786,7 @@ static tbool rdp_recv_tpkt_pdu(rdpRdp* rdp, STREAM* s)
 					break;
 
 				default:
-					printf("incorrect PDU type: 0x%04X\n", pduType);
+					LLOGLN(0, ("incorrect PDU type: 0x%04X", pduType));
 					break;
 			}
 			stream_set_mark(s, nextp);