[Qemu-devel] [PATCH v4 13/30] stellaris_enet: avoid buffer overrun on incoming migration

["Michael S. Tsirkin" <mst@redhat.com>]

CVE-2013-4532

s->next_packet is read from wire as an index into s->rx[]. If
s->next_packet exceeds the length of s->rx[], the buffer can be
subsequently overrun with arbitrary data from the wire.

Fix this by failing migration if s->next_packet we read from
the wire exceeds this.

Similarly, validate rx_fifo against sizeof(s->rx[].data).

Finally, constrain rx len to a sensibly small positive
value, to avoid integer overruns when data model
later uses this value.

Reported-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/stellaris_enet.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
index d04e6a4..182fd3e 100644
--- a/hw/net/stellaris_enet.c
+++ b/hw/net/stellaris_enet.c
@@ -358,7 +358,7 @@ static void stellaris_enet_save(QEMUFile *f, void *opaque)
 static int stellaris_enet_load(QEMUFile *f, void *opaque, int version_id)
 {
     stellaris_enet_state *s = (stellaris_enet_state *)opaque;
-    int i;
+    int i, v;
 
     if (version_id != 1)
         return -EINVAL;
@@ -381,9 +381,25 @@ static int stellaris_enet_load(QEMUFile *f, void *opaque, int version_id)
         qemu_get_buffer(f, s->rx[i].data, sizeof(s->rx[i].data));
 
     }
-    s->next_packet = qemu_get_be32(f);
-    s->rx_fifo = s->rx[s->next_packet].data + qemu_get_be32(f);
-    s->rx_fifo_len = qemu_get_be32(f);
+    v = qemu_get_be32(f);
+    if (v < 0 || v >= ARRAY_SIZE(s->rx)) {
+        return -EINVAL;
+    }
+    s->next_packet = v;
+    v = qemu_get_be32(f);
+    if (v < 0 || v >= sizeof(s->rx[i].data)) {
+        return -EINVAL;
+    }
+    s->rx_fifo = s->rx[s->next_packet].data + v;
+    v = qemu_get_be32(f);
+    /* Set limit low enough to avoid integer overflow when
+     * we do math on len later, but high enough to avoid
+     * truncating any packets.
+     */
+    if (v < 0 || v >= 0x100000) {
+        return -EINVAL;
+    }
+    s->rx_fifo_len = v;
 
     return 0;
 }
-- 
MST