From 86ca19bfe8b227291041694348c55468968ff25b Mon Sep 17 00:00:00 2001
From: matt335672 <30179339+matt335672@users.noreply.github.com>
Date: Tue, 6 Dec 2022 12:48:57 +0000
Subject: [PATCH 4/8] CVE-2022-23480

Added length checking to redirector response parsing
---
 sesman/chansrv/devredir.c | 142 +++++++++++++++++++++++++++++---------
 sesman/chansrv/devredir.h |   6 +-
 2 files changed, 113 insertions(+), 35 deletions(-)

diff --git a/sesman/chansrv/devredir.c b/sesman/chansrv/devredir.c
index 629b3cc9..5a45589d 100644
--- a/sesman/chansrv/devredir.c
+++ b/sesman/chansrv/devredir.c
@@ -204,6 +204,11 @@ dev_redir_data_in(struct stream *s, int chan_id, int chan_flags, int length,
     }
 
     /* read header from incoming data */
+    if (!s_check_rem_and_log(ls, 4, "Parsing [MS-RDPEFS] RDPDR_HEADER"))
+    {
+        rv = -1;
+        goto done;
+    }
     xstream_rd_u16_le(ls, comp_type);
     xstream_rd_u16_le(ls, pktID);
 
@@ -221,27 +226,34 @@ dev_redir_data_in(struct stream *s, int chan_id, int chan_flags, int length,
     switch (pktID)
     {
         case PAKID_CORE_CLIENTID_CONFIRM:
-            xstream_seek(ls, 2);  /* major version, we ignore it */
-            xstream_rd_u16_le(ls, minor_ver);
-            xstream_rd_u32_le(ls, g_clientID);
+            if (!s_check_rem_and_log(ls, 6, "Parsing [MS-RDPEFS] DR_CORE_CLIENT_ANNOUNCE_RSP"))
+            {
+                rv = -1;
+            }
+            else
+            {
+                xstream_seek(ls, 2);  /* major version, we ignore it */
+                xstream_rd_u16_le(ls, minor_ver);
+                xstream_rd_u32_le(ls, g_clientID);
 
-            g_client_rdp_version = minor_ver;
+                g_client_rdp_version = minor_ver;
 
-            switch (minor_ver)
-            {
-                case RDP_CLIENT_50:
-                    break;
+                switch (minor_ver)
+                {
+                    case RDP_CLIENT_50:
+                        break;
 
-                case RDP_CLIENT_51:
-                    break;
+                    case RDP_CLIENT_51:
+                        break;
 
-                case RDP_CLIENT_52:
-                    break;
+                    case RDP_CLIENT_52:
+                        break;
 
-                case RDP_CLIENT_60_61:
-                    break;
+                    case RDP_CLIENT_60_61:
+                        break;
+                }
+                // LK_TODO devredir_send_server_clientID_confirm();
             }
-            // LK_TODO dev_redir_send_server_clientID_confirm();
             break;
 
         case PAKID_CORE_CLIENT_NAME:
@@ -259,15 +271,15 @@ dev_redir_data_in(struct stream *s, int chan_id, int chan_flags, int length,
             break;
 
         case PAKID_CORE_CLIENT_CAPABILITY:
-            dev_redir_proc_client_core_cap_resp(ls);
+            rv = devredir_proc_client_core_cap_resp(ls);
             break;
 
         case PAKID_CORE_DEVICELIST_ANNOUNCE:
-            devredir_proc_client_devlist_announce_req(ls);
+            rv = devredir_proc_client_devlist_announce_req(ls);
             break;
 
         case PAKID_CORE_DEVICE_IOCOMPLETION:
-            dev_redir_proc_device_iocompletion(ls);
+            rv = devredir_proc_device_iocompletion(ls);
             break;
 
         default:
@@ -585,8 +597,10 @@ void dev_redir_send_drive_dir_request(IRP *irp, tui32 device_id,
  * @brief process client's response to our core_capability_req() msg
  *
  * @param   s   stream containing client's response
+ * @return  0   for success, -1 otherwise
  *****************************************************************************/
-void dev_redir_proc_client_core_cap_resp(struct stream *s)
+int
+devredir_proc_client_core_cap_resp(struct stream *s)
 {
     int i;
     tui16 num_caps;
@@ -595,15 +609,31 @@ void dev_redir_proc_client_core_cap_resp(struct stream *s)
     tui32 cap_version;
     char* holdp;
 
+    if (!s_check_rem_and_log(s, 4, "Parsing [MS-RDPEFS] DR_CORE_CAPABLITY_RSP"))
+    {
+        return -1;
+    }
     xstream_rd_u16_le(s, num_caps);
     xstream_seek(s, 2);  /* padding */
 
     for (i = 0; i < num_caps; i++)
     {
         holdp = s->p;
+        if (!s_check_rem_and_log(s, 8, "Parsing [MS-RDPEFS] CAPABILITY_HEADER"))
+        {
+            return -1;
+        }
         xstream_rd_u16_le(s, cap_type);
         xstream_rd_u16_le(s, cap_len);
         xstream_rd_u32_le(s, cap_version);
+        /* Convert the length to a remaining length. Underflow is possible,
+         * but this is an unsigned type so that's OK */
+        cap_len -= (s->p - holdp);
+        if (cap_len > 0 &&
+                !s_check_rem_and_log(s, cap_len, "Parsing [MS-RDPEFS] CAPABILITY_HEADER length"))
+        {
+            return -1;
+        }
 
         switch (cap_type)
         {
@@ -636,11 +666,13 @@ void dev_redir_proc_client_core_cap_resp(struct stream *s)
                 scard_init();
                 break;
         }
-        s->p = holdp + cap_len;
+        xstream_seek(s, cap_len);
     }
+    return 0;
 }
 
-void devredir_proc_client_devlist_announce_req(struct stream *s)
+int
+devredir_proc_client_devlist_announce_req(struct stream *s)
 {
     unsigned int i;
     int   j;
@@ -650,12 +682,22 @@ void devredir_proc_client_devlist_announce_req(struct stream *s)
     char  preferred_dos_name[9];
 
     /* get number of devices being announced */
+    if (!s_check_rem_and_log(s, 4, "Parsing [MS-RDPEFS] DR_CORE_DEVICELIST_ANNOUNCE_REQ"))
+    {
+        return -1;
+    }
+
     xstream_rd_u32_le(s, device_count);
 
     log_debug("num of devices announced: %d", device_count);
 
     for (i = 0; i < device_count; i++)
     {
+        if (!s_check_rem_and_log(s, 4 + 4 + 8 + 4,
+                                 "Parsing [MS-RDPEFS] DEVICE_ANNOUNCE"))
+        {
+            return -1;
+        }
         xstream_rd_u32_le(s, device_type);
         xstream_rd_u32_le(s, g_device_id);
 
@@ -718,10 +760,11 @@ void devredir_proc_client_devlist_announce_req(struct stream *s)
                 break;
         }
     }
+    return 0;
 }
 
-void
-dev_redir_proc_device_iocompletion(struct stream *s)
+int
+devredir_proc_device_iocompletion(struct stream *s)
 {
     FUSE_DATA *fuse_data = NULL;
     IRP       *irp       = NULL;
@@ -731,6 +774,10 @@ dev_redir_proc_device_iocompletion(struct stream *s)
     tui32      IoStatus;
     tui32      Length;
 
+    if (!s_check_rem_and_log(s, 12, "Parsing [MS-RDPEFS] DR_DEVICE_IOCOMPLETION"))
+    {
+        return -1;
+    }
     xstream_rd_u32_le(s, DeviceId);
     xstream_rd_u32_le(s, CompletionId);
     xstream_rd_u32_le(s, IoStatus);
@@ -740,7 +787,7 @@ dev_redir_proc_device_iocompletion(struct stream *s)
     if ((irp = devredir_irp_find(CompletionId)) == NULL)
     {
         log_error("IRP with completion ID %d not found", CompletionId);
-        return;
+        return -1;
     }
 
     /* if callback has been set, call it */
@@ -766,17 +813,27 @@ dev_redir_proc_device_iocompletion(struct stream *s)
                 free(fuse_data);
             }
             devredir_irp_delete(irp);
-            return;
         }
+	else
+	{
+	    if (!s_check_rem_and_log(s, 4, "Parsing [MS-RDPEFS] DR_CREATE_RSP"))
+	    {
+	        return -1;
+	    }
+	    xstream_rd_u32_le(s, irp->FileId);
+            log_debug("got CID_CREATE_DIR_REQ IoStatus=0x%x FileId=%d",
+                      IoStatus, irp->FileId);
+
+            dev_redir_send_drive_dir_request(irp, DeviceId, 1, irp->pathname);
+	}
 
-        xstream_rd_u32_le(s, irp->FileId);
-        log_debug("got CID_CREATE_DIR_REQ IoStatus=0x%x FileId=%d",
-                  IoStatus, irp->FileId);
-
-        dev_redir_send_drive_dir_request(irp, DeviceId, 1, irp->pathname);
         break;
 
     case CID_CREATE_OPEN_REQ:
+	if (!s_check_rem_and_log(s, 4, "Parsing [MS-RDPEFS] DR_CREATE_RSP"))
+	{
+	    return -1;
+	}
         xstream_rd_u32_le(s, irp->FileId);
 
         log_debug("got CID_CREATE_OPEN_REQ IoStatus=0x%x FileId=%d",
@@ -791,7 +848,15 @@ dev_redir_proc_device_iocompletion(struct stream *s)
 
     case CID_READ:
         log_debug("got CID_READ");
+	if (!s_check_rem_and_log(s, 4, "Parsing [MS-RDPEFS] DR_READ_RSP"))
+	{
+	    return -1;
+	}
         xstream_rd_u32_le(s, Length);
+	if (!s_check_rem_and_log(s, Length, "Parsing [MS-RDPEFS] DR_READ_RSP"))
+	{
+	    return -1;
+	}
         fuse_data = devredir_fuse_data_dequeue(irp);
 
         if (fuse_data == NULL)
@@ -807,6 +872,10 @@ dev_redir_proc_device_iocompletion(struct stream *s)
 
     case CID_WRITE:
         log_debug("got CID_WRITE");
+	if (!s_check_rem_and_log(s, 4, "Parsing [MS-RDPEFS] DR_WRITE_RSP"))
+	{
+	    return -1;
+	}
         xstream_rd_u32_le(s, Length);
         fuse_data = devredir_fuse_data_dequeue(irp);
 
@@ -844,9 +913,13 @@ dev_redir_proc_device_iocompletion(struct stream *s)
 
     case CID_RMDIR_OR_FILE:
         log_debug("got CID_RMDIR_OR_FILE");
+	if (!s_check_rem_and_log(s, 4, "Parsing [MS-RDPEFS] DR_CREATE_RSP"))
+	{
+	    return -1;
+	}
         xstream_rd_u32_le(s, irp->FileId);
         devredir_proc_cid_rmdir_or_file(irp, IoStatus);
-        return;
+        return 0;
         break;
 
     case CID_RMDIR_OR_FILE_RESP:
@@ -856,9 +929,13 @@ dev_redir_proc_device_iocompletion(struct stream *s)
 
     case CID_RENAME_FILE:
         log_debug("got CID_RENAME_FILE");
+	if (!s_check_rem_and_log(s, 4, "Parsing [MS-RDPEFS] DR_CREATE_RSP"))
+	{
+	    return -1;
+	}
         xstream_rd_u32_le(s, irp->FileId);
         devredir_proc_cid_rename_file(irp, IoStatus);
-        return;
+        return 0;
         break;
 
     case CID_RENAME_FILE_RESP:
@@ -882,6 +959,7 @@ done:
     }
 
     log_debug("exiting");
+    return 0;
 }
 
 void
diff --git a/sesman/chansrv/devredir.h b/sesman/chansrv/devredir.h
index 5618b39c..0905d26b 100644
--- a/sesman/chansrv/devredir.h
+++ b/sesman/chansrv/devredir.h
@@ -63,9 +63,9 @@ int dev_redir_send_drive_close_request(tui16 Component, tui16 PacketId,
                                        tui32 MinorFunc,
                                        int pad_len);
 
-void devredir_proc_client_devlist_announce_req(struct stream *s);
-void dev_redir_proc_client_core_cap_resp(struct stream *s);
-void dev_redir_proc_device_iocompletion(struct stream *s);
+int devredir_proc_client_devlist_announce_req(struct stream *s);
+int dev_redir_proc_client_core_cap_resp(struct stream *s);
+int dev_redir_proc_device_iocompletion(struct stream *s);
 
 void dev_redir_proc_query_dir_response(IRP *irp,
                                        struct stream *s,
-- 
2.39.0

