CVE-2020-11089: Out-of-bounds Read (CWE-125)
value: In FreeRDP before 2.1.0, there is an out-of-bound read in irp functions (parallel_process_irp_create, serial_process_irp_create, drive_process_irp_write, printer_process_irp_write, rdpei_recv_pdu, serial_process_irp_write). This has been fixed in 2.1.0.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.5
Published: 2020-05-29T20:15Z
Repository: https://github.com/FreeRDP/FreeRDP
Language: C
Files changed: channels/drive/client/drive_main.c, channels/printer/client/printer_main.c, channels/rdpei/client/rdpei_main.c, channels/serial/client/serial_main.c, libfreerdp/core/gateway/rdg.c, libfreerdp/core/nego.c
Commit date: 2020-05-06 13:31:57 +0200
Commit message: Fixed oob read in irp_write and similar

Diff: diff --git a/channels/drive/client/drive_main.c b/channels/drive/client/drive_main.c
index 861fad507d..1b54225225 100644
--- a/channels/drive/client/drive_main.c
+++ b/channels/drive/client/drive_main.c
@@ -329,20 +329,24 @@ static UINT drive_process_irp_read(DRIVE_DEVICE* drive, IRP* irp)
 static UINT drive_process_irp_write(DRIVE_DEVICE* drive, IRP* irp)
 {
 	DRIVE_FILE* file;
 	UINT32 Length;
 	UINT64 Offset;
+	void* ptr;
 
 	if (!drive || !irp || !irp->input || !irp->output || !irp->Complete)
 		return ERROR_INVALID_PARAMETER;
 
 	if (Stream_GetRemainingLength(irp->input) < 32)
 		return ERROR_INVALID_DATA;
 
 	Stream_Read_UINT32(irp->input, Length);
 	Stream_Read_UINT64(irp->input, Offset);
 	Stream_Seek(irp->input, 20); /* Padding */
+	ptr = Stream_Pointer(irp->input);
+	if (!Stream_SafeSeek(irp->input, Length))
+		return ERROR_INVALID_DATA;
 	file = drive_get_file_by_id(drive, irp->FileId);
 
 	if (!file)
 	{
 		irp->IoStatus = STATUS_UNSUCCESSFUL;
@@ -351,11 +355,11 @@ static UINT drive_process_irp_write(DRIVE_DEVICE* drive, IRP* irp)
 	else if (!drive_file_seek(file, Offset))
 	{
 		irp->IoStatus = drive_map_windows_err(GetLastError());
 		Length = 0;
 	}
-	else if (!drive_file_write(file, Stream_Pointer(irp->input), Length))
+	else if (!drive_file_write(file, ptr, Length))
 	{
 		irp->IoStatus = drive_map_windows_err(GetLastError());
 		Length = 0;
 	}
 
diff --git a/channels/printer/client/printer_main.c b/channels/printer/client/printer_main.c
index 42bcd3fbf4..39a474999a 100644
--- a/channels/printer/client/printer_main.c
+++ b/channels/printer/client/printer_main.c
@@ -476,25 +476,31 @@ static UINT printer_process_irp_write(PRINTER_DEVICE* printer_dev, IRP* irp)
 {
 	UINT32 Length;
 	UINT64 Offset;
 	rdpPrintJob* printjob = NULL;
 	UINT error = CHANNEL_RC_OK;
+	void* ptr;
+
+	if (Stream_GetRemainingLength(irp->input) < 32)
+		return ERROR_INVALID_DATA;
 	Stream_Read_UINT32(irp->input, Length);
 	Stream_Read_UINT64(irp->input, Offset);
 	Stream_Seek(irp->input, 20); /* Padding */
-
+	ptr = Stream_Pointer(irp->input);
+	if (!Stream_SafeSeek(irp->input, Length))
+		return ERROR_INVALID_DATA;
 	if (printer_dev->printer)
 		printjob = printer_dev->printer->FindPrintJob(printer_dev->printer, irp->FileId);
 
 	if (!printjob)
 	{
 		irp->IoStatus = STATUS_UNSUCCESSFUL;
 		Length = 0;
 	}
 	else
 	{
-		error = printjob->Write(printjob, Stream_Pointer(irp->input), Length);
+		error = printjob->Write(printjob, ptr, Length);
 	}
 
 	if (error)
 	{
 		WLog_ERR(TAG, "printjob->Write failed with error %" PRIu32 "!", error);
diff --git a/channels/rdpei/client/rdpei_main.c b/channels/rdpei/client/rdpei_main.c
index 2e72bd4498..7dd65211d1 100644
--- a/channels/rdpei/client/rdpei_main.c
+++ b/channels/rdpei/client/rdpei_main.c
@@ -443,10 +443,13 @@ static UINT rdpei_recv_resume_touch_pdu(RDPEI_CHANNEL_CALLBACK* callback, wStrea
 static UINT rdpei_recv_pdu(RDPEI_CHANNEL_CALLBACK* callback, wStream* s)
 {
 	UINT16 eventId;
 	UINT32 pduLength;
 	UINT error;
+	if (Stream_GetRemainingLength(s) < 6)
+		return ERROR_INVALID_DATA;
+
 	Stream_Read_UINT16(s, eventId);   /* eventId (2 bytes) */
 	Stream_Read_UINT32(s, pduLength); /* pduLength (4 bytes) */
 #ifdef WITH_DEBUG_RDPEI
 	WLog_DBG(TAG, "rdpei_recv_pdu: eventId: %" PRIu16 " (%s) length: %" PRIu32 "", eventId,
 	         rdpei_eventid_string(eventId), pduLength);
diff --git a/channels/serial/client/serial_main.c b/channels/serial/client/serial_main.c
index cac534c8de..afe67b4d31 100644
--- a/channels/serial/client/serial_main.c
+++ b/channels/serial/client/serial_main.c
@@ -140,14 +140,13 @@ static UINT serial_process_irp_create(SERIAL_DEVICE* serial, IRP* irp)
 	Stream_Read_UINT32(irp->input, SharedAccess);      /* SharedAccess (4 bytes) */
 	Stream_Read_UINT32(irp->input, CreateDisposition); /* CreateDisposition (4 bytes) */
 	Stream_Seek_UINT32(irp->input);                    /* CreateOptions (4 bytes) */
 	Stream_Read_UINT32(irp->input, PathLength);        /* PathLength (4 bytes) */
 
-	if (Stream_GetRemainingLength(irp->input) < PathLength)
+	if (!Stream_SafeSeek(irp->input, PathLength)) /* Path (variable) */
 		return ERROR_INVALID_DATA;
 
-	Stream_Seek(irp->input, PathLength); /* Path (variable) */
 	assert(PathLength == 0);             /* MS-RDPESP 2.2.2.2 */
 #ifndef _WIN32
 	/* Windows 2012 server sends on a first call :
 	 *     DesiredAccess     = 0x00100080: SYNCHRONIZE | FILE_READ_ATTRIBUTES
 	 *     SharedAccess      = 0x00000007: FILE_SHARE_DELETE | FILE_SHARE_WRITE | FILE_SHARE_READ
@@ -298,29 +297,35 @@ error_handle:
 
 static UINT serial_process_irp_write(SERIAL_DEVICE* serial, IRP* irp)
 {
 	UINT32 Length;
 	UINT64 Offset;
+	void* ptr;
 	DWORD nbWritten = 0;
 
 	if (Stream_GetRemainingLength(irp->input) < 32)
 		return ERROR_INVALID_DATA;
 
 	Stream_Read_UINT32(irp->input, Length); /* Length (4 bytes) */
 	Stream_Read_UINT64(irp->input, Offset); /* Offset (8 bytes) */
-	Stream_Seek(irp->input, 20);            /* Padding (20 bytes) */
+	if (!Stream_SafeSeek(irp->input, 20))   /* Padding (20 bytes) */
+		return ERROR_INVALID_DATA;
+
 	/* MS-RDPESP 3.2.5.1.5: The Offset field is ignored
 	 * assert(Offset == 0);
 	 *
 	 * Using a serial printer, noticed though this field could be
 	 * set.
 	 */
 	WLog_Print(serial->log, WLOG_DEBUG, "writing %" PRIu32 " bytes to %s", Length,
 	           serial->device.name);
 
+	ptr = Stream_Pointer(irp->input);
+	if (!Stream_SafeSeek(irp->input, Length))
+		return ERROR_INVALID_DATA;
 	/* FIXME: CommWriteFile to be replaced by WriteFile */
-	if (CommWriteFile(serial->hComm, Stream_Pointer(irp->input), Length, &nbWritten, NULL))
+	if (CommWriteFile(serial->hComm, ptr, Length, &nbWritten, NULL))
 	{
 		irp->IoStatus = STATUS_SUCCESS;
 	}
 	else
 	{
diff --git a/libfreerdp/core/gateway/rdg.c b/libfreerdp/core/gateway/rdg.c
index d8f17c20bc..6ea9e4f06d 100644
--- a/libfreerdp/core/gateway/rdg.c
+++ b/libfreerdp/core/gateway/rdg.c
@@ -302,11 +302,12 @@ static wStream* rdg_receive_packet(rdpRdg* rdg)
 	}
 
 	Stream_Seek(s, 4);
 	Stream_Read_UINT32(s, packetLength);
 
-	if ((packetLength > INT_MAX) || !Stream_EnsureCapacity(s, packetLength))
+	if ((packetLength > INT_MAX) || !Stream_EnsureCapacity(s, packetLength) ||
+	    (packetLength < header))
 	{
 		Stream_Free(s, TRUE);
 		return NULL;
 	}
 
diff --git a/libfreerdp/core/nego.c b/libfreerdp/core/nego.c
index 543c2e54f0..7cb5094644 100644
--- a/libfreerdp/core/nego.c
+++ b/libfreerdp/core/nego.c
@@ -89,13 +89,13 @@ static BOOL nego_transport_connect(rdpNego* nego);
 static BOOL nego_transport_disconnect(rdpNego* nego);
 static BOOL nego_security_connect(rdpNego* nego);
 static BOOL nego_send_preconnection_pdu(rdpNego* nego);
 static BOOL nego_recv_response(rdpNego* nego);
 static void nego_send(rdpNego* nego);
-static void nego_process_negotiation_request(rdpNego* nego, wStream* s);
-static void nego_process_negotiation_response(rdpNego* nego, wStream* s);
-static void nego_process_negotiation_failure(rdpNego* nego, wStream* s);
+static BOOL nego_process_negotiation_request(rdpNego* nego, wStream* s);
+static BOOL nego_process_negotiation_response(rdpNego* nego, wStream* s);
+static BOOL nego_process_negotiation_failure(rdpNego* nego, wStream* s);
 
 /**
  * Negotiate protocol security and connect.
  * @param nego
  * @return
@@ -616,11 +616,12 @@ int nego_recv(rdpTransport* transport, wStream* s, void* extra)
 		Stream_Read_UINT8(s, type); /* Type */
 
 		switch (type)
 		{
 			case TYPE_RDP_NEG_RSP:
-				nego_process_negotiation_response(nego, s);
+				if (!nego_process_negotiation_response(nego, s))
+					return -1;
 				WLog_DBG(TAG, "selected_protocol: %" PRIu32 "", nego->SelectedProtocol);
 
 				/* enhanced security selected ? */
 
 				if (nego->SelectedProtocol)
@@ -643,11 +644,12 @@ int nego_recv(rdpTransport* transport, wStream* s, void* extra)
 				}
 
 				break;
 
 			case TYPE_RDP_NEG_FAILURE:
-				nego_process_negotiation_failure(nego, s);
+				if (!nego_process_negotiation_failure(nego, s))
+					return -1;
 				break;
 		}
 	}
 	else if (li == 6)
 	{
@@ -793,11 +795,12 @@ BOOL nego_read_request(rdpNego* nego, wStream* s)
 		{
 			WLog_ERR(TAG, "Incorrect negotiation request type %" PRIu8 "", type);
 			return FALSE;
 		}
 
-		nego_process_negotiation_request(nego, s);
+		if (!nego_process_negotiation_request(nego, s))
+			return FALSE;
 	}
 
 	return tpkt_ensure_stream_consumed(s, length);
 }
 
@@ -916,57 +919,64 @@ fail:
  * Process Negotiation Request from Connection Request message.
  * @param nego
  * @param s
  */
 
-void nego_process_negotiation_request(rdpNego* nego, wStream* s)
+BOOL nego_process_negotiation_request(rdpNego* nego, wStream* s)
 {
 	BYTE flags;
 	UINT16 length;
+
+	if (Stream_GetRemainingLength(s) < 7)
+		return FALSE;
 	Stream_Read_UINT8(s, flags);
 	Stream_Read_UINT16(s, length);
 	Stream_Read_UINT32(s, nego->RequestedProtocols);
 	WLog_DBG(TAG, "RDP_NEG_REQ: RequestedProtocol: 0x%08" PRIX32 "", nego->RequestedProtocols);
 	nego->state = NEGO_STATE_FINAL;
+	return TRUE;
 }
 
 /**
  * Process Negotiation Response from Connection Confirm message.
  * @param nego
  * @param s
  */
 
-void nego_process_negotiation_response(rdpNego* nego, wStream* s)
+BOOL nego_process_negotiation_response(rdpNego* nego, wStream* s)
 {
 	UINT16 length;
 	WLog_DBG(TAG, "RDP_NEG_RSP");
 
 	if (Stream_GetRemainingLength(s) < 7)
 	{
 		WLog_ERR(TAG, "Invalid RDP_NEG_RSP");
 		nego->state = NEGO_STATE_FAIL;
-		return;
+		return FALSE;
 	}
 
 	Stream_Read_UINT8(s, nego->flags);
 	Stream_Read_UINT16(s, length);
 	Stream_Read_UINT32(s, nego->SelectedProtocol);
 	nego->state = NEGO_STATE_FINAL;
+	return TRUE;
 }
 
 /**
  * Process Negotiation Failure from Connection Confirm message.
  * @param nego
  * @param s
  */
 
-void nego_process_negotiation_failure(rdpNego* nego, wStream* s)
+BOOL nego_process_negotiation_failure(rdpNego* nego, wStream* s)
 {
 	BYTE flags;
 	UINT16 length;
 	UINT32 failureCode;
 	WLog_DBG(TAG, "RDP_NEG_FAILURE");
+	if (Stream_GetRemainingLength(s) < 7)
+		return FALSE;
 	Stream_Read_UINT8(s, flags);
 	Stream_Read_UINT16(s, length);
 	Stream_Read_UINT32(s, failureCode);
 
 	switch (failureCode)
@@ -997,10 +1007,11 @@ void nego_process_negotiation_failure(rdpNego* nego, wStream* s)
 			WLog_ERR(TAG, "Error: Unknown protocol security error %" PRIu32 "", failureCode);
 			break;
 	}
 
 	nego->state = NEGO_STATE_FAIL;
+	return TRUE;
 }
 
 /**
  * Send RDP Negotiation Response (RDP_NEG_RSP).\n
  * @param nego

Vulnerable code: 	else if (!drive_file_write(file, Stream_Pointer(irp->input), Length))

		error = printjob->Write(printjob, Stream_Pointer(irp->input), Length);
	if (Stream_GetRemainingLength(irp->input) < PathLength)
	Stream_Seek(irp->input, PathLength); /* Path (variable) */
	Stream_Seek(irp->input, 20);            /* Padding (20 bytes) */
	if (CommWriteFile(serial->hComm, Stream_Pointer(irp->input), Length, &nbWritten, NULL))
	if ((packetLength > INT_MAX) || !Stream_EnsureCapacity(s, packetLength))
static void nego_process_negotiation_request(rdpNego* nego, wStream* s);
static void nego_process_negotiation_response(rdpNego* nego, wStream* s);
static void nego_process_negotiation_failure(rdpNego* nego, wStream* s);
				nego_process_negotiation_response(nego, s);
				nego_process_negotiation_failure(nego, s);
		nego_process_negotiation_request(nego, s);
void nego_process_negotiation_request(rdpNego* nego, wStream* s)
void nego_process_negotiation_response(rdpNego* nego, wStream* s)
		return;
void nego_process_negotiation_failure(rdpNego* nego, wStream* s)
Fixed code: 	void* ptr;
	ptr = Stream_Pointer(irp->input);
	if (!Stream_SafeSeek(irp->input, Length))
		return ERROR_INVALID_DATA;
	else if (!drive_file_write(file, ptr, Length))
	void* ptr;

	if (Stream_GetRemainingLength(irp->input) < 32)
		return ERROR_INVALID_DATA;
	ptr = Stream_Pointer(irp->input);
	if (!Stream_SafeSeek(irp->input, Length))
		return ERROR_INVALID_DATA;
		error = printjob->Write(printjob, ptr, Length);
	if (Stream_GetRemainingLength(s) < 6)
		return ERROR_INVALID_DATA;

	if (!Stream_SafeSeek(irp->input, PathLength)) /* Path (variable) */
	void* ptr;
	if (!Stream_SafeSeek(irp->input, 20))   /* Padding (20 bytes) */
		return ERROR_INVALID_DATA;

	ptr = Stream_Pointer(irp->input);
	if (!Stream_SafeSeek(irp->input, Length))
		return ERROR_INVALID_DATA;
	if (CommWriteFile(serial->hComm, ptr, Length, &nbWritten, NULL))
	if ((packetLength > INT_MAX) || !Stream_EnsureCapacity(s, packetLength) ||
	    (packetLength < header))
static BOOL nego_process_negotiation_request(rdpNego* nego, wStream* s);
static BOOL nego_process_negotiation_response(rdpNego* nego, wStream* s);
static BOOL nego_process_negotiation_failure(rdpNego* nego, wStream* s);
				if (!nego_process_negotiation_response(nego, s))
					return -1;
				if (!nego_process_negotiation_failure(nego, s))
					return -1;
		if (!nego_process_negotiation_request(nego, s))
			return FALSE;
BOOL nego_process_negotiation_request(rdpNego* nego, wStream* s)

	if (Stream_GetRemainingLength(s) < 7)
		return FALSE;
	return TRUE;
BOOL nego_process_negotiation_response(rdpNego* nego, wStream* s)
		return FALSE;
	return TRUE;
BOOL nego_process_negotiation_failure(rdpNego* nego, wStream* s)
	if (Stream_GetRemainingLength(s) < 7)
		return FALSE;
	return TRUE;