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 <[email protected]> --- 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)
