[Spice-devel] [PATCH spice-server v2 5/8] Make QXLMessage handling safe

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/memslot.c       | 14 ++++++++++++++
 server/memslot.h       |  3 +++
 server/red-parse-qxl.c | 11 +++++++++++
 server/red-parse-qxl.h |  1 +
 server/red-worker.c    |  3 +--
 5 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/server/memslot.c b/server/memslot.c
index 99c63e4..75cb75f 100644
--- a/server/memslot.c
+++ b/server/memslot.c
@@ -71,6 +71,20 @@ int memslot_validate_virt(RedMemSlotInfo *info, unsigned long virt, int slot_id,
     return 1;
 }
 
+unsigned long memslot_max_size_virt(RedMemSlotInfo *info,
+                                    unsigned long virt, int slot_id,
+                                    uint32_t group_id)
+{
+    MemSlot *slot;
+
+    slot = &info->mem_slots[group_id][slot_id];
+
+    if (virt < slot->virt_start_addr || virt > slot->virt_end_addr) {
+        return 0;
+    }
+    return slot->virt_end_addr - virt;
+}
+
 /*
  * return virtual address if successful, which may be 0.
  * returns 0 and sets error to 1 if an error condition occurs.
diff --git a/server/memslot.h b/server/memslot.h
index a29837c..5046d0f 100644
--- a/server/memslot.h
+++ b/server/memslot.h
@@ -55,6 +55,9 @@ static inline int memslot_get_generation(RedMemSlotInfo *info, uint64_t addr)
 
 int memslot_validate_virt(RedMemSlotInfo *info, unsigned long virt, int slot_id,
                           uint32_t add_size, uint32_t group_id);
+unsigned long memslot_max_size_virt(RedMemSlotInfo *info,
+                                    unsigned long virt, int slot_id,
+                                    uint32_t group_id);
 unsigned long memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t add_size,
                        int group_id, int *error);
 
diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index d75e27e..77533ca 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1300,6 +1300,9 @@ int red_get_message(RedMemSlotInfo *slots, int group_id,
 {
     QXLMessage *qxl;
     int error;
+    int memslot_id;
+    unsigned long len;
+    uint8_t *end;
 
     /*
      * security alert:
@@ -1314,6 +1317,14 @@ int red_get_message(RedMemSlotInfo *slots, int group_id,
     red->release_info_ext.info      = &qxl->release_info;
     red->release_info_ext.group_id  = group_id;
     red->data                       = qxl->data;
+    memslot_id = memslot_get_id(slots, addr+sizeof(*qxl));
+    len = memslot_max_size_virt(slots, ((intptr_t) qxl)+sizeof(*qxl), memslot_id, group_id);
+    len = MIN(len, 100000);
+    end = (uint8_t *)memchr(qxl->data, 0, len);
+    if (end == NULL) {
+        return 1;
+    }
+    red->len = end - qxl->data;
     return 0;
 }
 
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 0da20ad..e174ccc 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -74,6 +74,7 @@ typedef struct RedUpdateCmd {
 
 typedef struct RedMessage {
     QXLReleaseInfoExt release_info_ext;
+    int len;
     uint8_t *data;
 } RedMessage;
 
diff --git a/server/red-worker.c b/server/red-worker.c
index a4efea1..1469f4a 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -238,8 +238,7 @@ static int red_process_display(RedWorker *worker, int *ring_is_empty)
                 break;
             }
 #ifdef DEBUG
-            /* alert: accessing message.data is insecure */
-            spice_warning("MESSAGE: %s", message.data);
+            spice_warning("MESSAGE: %.*s", message.len, message.data);
 #endif
             red_qxl_release_resource(worker->qxl, message.release_info_ext);
             red_put_message(&message);
-- 
2.7.4