Updating to refactor xrdp_client_info

- Eliminate duplicaiton for display_size_description
- monitorCount needs to be uint32_t
- width/height -> session_width/session_height
- Update CLIENT_INFO_CURRENT_VERSION
- Also some misc unit test updates.

common/xrdp_client_info.h

@@ -39,14 +39,14 @@ struct monitor_info
     int flags;
 
     /* From 2.2.2.2.1 DISPLAYCONTROL_MONITOR_LAYOUT */
-    int physical_width;
-    int physical_height;
-    int orientation;
-    int desktop_scale_factor;
-    int device_scale_factor;
+    unsigned int physical_width;
+    unsigned int physical_height;
+    unsigned int orientation;
+    unsigned int desktop_scale_factor;
+    unsigned int device_scale_factor;
 
     /* Derived setting */
-    int is_primary;
+    unsigned int is_primary;
 };
 
 /* xrdp keyboard overrids */
@@ -59,11 +59,11 @@ struct xrdp_keyboard_overrides
 
 struct display_size_description
 {
-    int monitorCount; /* number of monitors detected (max = 16) */
+    unsigned int monitorCount; /* 2.2.2.2 DISPLAYCONTROL_MONITOR_LAYOUT_PDU: number of monitors detected (max = 16) */
     struct monitor_info minfo[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data */
     struct monitor_info minfo_wm[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data, non-negative values */
-    int session_width;
-    int session_height;
+    unsigned int session_width;
+    unsigned int session_height;
 };
 
 /**
@@ -79,8 +79,6 @@ struct xrdp_client_info
     int size; /* bytes for this structure */
     int version; /* Should be CLIENT_INFO_CURRENT_VERSION */
     int bpp;
-    int width;
-    int height;
     /* bitmap cache info */
     int cache1_entries;
     int cache1_size;
@@ -153,9 +151,7 @@ struct xrdp_client_info
 
     int security_layer; /* 0 = rdp, 1 = tls , 2 = hybrid */
     int multimon; /* 0 = deny , 1 = allow */
-    int monitorCount; /* number of monitors detected (max = 16) */
-    struct monitor_info minfo[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data */
-    struct monitor_info minfo_wm[CLIENT_MONITOR_DATA_MAXIMUM_MONITORS]; /* client monitor data, non-negative values */
+    struct display_size_description display_sizes;
 
     int keyboard_type;
     int keyboard_subtype;
@@ -211,6 +207,6 @@ struct xrdp_client_info
 };
 
 /* yyyymmdd of last incompatible change to xrdp_client_info */
-#define CLIENT_INFO_CURRENT_VERSION 20210723
+#define CLIENT_INFO_CURRENT_VERSION 20220320
 
 #endif

libxrdp/libxrdp.c

@@ -1134,7 +1134,7 @@ libxrdp_orders_send_font(struct xrdp_session *session,
  * to a single monitor */
 int EXPORT_CC
 libxrdp_reset(struct xrdp_session *session,
-              int width, int height, int bpp)
+              unsigned int width, unsigned int height, int bpp)
 {
     if (session->client_info != 0)
     {
@@ -1147,18 +1147,18 @@ libxrdp_reset(struct xrdp_session *session,
         }
 
         /* if same (and only one monitor on client) don't need to do anything */
-        if (client_info->width == width &&
-                client_info->height == height &&
+        if (client_info->display_sizes.session_width == width &&
+                client_info->display_sizes.session_height == height &&
                 client_info->bpp == bpp &&
-                (client_info->monitorCount == 0 || client_info->multimon == 0))
+                (client_info->display_sizes.monitorCount == 0 || client_info->multimon == 0))
         {
             return 0;
         }
 
-        client_info->width = width;
-        client_info->height = height;
+        client_info->display_sizes.session_width = width;
+        client_info->display_sizes.session_height = height;
+        client_info->display_sizes.monitorCount = 0;
         client_info->bpp = bpp;
-        client_info->monitorCount = 0;
         client_info->multimon = 0;
     }
     else
@@ -1908,8 +1908,39 @@ libxrdp_process_monitor_stream(struct stream *s,
             in_uint32_le(s, monitor_layout->physical_width);
             in_uint32_le(s, monitor_layout->physical_height);
 
+            /* Per spec (2.2.2.2.1 DISPLAYCONTROL_MONITOR_LAYOUT),
+             * if EITHER physical_width or physical_height are
+             * out of range, BOTH must be ignored.
+             */
+            if (monitor_layout->physical_width > 10000
+                    || monitor_layout->physical_width < 10)
+            {
+                LOG(LOG_LEVEL_WARNING, "libxrdp_process_monitor_stream:"
+                    " physical_width is not within valid range."
+                    " Setting physical_width to 0mm,"
+                    " Setting physical_height to 0mm,"
+                    " physical_width was: %d",
+                    monitor_layout->physical_width);
+                monitor_layout->physical_width = 0;
+                monitor_layout->physical_height = 0;
+            }
+
+            if (monitor_layout->physical_height > 10000
+                    || monitor_layout->physical_height < 10)
+            {
+                LOG(LOG_LEVEL_WARNING, "libxrdp_process_monitor_stream:"
+                    " physical_height is not within valid range."
+                    " Setting physical_width to 0mm,"
+                    " Setting physical_height to 0mm,"
+                    " physical_height was: %d",
+                    monitor_layout->physical_height);
+                monitor_layout->physical_width = 0;
+                monitor_layout->physical_height = 0;
+            }
+
             in_uint32_le(s, monitor_layout->orientation);
-            switch (monitor_layout->orientation) {
+            switch (monitor_layout->orientation)
+            {
                 case ORIENTATION_LANDSCAPE:
                 case ORIENTATION_PORTRAIT:
                 case ORIENTATION_LANDSCAPE_FLIPPED:
@@ -1918,16 +1949,42 @@ libxrdp_process_monitor_stream(struct stream *s,
                 default:
                     LOG(LOG_LEVEL_WARNING, "libxrdp_process_monitor_stream:"
                         " Orientation is not one of %d, %d, %d, or %d."
-                        " Value was %d and ignored.",
+                        " Value was %d and ignored and set to default value of LANDSCAPE.",
                         ORIENTATION_LANDSCAPE,
                         ORIENTATION_PORTRAIT,
                         ORIENTATION_LANDSCAPE_FLIPPED,
                         ORIENTATION_PORTRAIT_FLIPPED,
                         monitor_layout->orientation);
+                    monitor_layout->orientation = ORIENTATION_LANDSCAPE;
             }
 
             in_uint32_le(s, monitor_layout->desktop_scale_factor);
+            if (monitor_layout->desktop_scale_factor < 100
+                    || monitor_layout->desktop_scale_factor > 500
+                    || (monitor_layout->desktop_scale_factor != 100
+                        && monitor_layout->desktop_scale_factor != 140
+                        && monitor_layout->desktop_scale_factor != 180))
+            {
+                LOG(LOG_LEVEL_WARNING, "libxrdp_process_monitor_stream:"
+                    " desktop_scale_factor is not within valid range. Assuming 100."
+                    " Value was: %d",
+                    monitor_layout->desktop_scale_factor);
+                monitor_layout->desktop_scale_factor = 100;
+            }
+
             in_uint32_le(s, monitor_layout->device_scale_factor);
+            if (monitor_layout->device_scale_factor < 100
+                    || monitor_layout->device_scale_factor > 500
+                    || (monitor_layout->device_scale_factor != 100
+                        && monitor_layout->device_scale_factor != 140
+                        && monitor_layout->device_scale_factor != 180))
+            {
+                LOG(LOG_LEVEL_WARNING, "libxrdp_process_monitor_stream:"
+                    " device_scale_factor is not within valid range. Assuming 100."
+                    " Value was: %d",
+                    monitor_layout->device_scale_factor);
+                monitor_layout->device_scale_factor = 100;
+            }
 
             /*
              * 2.2.2.2.1 DISPLAYCONTROL_MONITOR_LAYOUT

libxrdp/libxrdp.h

@@ -361,8 +361,10 @@ xrdp_mcs_disconnect(struct xrdp_mcs *self);
 /* xrdp_sec.c */
 
 /*
-    These are return values for xrdp_sec_process_mcs_data_monitors
-    to clarify any reason for a non-zero response code.
+    These are error return codes for both:
+        1. xrdp_sec_process_mcs_data_monitors
+        2. libxrdp_process_monitor_stream
+    To clarify any reason for a non-zero response code.
 */
 #define SEC_PROCESS_MONITORS_ERR 1
 #define SEC_PROCESS_MONITORS_ERR_TOO_MANY_MONITORS 2

libxrdp/libxrdpinc.h

@@ -193,7 +193,7 @@ libxrdp_orders_send_font(struct xrdp_session *session,
                          int font_index, int char_index);
 int
 libxrdp_reset(struct xrdp_session *session,
-              int width, int height, int bpp);
+              unsigned int width, unsigned int height, int bpp);
 int
 libxrdp_orders_send_raw_bitmap2(struct xrdp_session *session,
                                 int width, int height, int bpp, char *data,

libxrdp/xrdp_caps.c

@@ -45,7 +45,8 @@ static int
 xrdp_caps_send_monitorlayout(struct xrdp_rdp *self)
 {
     struct stream *s;
-    int i;
+    uint32_t i;
+    struct display_size_description *description;
 
     make_stream(s);
     init_stream(s, 8192);
@@ -56,16 +57,18 @@ xrdp_caps_send_monitorlayout(struct xrdp_rdp *self)
         return 1;
     }
 
-    out_uint32_le(s, self->client_info.monitorCount); /* monitorCount (4 bytes) */
+    description = &self->client_info.display_sizes;
+
+    out_uint32_le(s, description->monitorCount); /* monitorCount (4 bytes) */
 
     /* TODO: validate for allowed monitors in terminal server (maybe by config?) */
-    for (i = 0; i < self->client_info.monitorCount; i++)
+    for (i = 0; i < description->monitorCount; i++)
     {
-        out_uint32_le(s, self->client_info.minfo[i].left);
-        out_uint32_le(s, self->client_info.minfo[i].top);
-        out_uint32_le(s, self->client_info.minfo[i].right);
-        out_uint32_le(s, self->client_info.minfo[i].bottom);
-        out_uint32_le(s, self->client_info.minfo[i].is_primary);
+        out_uint32_le(s, description->minfo[i].left);
+        out_uint32_le(s, description->minfo[i].top);
+        out_uint32_le(s, description->minfo[i].right);
+        out_uint32_le(s, description->minfo[i].bottom);
+        out_uint32_le(s, description->minfo[i].is_primary);
     }
 
     s_mark_end(s);
@@ -883,8 +886,8 @@ unsigned int calculate_multifragmentupdate_len(const struct xrdp_rdp *self)
 {
     unsigned int result = MAX_MULTIFRAGMENTUPDATE_SIZE;
 
-    unsigned int x_tiles = (self->client_info.width + 63) / 64;
-    unsigned int y_tiles = (self->client_info.height + 63) / 64;
+    unsigned int x_tiles = (self->client_info.display_sizes.session_width + 63) / 64;
+    unsigned int y_tiles = (self->client_info.display_sizes.session_height + 63) / 64;
 
     /* Check for overflow on calculation if bad parameters are supplied */
     if ((x_tiles * y_tiles  + 1) < (UINT_MAX / 16384))
@@ -979,8 +982,8 @@ xrdp_caps_send_demand_active(struct xrdp_rdp *self)
     out_uint16_le(s, 1); /* Receive 1 BPP */
     out_uint16_le(s, 1); /* Receive 4 BPP */
     out_uint16_le(s, 1); /* Receive 8 BPP */
-    out_uint16_le(s, self->client_info.width); /* width */
-    out_uint16_le(s, self->client_info.height); /* height */
+    out_uint16_le(s, self->client_info.display_sizes.session_width); /* width */
+    out_uint16_le(s, self->client_info.display_sizes.session_height); /* height */
     out_uint16_le(s, 0); /* Pad */
     out_uint16_le(s, 1); /* Allow resize */
     out_uint16_le(s, 1); /* bitmap compression */
@@ -1242,7 +1245,7 @@ xrdp_caps_send_demand_active(struct xrdp_rdp *self)
     }
 
     /* send Monitor Layout PDU for dual monitor */
-    if (self->client_info.monitorCount > 0 &&
+    if (self->client_info.display_sizes.monitorCount > 0 &&
             self->client_info.multimon == 1)
     {
         LOG_DEVEL(LOG_LEVEL_TRACE, "xrdp_caps_send_demand_active: sending monitor layout pdu");

libxrdp/xrdp_sec.c

@@ -1953,8 +1953,8 @@ xrdp_sec_process_mcs_data_CS_CORE(struct xrdp_sec *self, struct stream *s)
 
     /* TS_UD_CS_CORE requiered fields */
     in_uint8s(s, 4); /* version */
-    in_uint16_le(s, self->rdp_layer->client_info.width);
-    in_uint16_le(s, self->rdp_layer->client_info.height);
+    in_uint16_le(s, self->rdp_layer->client_info.display_sizes.session_width);
+    in_uint16_le(s, self->rdp_layer->client_info.display_sizes.session_height);
     in_uint16_le(s, colorDepth);
     switch (colorDepth)
     {
@@ -1981,8 +1981,8 @@ xrdp_sec_process_mcs_data_CS_CORE(struct xrdp_sec *self, struct stream *s)
               "clientName %s, keyboardType (ignored), "
               "keyboardSubType (ignored), keyboardFunctionKey (ignored), "
               "imeFileName (ignroed)",
-              self->rdp_layer->client_info.width,
-              self->rdp_layer->client_info.height,
+              self->rdp_layer->client_info.display_sizes.session_width,
+              self->rdp_layer->client_info.display_sizes.session_height,
               (colorDepth == 0xca00 ? "RNS_UD_COLOR_4BPP" :
                colorDepth == 0xca01 ? "RNS_UD_COLOR_8BPP" : "unknown"),
               clientName);
@@ -2370,17 +2370,17 @@ xrdp_sec_process_mcs_data_monitors(struct xrdp_sec *self, struct stream *s)
     error = libxrdp_process_monitor_stream(s, description, 0);
     if (error == 0)
     {
-        client_info->monitorCount = description->monitorCount;
+        client_info->display_sizes.monitorCount = description->monitorCount;
 
         LOG_DEVEL(LOG_LEVEL_TRACE, "xrdp_sec_process_mcs_data_monitors:"
                   " Received [MS-RDPBCGR] TS_UD_CS_MONITOR"
                   " flags 0x%8.8x, monitorCount %d",
                   flags, description->monitorCount);
 
-        client_info->width = description->session_width;
-        client_info->height = description->session_height;
-        g_memcpy(client_info->minfo, description->minfo, sizeof(struct monitor_info) * CLIENT_MONITOR_DATA_MAXIMUM_MONITORS);
-        g_memcpy(client_info->minfo_wm, description->minfo_wm, sizeof(struct monitor_info) * CLIENT_MONITOR_DATA_MAXIMUM_MONITORS);
+        client_info->display_sizes.session_width = description->session_width;
+        client_info->display_sizes.session_height = description->session_height;
+        g_memcpy(client_info->display_sizes.minfo, description->minfo, sizeof(struct monitor_info) * CLIENT_MONITOR_DATA_MAXIMUM_MONITORS);
+        g_memcpy(client_info->display_sizes.minfo_wm, description->minfo_wm, sizeof(struct monitor_info) * CLIENT_MONITOR_DATA_MAXIMUM_MONITORS);
     }
 
     g_free(description);

tests/libxrdp/Makefile.am

@@ -14,7 +14,8 @@ check_PROGRAMS = test_libxrdp
 test_libxrdp_SOURCES = \
     test_libxrdp.h \
     test_libxrdp_main.c \
-    test_monitor_processing.c
+    test_libxrdp_process_monitor_stream.c \
+    test_xrdp_sec_process_mcs_data_monitors.c
 
 test_libxrdp_CFLAGS = \
     @CHECK_CFLAGS@

tests/libxrdp/test_libxrdp.h

@@ -3,6 +3,7 @@
 
 #include <check.h>
 
+Suite *make_suite_test_xrdp_sec_process_mcs_data_monitors(void);
 Suite *make_suite_test_monitor_processing(void);
 
 #endif /* TEST_LIBXRDP_H */

tests/libxrdp/test_libxrdp_main.c

@@ -11,8 +11,8 @@ int main (void)
     int number_failed;
     SRunner *sr;
 
-    sr = srunner_create (make_suite_test_monitor_processing());
-    //   srunner_add_suite(sr, make_list_suite());
+    sr = srunner_create(make_suite_test_xrdp_sec_process_mcs_data_monitors());
+    srunner_add_suite(sr, make_suite_test_monitor_processing());
 
     srunner_set_tap(sr, "-");
     srunner_run_all (sr, CK_ENV);