Make QXLMessage handling safe

The QXLMessage has no size so potentially a guest could give an
address that cause the string to overflow out of the video memory.
The current solution is to parse the message, release the resources
associated without printing the message from the client.
This also considering that the QXLMessage usage was deprecated
a while ago (I don't know exactly when).
This patches limit the string to 100000 characters (guest can feed
so much logs in other way) and limit to video memory.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>

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.

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);
 

server/red-parse-qxl.c

@@ -1293,6 +1293,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:
@@ -1307,6 +1310,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;
 }
 

server/red-parse-qxl.h

@@ -76,6 +76,7 @@ typedef struct RedUpdateCmd {
 
 typedef struct RedMessage {
     QXLReleaseInfoExt release_info_ext;
+    int len;
     uint8_t *data;
 } RedMessage;
 

server/red-worker.c

@@ -234,8 +234,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);