[PATCH] hw/usb/u2f: validate ring buffer indices on migration load

[PATCH] hw/usb/u2f: validate ring buffer indices on migration load

[Bin Guo]

The U2F key pending_in ring buffer uses uint8_t start/end/num indices
over a 32-entry array, all serialized in the vmstate.  A malicious
migration stream can inject values >= U2FHID_PENDING_IN_NUM (32),
causing an out-of-bounds heap read in u2f_pending_in_get() on the
first USB IN token after migration completes.

Add a post_load callback to validate the restored indices and reset
the ring buffer to empty on any out-of-range value.  Pending packets
are non-critical (the device will regenerate them), so resetting the
ring is preferable to rejecting migration entirely.

As defense-in-depth, also add a bounds clamp in u2f_pending_in_get()
so that even if corrupted state somehow reaches the consumer path,
the index is folded into range before array access.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4483
Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 hw/usb/u2f.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/hw/usb/u2f.c b/hw/usb/u2f.c
index d6291852f6..9ca9879f3e 100644
--- a/hw/usb/u2f.c
+++ b/hw/usb/u2f.c
@@ -228,6 +228,11 @@ static uint8_t *u2f_pending_in_get(U2FKeyState *key)
         return NULL;
     }
 
+    /* Clamp to valid range in case of corrupted state (e.g. migration) */
+    if (key->pending_in_start >= U2FHID_PENDING_IN_NUM) {
+        key->pending_in_start %= U2FHID_PENDING_IN_NUM;
+    }
+
     index = key->pending_in_start;
     key->pending_in_start = (index + 1) % U2FHID_PENDING_IN_NUM;
     --key->pending_in_num;
@@ -301,10 +306,36 @@ static void u2f_key_realize(USBDevice *dev, Error **errp)
     key->ep = usb_ep_get(dev, USB_TOKEN_IN, 1);
 }
 
+static int u2f_key_post_load(void *opaque, int version_id)
+{
+    U2FKeyState *key = opaque;
+
+    /*
+     * Validate pending_in ring buffer indices restored from the migration
+     * stream.  An attacker-controlled stream could inject values >= 32
+     * (U2FHID_PENDING_IN_NUM), causing out-of-bounds heap access in
+     * u2f_pending_in_get() on the first IN token after migration.
+     *
+     * Reset the ring on any invalid value: pending packets are non-critical
+     * (they will be regenerated by the device), so rejecting migration
+     * entirely is not necessary.
+     */
+    if (key->pending_in_start >= U2FHID_PENDING_IN_NUM ||
+        key->pending_in_end >= U2FHID_PENDING_IN_NUM ||
+        key->pending_in_num > U2FHID_PENDING_IN_NUM) {
+        key->pending_in_start = 0;
+        key->pending_in_end = 0;
+        key->pending_in_num = 0;
+    }
+
+    return 0;
+}
+
 const VMStateDescription vmstate_u2f_key = {
     .name = "u2f-key",
     .version_id = 1,
     .minimum_version_id = 1,
+    .post_load = u2f_key_post_load,
     .fields = (const VMStateField[]) {
         VMSTATE_USB_DEVICE(dev, U2FKeyState),
         VMSTATE_UINT8(idle, U2FKeyState),
-- 
2.50.1 (Apple Git-155)