CVE-2013-4532: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: Qemu 1.1.2+dfsg to 2.1+dfsg suffers from a buffer overrun which could potentially result in arbitrary code execution on the host with the privileges of the QEMU process.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 7.8
Published: 2020-01-02T16:15Z
Repository: https://github.com/qemu/qemu
Language: C
Files changed: hw/net/stellaris_enet.c
Commit date: 2014-05-13 16:09:38 +0100
Commit message: hw/net/stellaris_enet: Convert to vmstate

Convert this device to use vmstate for its save/load, including
providing a post_load function that sanitizes inbound data to
avoid possible buffer overflows if it is malicious.

The sanitizing fixes CVE-2013-4532 (though nobody should be
relying on the security properties of most of the unmaintained
ARM board models anyway, and migration doesn't actually
work on this board due to issues in other device models).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Diff: diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
index 9e8f143ea5..c9ee5d3f10 100644
--- a/hw/net/stellaris_enet.c
+++ b/hw/net/stellaris_enet.c
@@ -44,10 +44,15 @@ do { fprintf(stderr, "stellaris_enet: error: " fmt , ## __VA_ARGS__);} while (0)
 
 #define TYPE_STELLARIS_ENET "stellaris_enet"
 #define STELLARIS_ENET(obj) \
     OBJECT_CHECK(stellaris_enet_state, (obj), TYPE_STELLARIS_ENET)
 
+typedef struct {
+    uint8_t data[2048];
+    uint32_t len;
+} StellarisEnetRxFrame;
+
 typedef struct {
     SysBusDevice parent_obj;
 
     uint32_t ris;
     uint32_t im;
@@ -57,26 +62,93 @@ typedef struct {
     uint32_t mctl;
     uint32_t mdv;
     uint32_t mtxd;
     uint32_t mrxd;
     uint32_t np;
-    int tx_fifo_len;
+    uint32_t tx_fifo_len;
     uint8_t tx_fifo[2048];
     /* Real hardware has a 2k fifo, which works out to be at most 31 packets.
        We implement a full 31 packet fifo.  */
-    struct {
-        uint8_t data[2048];
-        int len;
-    } rx[31];
-    int rx_fifo_offset;
-    int next_packet;
+    StellarisEnetRxFrame rx[31];
+    uint32_t rx_fifo_offset;
+    uint32_t next_packet;
     NICState *nic;
     NICConf conf;
     qemu_irq irq;
     MemoryRegion mmio;
 } stellaris_enet_state;
 
+static const VMStateDescription vmstate_rx_frame = {
+    .name = "stellaris_enet/rx_frame",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8_ARRAY(data, StellarisEnetRxFrame, 2048),
+        VMSTATE_UINT32(len, StellarisEnetRxFrame),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static int stellaris_enet_post_load(void *opaque, int version_id)
+{
+    stellaris_enet_state *s = opaque;
+    int i;
+
+    /* Sanitize inbound state. Note that next_packet is an index but
+     * np is a size; hence their valid upper bounds differ.
+     */
+    if (s->next_packet >= ARRAY_SIZE(s->rx)) {
+        return -1;
+    }
+
+    if (s->np > ARRAY_SIZE(s->rx)) {
+        return -1;
+    }
+
+    for (i = 0; i < ARRAY_SIZE(s->rx); i++) {
+        if (s->rx[i].len > ARRAY_SIZE(s->rx[i].data)) {
+            return -1;
+        }
+    }
+
+    if (s->rx_fifo_offset > ARRAY_SIZE(s->rx[0].data) - 4) {
+        return -1;
+    }
+
+    if (s->tx_fifo_len > ARRAY_SIZE(s->tx_fifo)) {
+        return -1;
+    }
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_stellaris_enet = {
+    .name = "stellaris_enet",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .post_load = stellaris_enet_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(ris, stellaris_enet_state),
+        VMSTATE_UINT32(im, stellaris_enet_state),
+        VMSTATE_UINT32(rctl, stellaris_enet_state),
+        VMSTATE_UINT32(tctl, stellaris_enet_state),
+        VMSTATE_UINT32(thr, stellaris_enet_state),
+        VMSTATE_UINT32(mctl, stellaris_enet_state),
+        VMSTATE_UINT32(mdv, stellaris_enet_state),
+        VMSTATE_UINT32(mtxd, stellaris_enet_state),
+        VMSTATE_UINT32(mrxd, stellaris_enet_state),
+        VMSTATE_UINT32(np, stellaris_enet_state),
+        VMSTATE_UINT32(tx_fifo_len, stellaris_enet_state),
+        VMSTATE_UINT8_ARRAY(tx_fifo, stellaris_enet_state, 2048),
+        VMSTATE_STRUCT_ARRAY(rx, stellaris_enet_state, 31, 1,
+                             vmstate_rx_frame, StellarisEnetRxFrame),
+        VMSTATE_UINT32(rx_fifo_offset, stellaris_enet_state),
+        VMSTATE_UINT32(next_packet, stellaris_enet_state),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void stellaris_enet_update(stellaris_enet_state *s)
 {
     qemu_set_irq(s->irq, (s->ris & s->im) != 0);
 }
 
@@ -377,67 +449,10 @@ static void stellaris_enet_reset(stellaris_enet_state *s)
             | SE_INT_TXER | SE_INT_RX;
     s->thr = 0x3f;
     s->tx_fifo_len = 0;
 }
 
-static void stellaris_enet_save(QEMUFile *f, void *opaque)
-{
-    stellaris_enet_state *s = (stellaris_enet_state *)opaque;
-    int i;
-
-    qemu_put_be32(f, s->ris);
-    qemu_put_be32(f, s->im);
-    qemu_put_be32(f, s->rctl);
-    qemu_put_be32(f, s->tctl);
-    qemu_put_be32(f, s->thr);
-    qemu_put_be32(f, s->mctl);
-    qemu_put_be32(f, s->mdv);
-    qemu_put_be32(f, s->mtxd);
-    qemu_put_be32(f, s->mrxd);
-    qemu_put_be32(f, s->np);
-    qemu_put_be32(f, s->tx_fifo_len);
-    qemu_put_buffer(f, s->tx_fifo, sizeof(s->tx_fifo));
-    for (i = 0; i < 31; i++) {
-        qemu_put_be32(f, s->rx[i].len);
-        qemu_put_buffer(f, s->rx[i].data, sizeof(s->rx[i].data));
-
-    }
-    qemu_put_be32(f, s->next_packet);
-    qemu_put_be32(f, s->rx_fifo_offset);
-}
-
-static int stellaris_enet_load(QEMUFile *f, void *opaque, int version_id)
-{
-    stellaris_enet_state *s = (stellaris_enet_state *)opaque;
-    int i;
-
-    if (version_id != 1)
-        return -EINVAL;
-
-    s->ris = qemu_get_be32(f);
-    s->im = qemu_get_be32(f);
-    s->rctl = qemu_get_be32(f);
-    s->tctl = qemu_get_be32(f);
-    s->thr = qemu_get_be32(f);
-    s->mctl = qemu_get_be32(f);
-    s->mdv = qemu_get_be32(f);
-    s->mtxd = qemu_get_be32(f);
-    s->mrxd = qemu_get_be32(f);
-    s->np = qemu_get_be32(f);
-    s->tx_fifo_len = qemu_get_be32(f);
-    qemu_get_buffer(f, s->tx_fifo, sizeof(s->tx_fifo));
-    for (i = 0; i < 31; i++) {
-        s->rx[i].len = qemu_get_be32(f);
-        qemu_get_buffer(f, s->rx[i].data, sizeof(s->rx[i].data));
-
-    }
-    s->next_packet = qemu_get_be32(f);
-    s->rx_fifo_offset = qemu_get_be32(f);
-
-    return 0;
-}
-
 static void stellaris_enet_cleanup(NetClientState *nc)
 {
     stellaris_enet_state *s = qemu_get_nic_opaque(nc);
 
     s->nic = NULL;
@@ -465,21 +480,17 @@ static int stellaris_enet_init(SysBusDevice *sbd)
     s->nic = qemu_new_nic(&net_stellaris_enet_info, &s->conf,
                           object_get_typename(OBJECT(dev)), dev->id, s);
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
 
     stellaris_enet_reset(s);
-    register_savevm(dev, "stellaris_enet", -1, 1,
-                    stellaris_enet_save, stellaris_enet_load, s);
     return 0;
 }
 
 static void stellaris_enet_unrealize(DeviceState *dev, Error **errp)
 {
     stellaris_enet_state *s = STELLARIS_ENET(dev);
 
-    unregister_savevm(DEVICE(s), "stellaris_enet", s);
-
     memory_region_destroy(&s->mmio);
 }
 
 static Property stellaris_enet_properties[] = {
     DEFINE_NIC_PROPERTIES(stellaris_enet_state, conf),
@@ -492,10 +503,11 @@ static void stellaris_enet_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = stellaris_enet_init;
     dc->unrealize = stellaris_enet_unrealize;
     dc->props = stellaris_enet_properties;
+    dc->vmsd = &vmstate_stellaris_enet;
 }
 
 static const TypeInfo stellaris_enet_info = {
     .name          = TYPE_STELLARIS_ENET,
     .parent        = TYPE_SYS_BUS_DEVICE,

Vulnerable code:     int tx_fifo_len;
    struct {
        uint8_t data[2048];
        int len;
    } rx[31];
    int rx_fifo_offset;
    int next_packet;
static void stellaris_enet_save(QEMUFile *f, void *opaque)
{
    stellaris_enet_state *s = (stellaris_enet_state *)opaque;
    int i;

    qemu_put_be32(f, s->ris);
    qemu_put_be32(f, s->im);
    qemu_put_be32(f, s->rctl);
    qemu_put_be32(f, s->tctl);
    qemu_put_be32(f, s->thr);
    qemu_put_be32(f, s->mctl);
    qemu_put_be32(f, s->mdv);
    qemu_put_be32(f, s->mtxd);
    qemu_put_be32(f, s->mrxd);
    qemu_put_be32(f, s->np);
    qemu_put_be32(f, s->tx_fifo_len);
    qemu_put_buffer(f, s->tx_fifo, sizeof(s->tx_fifo));
    for (i = 0; i < 31; i++) {
        qemu_put_be32(f, s->rx[i].len);
        qemu_put_buffer(f, s->rx[i].data, sizeof(s->rx[i].data));

    }
    qemu_put_be32(f, s->next_packet);
    qemu_put_be32(f, s->rx_fifo_offset);
}

static int stellaris_enet_load(QEMUFile *f, void *opaque, int version_id)
{
    stellaris_enet_state *s = (stellaris_enet_state *)opaque;
    int i;

    if (version_id != 1)
        return -EINVAL;

    s->ris = qemu_get_be32(f);
    s->im = qemu_get_be32(f);
    s->rctl = qemu_get_be32(f);
    s->tctl = qemu_get_be32(f);
    s->thr = qemu_get_be32(f);
    s->mctl = qemu_get_be32(f);
    s->mdv = qemu_get_be32(f);
    s->mtxd = qemu_get_be32(f);
    s->mrxd = qemu_get_be32(f);
    s->np = qemu_get_be32(f);
    s->tx_fifo_len = qemu_get_be32(f);
    qemu_get_buffer(f, s->tx_fifo, sizeof(s->tx_fifo));
    for (i = 0; i < 31; i++) {
        s->rx[i].len = qemu_get_be32(f);
        qemu_get_buffer(f, s->rx[i].data, sizeof(s->rx[i].data));

    }
    s->next_packet = qemu_get_be32(f);
    s->rx_fifo_offset = qemu_get_be32(f);

    return 0;
}

    register_savevm(dev, "stellaris_enet", -1, 1,
                    stellaris_enet_save, stellaris_enet_load, s);
    unregister_savevm(DEVICE(s), "stellaris_enet", s);

Fixed code: typedef struct {
    uint8_t data[2048];
    uint32_t len;
} StellarisEnetRxFrame;

    uint32_t tx_fifo_len;
    StellarisEnetRxFrame rx[31];
    uint32_t rx_fifo_offset;
    uint32_t next_packet;
static const VMStateDescription vmstate_rx_frame = {
    .name = "stellaris_enet/rx_frame",
    .version_id = 1,
    .minimum_version_id = 1,
    .fields = (VMStateField[]) {
        VMSTATE_UINT8_ARRAY(data, StellarisEnetRxFrame, 2048),
        VMSTATE_UINT32(len, StellarisEnetRxFrame),
        VMSTATE_END_OF_LIST()
    }
};

static int stellaris_enet_post_load(void *opaque, int version_id)
{
    stellaris_enet_state *s = opaque;
    int i;

    /* Sanitize inbound state. Note that next_packet is an index but
     * np is a size; hence their valid upper bounds differ.
     */
    if (s->next_packet >= ARRAY_SIZE(s->rx)) {
        return -1;
    }

    if (s->np > ARRAY_SIZE(s->rx)) {
        return -1;
    }

    for (i = 0; i < ARRAY_SIZE(s->rx); i++) {
        if (s->rx[i].len > ARRAY_SIZE(s->rx[i].data)) {
            return -1;
        }
    }

    if (s->rx_fifo_offset > ARRAY_SIZE(s->rx[0].data) - 4) {
        return -1;
    }

    if (s->tx_fifo_len > ARRAY_SIZE(s->tx_fifo)) {
        return -1;
    }

    return 0;
}

static const VMStateDescription vmstate_stellaris_enet = {
    .name = "stellaris_enet",
    .version_id = 2,
    .minimum_version_id = 2,
    .post_load = stellaris_enet_post_load,
    .fields = (VMStateField[]) {
        VMSTATE_UINT32(ris, stellaris_enet_state),
        VMSTATE_UINT32(im, stellaris_enet_state),
        VMSTATE_UINT32(rctl, stellaris_enet_state),
        VMSTATE_UINT32(tctl, stellaris_enet_state),
        VMSTATE_UINT32(thr, stellaris_enet_state),
        VMSTATE_UINT32(mctl, stellaris_enet_state),
        VMSTATE_UINT32(mdv, stellaris_enet_state),
        VMSTATE_UINT32(mtxd, stellaris_enet_state),
        VMSTATE_UINT32(mrxd, stellaris_enet_state),
        VMSTATE_UINT32(np, stellaris_enet_state),
        VMSTATE_UINT32(tx_fifo_len, stellaris_enet_state),
        VMSTATE_UINT8_ARRAY(tx_fifo, stellaris_enet_state, 2048),
        VMSTATE_STRUCT_ARRAY(rx, stellaris_enet_state, 31, 1,
                             vmstate_rx_frame, StellarisEnetRxFrame),
        VMSTATE_UINT32(rx_fifo_offset, stellaris_enet_state),
        VMSTATE_UINT32(next_packet, stellaris_enet_state),
        VMSTATE_END_OF_LIST()
    }
};

    dc->vmsd = &vmstate_stellaris_enet;