On 8/18/21 2:05 PM, Gerd Hoffmann wrote:
> The device uses the guest-supplied stream number unchecked, which can
> lead to guest-triggered out-of-band access to the UASDevice->data3 and
> UASDevice->status3 fields. Add the missing checks.
>
> Fixes: CVE-2021-3713
Reported-by: Chen Zhe <chen...@huawei.com>
Reported-by: Tan Jingguo <tanjing...@huawei.com>
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> ---
> hw/usb/dev-uas.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
> index 263056231c79..f6309a5ebfdc 100644
> --- a/hw/usb/dev-uas.c
> +++ b/hw/usb/dev-uas.c
> @@ -840,6 +840,9 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket
> *p)
> }
> break;
> case UAS_PIPE_ID_STATUS:
> + if (p->stream > UAS_MAX_STREAMS) {
> + goto err_stream;
> + }
> if (p->stream) {
> QTAILQ_FOREACH(st, &uas->results, next) {
> if (st->stream == p->stream) {
> @@ -867,6 +870,9 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket
> *p)
> break;
> case UAS_PIPE_ID_DATA_IN:
> case UAS_PIPE_ID_DATA_OUT:
> + if (p->stream > UAS_MAX_STREAMS) {
> + goto err_stream;
> + }
> if (p->stream) {
> req = usb_uas_find_request(uas, p->stream);
> } else {
> @@ -902,6 +908,11 @@ static void usb_uas_handle_data(USBDevice *dev,
> USBPacket *p)
> p->status = USB_RET_STALL;
> break;
> }
> +
> +err_stream:
> + error_report("%s: invalid stream %d", __func__, p->stream);
> + p->status = USB_RET_STALL;
> + return;
> }
Thanks for tackling this, I was looking but thought it was too
late for 6.1. Beside your obvious checks, I'd also modify
usb_uas_alloc_status() to feel safer, by checking 'tag'.
Eventually returning NULL and adapt the call sites? Anyway
this can wait 6.2, so for this patch:
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
