On Mon, Jan 13, 2025 at 05:38:56PM +0800, Hongren Zheng wrote:
> When USBPacket in OUT direction has larger payload
> than the ep_out_buffer (of size 512), a buffer overflow
> would occur.
>
> It could be fixed by limiting the size of usb_packet_copy
> to be at most buffer size. Further optimization gets rid
> of the ep_out_buffer and directly uses ep_out as the target
> buffer.
>
> This is reported by a security researcher who artificially
> constructed an OUT packet of size 2047. The report has gone
> through the QEMU security process, and as this device is for
> testing purpose and no deployment of it in virtualization
> environment is observed, it is triaged not to be a security bug.
>
> Reported-by: Juan Jose Lopez Jaimez <thatjia...@gmail.com>
> Signed-off-by: Hongren Zheng <i...@zenithal.me>
> ---
> hw/usb/canokey.c | 6 +++---
> hw/usb/canokey.h | 4 ----
> 2 files changed, 3 insertions(+), 7 deletions(-)
Seems that the USB subsystem has been orphaned
and there is no maintainer now.
I used to ask the USB maintainer to pass the patch
because I could not send a PULL, which requires
gpg signature.
I wonder which maintainer I should ask for now.
>
> diff --git a/hw/usb/canokey.c b/hw/usb/canokey.c
> index fae212f053..e2d66179e0 100644
> --- a/hw/usb/canokey.c
> +++ b/hw/usb/canokey.c
> @@ -197,8 +197,8 @@ static void canokey_handle_data(USBDevice *dev, USBPacket
> *p)
> switch (p->pid) {
> case USB_TOKEN_OUT:
> trace_canokey_handle_data_out(ep_out, p->iov.size);
> - usb_packet_copy(p, key->ep_out_buffer[ep_out], p->iov.size);
> out_pos = 0;
> + /* segment packet into (possibly multiple) ep_out */
> while (out_pos != p->iov.size) {
> /*
> * key->ep_out[ep_out] set by prepare_receive
> @@ -207,8 +207,8 @@ static void canokey_handle_data(USBDevice *dev, USBPacket
> *p)
> * to be the buffer length
> */
> out_len = MIN(p->iov.size - out_pos, key->ep_out_size[ep_out]);
> - memcpy(key->ep_out[ep_out],
> - key->ep_out_buffer[ep_out] + out_pos, out_len);
> + /* usb_packet_copy would update the pos offset internally */
> + usb_packet_copy(p, key->ep_out[ep_out], out_len);
> out_pos += out_len;
> /* update ep_out_size to actual len */
> key->ep_out_size[ep_out] = out_len;
> diff --git a/hw/usb/canokey.h b/hw/usb/canokey.h
> index e528889d33..1b60d73485 100644
> --- a/hw/usb/canokey.h
> +++ b/hw/usb/canokey.h
> @@ -24,8 +24,6 @@
> #define CANOKEY_EP_NUM 3
> /* BULK/INTR IN can be up to 1352 bytes, e.g. get key info */
> #define CANOKEY_EP_IN_BUFFER_SIZE 2048
> -/* BULK OUT can be up to 270 bytes, e.g. PIV import cert */
> -#define CANOKEY_EP_OUT_BUFFER_SIZE 512
>
> typedef enum {
> CANOKEY_EP_IN_WAIT,
> @@ -59,8 +57,6 @@ typedef struct CanoKeyState {
> /* OUT pointer to canokey recv buffer */
> uint8_t *ep_out[CANOKEY_EP_NUM];
> uint32_t ep_out_size[CANOKEY_EP_NUM];
> - /* For large BULK OUT, multiple write to ep_out is needed */
> - uint8_t ep_out_buffer[CANOKEY_EP_NUM][CANOKEY_EP_OUT_BUFFER_SIZE];
>
> /* Properties */
> char *file; /* canokey-file */
> --
> 2.47.1
>
