[Spice-devel] [vdagent-linux v5 1/3] vdagentd: early return on bad message size

[Victor Toso]

From: Victor Toso <me at victortoso.com>

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 at redhat.com>
---
 src/vdagentd/vdagentd.c | 56 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index a1faf23..4eb8e80 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -77,6 +77,25 @@ static int retval = 0;
 static int client_connected = 0;
 static int max_clipboard = -1;
 
+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 */
+};
+
 /* utility functions */
 /* vdagentd <-> spice-client communication handling */
 static void send_capabilities(struct vdagent_virtio_port *vport,
@@ -329,10 +348,20 @@ static int virtio_port_read_complete(
         return 0;
     }
 
+    if (message_header->type >= VD_AGENT_END_MESSAGE) {
+        syslog(LOG_WARNING, "unknown message type %d, ignoring",
+               message_header->type);
+        return 0;
+    }
+
+    if (message_header->size < vdagent_message_min_size[message_header->type]) {
+        syslog(LOG_ERR, "read: invalid message size: %u for message type: %u",
+               message_header->size, message_header->type);
+        return 0;
+    }
+
     switch (message_header->type) {
     case VD_AGENT_MOUSE_STATE:
-        if (message_header->size != sizeof(VDAgentMouseState))
-            goto size_error;
         vdagentd_uinput_do_mouse(&uinput, (VDAgentMouseState *)data);
         if (!uinput) {
             /* Try to re-open the tablet */
@@ -354,14 +383,10 @@ static int virtio_port_read_complete(
         }
         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;
@@ -381,9 +406,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:
@@ -395,31 +417,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.9.3