CVE-2020-11048: Out-of-bounds Read (CWE-125)
value: In FreeRDP after 1.0 and before 2.0.0, there is an out-of-bounds read. It only allows to abort a session. No data extraction is possible. This has been fixed in 2.0.0.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 2.2
Published: 2020-05-07T20:15Z
Repository: https://github.com/FreeRDP/FreeRDP
Language: C
Files changed: libfreerdp/core/rdp.c
Commit date: 2020-04-02 17:27:53 +0200
Commit message: Fixed #6007: Boundary checks in rdp_read_flow_control_pdu

Diff: diff --git a/libfreerdp/core/rdp.c b/libfreerdp/core/rdp.c
index 169c673f1b..84e03d63ba 100644
--- a/libfreerdp/core/rdp.c
+++ b/libfreerdp/core/rdp.c
@@ -100,11 +100,11 @@ const char* DATA_PDU_TYPE_STRINGS[80] = {
 	"?",
 	"?",
 	"?" /* 0x41 - 0x46 */
 };
 
-static void rdp_read_flow_control_pdu(wStream* s, UINT16* type);
+static BOOL rdp_read_flow_control_pdu(wStream* s, UINT16* type);
 static void rdp_write_share_control_header(wStream* s, UINT16 length, UINT16 type,
                                            UINT16 channel_id);
 static void rdp_write_share_data_header(wStream* s, UINT16 length, BYTE type, UINT32 share_id);
 
 /**
@@ -143,33 +143,37 @@ void rdp_write_security_header(wStream* s, UINT16 flags)
 	Stream_Write_UINT16(s, 0);     /* flagsHi (unused) */
 }
 
 BOOL rdp_read_share_control_header(wStream* s, UINT16* length, UINT16* type, UINT16* channel_id)
 {
+	UINT16 len;
 	if (Stream_GetRemainingLength(s) < 2)
 		return FALSE;
 
 	/* Share Control Header */
-	Stream_Read_UINT16(s, *length); /* totalLength */
+	Stream_Read_UINT16(s, len); /* totalLength */
+
+	*length = len;
 
 	/* If length is 0x8000 then we actually got a flow control PDU that we should ignore
 	 http://msdn.microsoft.com/en-us/library/cc240576.aspx */
-	if (*length == 0x8000)
+	if (len == 0x8000)
 	{
-		rdp_read_flow_control_pdu(s, type);
+		if (!rdp_read_flow_control_pdu(s, type))
+			return FALSE;
 		*channel_id = 0;
 		*length = 8; /* Flow control PDU is 8 bytes */
 		return TRUE;
 	}
 
-	if (((size_t)*length - 2) > Stream_GetRemainingLength(s))
+	if ((len < 4) || ((len - 2) > Stream_GetRemainingLength(s)))
 		return FALSE;
 
 	Stream_Read_UINT16(s, *type); /* pduType */
 	*type &= 0x0F;                /* type is in the 4 least significant bits */
 
-	if (*length > 4)
+	if (len > 4)
 		Stream_Read_UINT16(s, *channel_id); /* pduSource */
 	else
 		*channel_id = 0; /* Windows XP can send such short DEACTIVATE_ALL PDUs. */
 
 	return TRUE;
@@ -1114,26 +1118,31 @@ int rdp_recv_out_of_sequence_pdu(rdpRdp* rdp, wStream* s)
 	{
 		return -1;
 	}
 }
 
-void rdp_read_flow_control_pdu(wStream* s, UINT16* type)
+BOOL rdp_read_flow_control_pdu(wStream* s, UINT16* type)
 {
 	/*
 	 * Read flow control PDU - documented in FlowPDU section in T.128
 	 * http://www.itu.int/rec/T-REC-T.128-199802-S/en
 	 * The specification for the PDU has pad8bits listed BEFORE pduTypeFlow.
 	 * However, so far pad8bits has always been observed to arrive AFTER pduTypeFlow.
 	 * Switched the order of these two fields to match this observation.
 	 */
 	UINT8 pduType;
+	if (!type)
+		return FALSE;
+	if (Stream_GetRemainingLength(s) < 6)
+		return FALSE;
 	Stream_Read_UINT8(s, pduType); /* pduTypeFlow */
 	*type = pduType;
 	Stream_Seek_UINT8(s);  /* pad8bits */
 	Stream_Seek_UINT8(s);  /* flowIdentifier */
 	Stream_Seek_UINT8(s);  /* flowNumber */
 	Stream_Seek_UINT16(s); /* pduSource */
+	return TRUE;
 }
 
 /**
  * Decrypt an RDP packet.\n
  * @param rdp RDP module

Vulnerable code: static void rdp_read_flow_control_pdu(wStream* s, UINT16* type);
	Stream_Read_UINT16(s, *length); /* totalLength */
	if (*length == 0x8000)
		rdp_read_flow_control_pdu(s, type);
	if (((size_t)*length - 2) > Stream_GetRemainingLength(s))
	if (*length > 4)
void rdp_read_flow_control_pdu(wStream* s, UINT16* type)
Fixed code: static BOOL rdp_read_flow_control_pdu(wStream* s, UINT16* type);
	UINT16 len;
	Stream_Read_UINT16(s, len); /* totalLength */

	*length = len;
	if (len == 0x8000)
		if (!rdp_read_flow_control_pdu(s, type))
			return FALSE;
	if ((len < 4) || ((len - 2) > Stream_GetRemainingLength(s)))
	if (len > 4)
BOOL rdp_read_flow_control_pdu(wStream* s, UINT16* type)
	if (!type)
		return FALSE;
	if (Stream_GetRemainingLength(s) < 6)
		return FALSE;
	return TRUE;