[Spice-commits] 2 commits - server/agent-msg-filter.c server/reds.cpp

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Sun Oct 25 20:02:36 UTC 2020


 server/agent-msg-filter.c |    3 +++
 server/reds.cpp           |   31 ++++++++++++++-----------------
 2 files changed, 17 insertions(+), 17 deletions(-)

New commits:
commit de5c20531e6845513ce0765435586606eb572c67
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Wed Sep 16 08:54:52 2020 +0100

    reds: Check VDAgentMonitorsConfig using spice-common code
    
    Reduce code duplication.
    Also this improve support for big endian machines as
    agent_check_message fix also message endianess.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Uri Lublin <ulublin at redhat.com>

diff --git a/server/reds.cpp b/server/reds.cpp
index cea9a4e9..fe69508e 100644
--- a/server/reds.cpp
+++ b/server/reds.cpp
@@ -51,6 +51,7 @@
 #include <spice/stats.h>
 
 #include <common/generated_server_marshallers.h>
+#include <common/agent.h>
 
 #include "spice-wrapped.h"
 #include "reds.h"
@@ -1116,9 +1117,7 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
 
     VDAgentMessage *msg_header;
     VDAgentMonitorsConfig *monitors_config;
-    size_t monitor_size = sizeof(VDAgentMonConfig);
     SpiceBuffer *cmc = &reds->client_monitors_config;
-    uint32_t max_monitors;
     uint32_t msg_size;
 
     // limit size of message sent by the client as this can cause a DoS through
@@ -1140,9 +1139,6 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
         spice_debug("not enough data yet. %" G_GSSIZE_FORMAT, cmc->offset);
         return;
     }
-    if (msg_size < sizeof(VDAgentMonitorsConfig)) {
-        goto overflow;
-    }
 
     // convert VDAgentMessage endianness
     msg_header->protocol = GUINT32_FROM_LE(msg_header->protocol);
@@ -1151,16 +1147,8 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
     msg_header->size = GUINT32_FROM_LE(msg_header->size);
 
     monitors_config = (VDAgentMonitorsConfig *)(cmc->buffer + sizeof(*msg_header));
-    /* filter out not known flags */
-    monitors_config->flags &= VD_AGENT_CONFIG_MONITORS_FLAG_USE_POS |
-        VD_AGENT_CONFIG_MONITORS_FLAG_PHYSICAL_SIZE;
-    if ((monitors_config->flags & VD_AGENT_CONFIG_MONITORS_FLAG_PHYSICAL_SIZE) != 0) {
-        monitor_size += sizeof(VDAgentMonitorMM);
-    }
-    // limit the monitor number to avoid buffer overflows
-    max_monitors = (msg_size - sizeof(VDAgentMonitorsConfig)) /
-                   monitor_size;
-    if (monitors_config->num_of_monitors > max_monitors) {
+    if (agent_check_message(msg_header, (uint8_t *) monitors_config,
+                            NULL, 0) != AGENT_CHECK_NO_ERROR) {
         goto overflow;
     }
     spice_debug("monitors_config->num_of_monitors: %d", monitors_config->num_of_monitors);
commit 587c04f79ee8b6d105063391c8b07c01715a5cc6
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Wed Sep 16 08:35:18 2020 +0100

    Improve big endian support for agent messages
    
    Big endian machines on server are not officially supported but won't
    hurt.
    Messages from client are always encoded as little endian (as all
    SPICE protocol).
    Convert fields from little endian to host endian on some places
    where numbers are used and not just binary copied.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Uri Lublin <ulublin at redhat.com>

diff --git a/server/agent-msg-filter.c b/server/agent-msg-filter.c
index 11612c6b..e18e8fcf 100644
--- a/server/agent-msg-filter.c
+++ b/server/agent-msg-filter.c
@@ -70,6 +70,9 @@ data_to_read:
         return AGENT_MSG_FILTER_PROTO_ERROR;
     }
     memcpy(&msg_header, data, sizeof(msg_header));
+    msg_header.protocol = GUINT32_FROM_LE(msg_header.protocol);
+    msg_header.type = GUINT32_FROM_LE(msg_header.type);
+    msg_header.size = GUINT32_FROM_LE(msg_header.size);
     len -= sizeof(msg_header);
 
     if (msg_header.protocol != VD_AGENT_PROTOCOL) {
diff --git a/server/reds.cpp b/server/reds.cpp
index fd3632f0..cea9a4e9 100644
--- a/server/reds.cpp
+++ b/server/reds.cpp
@@ -1119,6 +1119,7 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
     size_t monitor_size = sizeof(VDAgentMonConfig);
     SpiceBuffer *cmc = &reds->client_monitors_config;
     uint32_t max_monitors;
+    uint32_t msg_size;
 
     // limit size of message sent by the client as this can cause a DoS through
     // memory exhaustion, or potentially some integer overflows
@@ -1131,16 +1132,24 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
         return;
     }
     msg_header = (VDAgentMessage *)cmc->buffer;
-    if (msg_header->size > MAX_MONITOR_CONFIG_SIZE) {
+    msg_size = GUINT32_FROM_LE(msg_header->size);
+    if (msg_size > MAX_MONITOR_CONFIG_SIZE) {
         goto overflow;
     }
-    if (msg_header->size > cmc->offset - sizeof(VDAgentMessage)) {
+    if (msg_size > cmc->offset - sizeof(VDAgentMessage)) {
         spice_debug("not enough data yet. %" G_GSSIZE_FORMAT, cmc->offset);
         return;
     }
-    if (msg_header->size < sizeof(VDAgentMonitorsConfig)) {
+    if (msg_size < sizeof(VDAgentMonitorsConfig)) {
         goto overflow;
     }
+
+    // convert VDAgentMessage endianness
+    msg_header->protocol = GUINT32_FROM_LE(msg_header->protocol);
+    msg_header->type = GUINT32_FROM_LE(msg_header->type);
+    msg_header->opaque = GUINT64_FROM_LE(msg_header->opaque);
+    msg_header->size = GUINT32_FROM_LE(msg_header->size);
+
     monitors_config = (VDAgentMonitorsConfig *)(cmc->buffer + sizeof(*msg_header));
     /* filter out not known flags */
     monitors_config->flags &= VD_AGENT_CONFIG_MONITORS_FLAG_USE_POS |
@@ -1149,7 +1158,7 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
         monitor_size += sizeof(VDAgentMonitorMM);
     }
     // limit the monitor number to avoid buffer overflows
-    max_monitors = (msg_header->size - sizeof(VDAgentMonitorsConfig)) /
+    max_monitors = (msg_size - sizeof(VDAgentMonitorsConfig)) /
                    monitor_size;
     if (monitors_config->num_of_monitors > max_monitors) {
         goto overflow;


More information about the Spice-commits mailing list