[PATCH v6 4/5] vdagentd: early return on bad message size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Victor Toso <me@xxxxxxxxxxxxxx>

The payload size for each message should be the size of the expected
struct or bigger when it contain arrays of no fixed size.

This patch creates the vdagent_message_min_size[] static array with
the expected size for each message so we can check on
virtio_port_read_complete().

Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
Signed-off-by: Michal Suchanek <msuchanek@xxxxxxx>
---
v6
 - bring back the proper size check for clipboard messages
 - sort messages to fixed and variable length and check size accordingly
 - size the vdagent_message_min_size array with VD_AGENT_END_MESSAGE
---
 src/vdagentd/vdagentd.c | 111 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 86 insertions(+), 25 deletions(-)

diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index 785ce50..7e16cd2 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -341,6 +341,88 @@ static void do_client_file_xfer(struct vdagent_virtio_port *vport,
     udscs_write(conn, msg_type, 0, 0, data, message_header->size);
 }
 
+static gsize vdagent_message_min_size[VD_AGENT_END_MESSAGE] =
+{
+    -1, /* Does not exist */
+    sizeof(VDAgentMouseState), /* VD_AGENT_MOUSE_STATE */
+    sizeof(VDAgentMonitorsConfig), /* VD_AGENT_MONITORS_CONFIG */
+    sizeof(VDAgentReply), /* VD_AGENT_REPLY */
+    sizeof(VDAgentClipboard), /* VD_AGENT_CLIPBOARD */
+    sizeof(VDAgentDisplayConfig), /* VD_AGENT_DISPLAY_CONFIG */
+    sizeof(VDAgentAnnounceCapabilities), /* VD_AGENT_ANNOUNCE_CAPABILITIES */
+    sizeof(VDAgentClipboardGrab), /* VD_AGENT_CLIPBOARD_GRAB */
+    sizeof(VDAgentClipboardRequest), /* VD_AGENT_CLIPBOARD_REQUEST */
+    sizeof(VDAgentClipboardRelease), /* VD_AGENT_CLIPBOARD_RELEASE */
+    sizeof(VDAgentFileXferStartMessage), /* VD_AGENT_FILE_XFER_START */
+    sizeof(VDAgentFileXferStatusMessage), /* VD_AGENT_FILE_XFER_STATUS */
+    sizeof(VDAgentFileXferDataMessage), /* VD_AGENT_FILE_XFER_DATA */
+    0, /* VD_AGENT_CLIENT_DISCONNECTED */
+    sizeof(VDAgentMaxClipboard), /* VD_AGENT_MAX_CLIPBOARD */
+    sizeof(VDAgentAudioVolumeSync), /* VD_AGENT_AUDIO_VOLUME_SYNC */
+};
+
+static gboolean vdagent_message_check_size(VDAgentMessage *message_header)
+{
+    uint32_t min_size = 0;
+
+    if (message_header->protocol != VD_AGENT_PROTOCOL) {
+        syslog(LOG_ERR, "message with wrong protocol version ignoring");
+        return FALSE;
+    }
+
+    if (message_header->type >= VD_AGENT_END_MESSAGE) {
+        syslog(LOG_WARNING, "unknown message type %d, ignoring",
+               message_header->type);
+        return FALSE;
+    }
+
+    min_size = vdagent_message_min_size[message_header->type];
+    if (VD_AGENT_HAS_CAPABILITY(capabilities, capabilities_size,
+                                VD_AGENT_CAP_CLIPBOARD_SELECTION)) {
+        switch (message_header->type) {
+        case VD_AGENT_CLIPBOARD_GRAB:
+        case VD_AGENT_CLIPBOARD_REQUEST:
+        case VD_AGENT_CLIPBOARD:
+        case VD_AGENT_CLIPBOARD_RELEASE:
+          min_size += 4;
+        }
+    }
+
+    switch (message_header->type) {
+    case VD_AGENT_MONITORS_CONFIG:
+    case VD_AGENT_FILE_XFER_START:
+    case VD_AGENT_FILE_XFER_DATA:
+    case VD_AGENT_CLIPBOARD:
+    case VD_AGENT_CLIPBOARD_GRAB:
+    case VD_AGENT_AUDIO_VOLUME_SYNC:
+    case VD_AGENT_ANNOUNCE_CAPABILITIES:
+        if (message_header->size < min_size) {
+            syslog(LOG_ERR, "read: invalid message size: %u for message type: %u",
+                   message_header->size, message_header->type);
+            return FALSE;
+        }
+        break;
+    case VD_AGENT_MOUSE_STATE:
+    case VD_AGENT_FILE_XFER_STATUS:
+    case VD_AGENT_DISPLAY_CONFIG:
+    case VD_AGENT_REPLY:
+    case VD_AGENT_CLIPBOARD_REQUEST:
+    case VD_AGENT_CLIPBOARD_RELEASE:
+    case VD_AGENT_MAX_CLIPBOARD:
+    case VD_AGENT_CLIENT_DISCONNECTED:
+        if (message_header->size != min_size) {
+            syslog(LOG_ERR, "read: invalid message size: %u for message type: %u",
+                   message_header->size, message_header->type);
+            return FALSE;
+        }
+        break;
+    default:
+        g_warn_if_reached();
+        return FALSE;
+    }
+    return TRUE;
+}
+
 static int virtio_port_read_complete(
         struct vdagent_virtio_port *vport,
         int port_nr,
@@ -349,26 +431,18 @@ static int virtio_port_read_complete(
 {
     uint32_t min_size = 0;
 
-    if (message_header->protocol != VD_AGENT_PROTOCOL) {
-        syslog(LOG_ERR, "message with wrong protocol version ignoring");
+    if (!vdagent_message_check_size(message_header))
         return 0;
-    }
 
     switch (message_header->type) {
     case VD_AGENT_MOUSE_STATE:
-        if (message_header->size != sizeof(VDAgentMouseState))
-            goto size_error;
         do_client_mouse(&uinput, (VDAgentMouseState *)data);
         break;
     case VD_AGENT_MONITORS_CONFIG:
-        if (message_header->size < sizeof(VDAgentMonitorsConfig))
-            goto size_error;
         do_client_monitors(vport, port_nr, message_header,
                     (VDAgentMonitorsConfig *)data);
         break;
     case VD_AGENT_ANNOUNCE_CAPABILITIES:
-        if (message_header->size < sizeof(VDAgentAnnounceCapabilities))
-            goto size_error;
         do_client_capabilities(vport, message_header,
                         (VDAgentAnnounceCapabilities *)data);
         break;
@@ -388,9 +462,6 @@ static int virtio_port_read_complete(
                                     VD_AGENT_CAP_CLIPBOARD_SELECTION)) {
             min_size += 4;
         }
-        if (message_header->size < min_size) {
-            goto size_error;
-        }
         do_client_clipboard(vport, message_header, data);
         break;
     case VD_AGENT_FILE_XFER_START:
@@ -402,31 +473,21 @@ static int virtio_port_read_complete(
         vdagent_virtio_port_reset(vport, VDP_CLIENT_PORT);
         do_client_disconnect();
         break;
-    case VD_AGENT_MAX_CLIPBOARD:
-        if (message_header->size != sizeof(VDAgentMaxClipboard))
-            goto size_error;
+    case VD_AGENT_MAX_CLIPBOARD: {
         VDAgentMaxClipboard *msg = (VDAgentMaxClipboard *)data;
         syslog(LOG_DEBUG, "Set max clipboard: %d", msg->max);
         max_clipboard = msg->max;
         break;
+    }
     case VD_AGENT_AUDIO_VOLUME_SYNC:
-        if (message_header->size < sizeof(VDAgentAudioVolumeSync))
-            goto size_error;
-
         do_client_volume_sync(vport, port_nr, message_header,
                 (VDAgentAudioVolumeSync *)data);
         break;
     default:
-        syslog(LOG_WARNING, "unknown message type %d, ignoring",
-               message_header->type);
+        g_warn_if_reached();
     }
 
     return 0;
-
-size_error:
-    syslog(LOG_ERR, "read: invalid message size: %u for message type: %u",
-           message_header->size, message_header->type);
-    return 0;
 }
 
 static void virtio_write_clipboard(uint8_t selection, uint32_t msg_type,
-- 
2.10.2

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]