[Spice-devel] [PATCH v7 2/3] vdagentd: early return on bad message size
Michal Suchanek
msuchanek at suse.de
Fri Jan 27 17:53:39 UTC 2017
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 and
function vdagent_message_check_size() that checks message size with the
expected size for each message type so we can check message size at
start of virtio_port_read_complete() before accessing the data fields.
The size of some message types was previously not checked before
accessing their data which could lead to access outside of the message
buffer. On the other hand, some fixed size messages were checked only
for being too short and not for extra garbage.
Signed-off-by: Victor Toso <victortoso at redhat.com>
Signed-off-by: Michal Suchanek <msuchanek at suse.de>
---
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
v7
- use G_N_ELEMENTS
- constify argument to size check function
- remove useless calculation
---
src/vdagentd/vdagentd.c | 131 ++++++++++++++++++++++++++++++++----------------
1 file changed, 89 insertions(+), 42 deletions(-)
diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index 5ca0106..2329489 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -341,34 +341,107 @@ 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[] =
+{
+ -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(const 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 ||
+ message_header->type >= G_N_ELEMENTS(vdagent_message_min_size)) {
+ 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,
VDAgentMessage *message_header,
uint8_t *data)
{
- 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;
@@ -376,21 +449,6 @@ static int virtio_port_read_complete(
case VD_AGENT_CLIPBOARD_REQUEST:
case VD_AGENT_CLIPBOARD:
case VD_AGENT_CLIPBOARD_RELEASE:
- switch (message_header->type) {
- case VD_AGENT_CLIPBOARD_GRAB:
- min_size = sizeof(VDAgentClipboardGrab); break;
- case VD_AGENT_CLIPBOARD_REQUEST:
- min_size = sizeof(VDAgentClipboardRequest); break;
- case VD_AGENT_CLIPBOARD:
- min_size = sizeof(VDAgentClipboard); break;
- }
- if (VD_AGENT_HAS_CAPABILITY(capabilities, capabilities_size,
- 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 +460,20 @@ 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;
- VDAgentMaxClipboard *msg = (VDAgentMaxClipboard *)data;
- syslog(LOG_DEBUG, "Set max clipboard: %d", msg->max);
- max_clipboard = msg->max;
+ case VD_AGENT_MAX_CLIPBOARD: {
+ max_clipboard = ((VDAgentMaxClipboard *)data)->max;
+ syslog(LOG_DEBUG, "Set max clipboard: %d", max_clipboard);
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
More information about the Spice-devel
mailing list