kern/58095: possible device-controlled buffer overrun in uvideo(4)

[campbell+netbsd%mumble.net@localhost]

>Number:         58095
>Category:       kern
>Synopsis:       possible device-controlled buffer overrun in uvideo(4)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Apr 01 02:10:00 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, 8, ...
>Organization:
The UNetBSD(4) Foundation
>Environment:
>Description:
uvideo_stream_recv_process verifies that the frame length is at least a header's worth (2 bytes):

   1881 	if (len < sizeof(uvideo_payload_header_t)) {
   1882 		DPRINTF(("uvideo_stream_recv_process: len %d < payload hdr\n",
   1883 			 len));
   1884 		return USBD_SHORT_XFER;
   1885 	}

https://nxr.netbsd.org/xref/src/sys/dev/usb/uvideo.c?r=1.85#1881

And it verifies that the header's self-described length is at least a header's worth (2 bytes), and at most 12 bytes:

   1889 	if (hdr->bHeaderLength > UVIDEO_PAYLOAD_HEADER_SIZE ||
   1890 	    hdr->bHeaderLength < sizeof(uvideo_payload_header_t))
   1891 		return USBD_INVAL;

https://nxr.netbsd.org/xref/src/sys/dev/usb/uvideo.c?r=1.85#1889

But it proceeds to compute the payload length as len - hdr->bHeaderLength without first verifying that the frame length is at least as large as the header's self-described length.  So we could have, for example, len=2 and hdr->bHeaderLength=3, in which case len - hdr->bHeaderLength (computed in uint32_t like len) yields 0xffffffff = 4294967295, when it passes the result into video(4):

   1897 	payload.data = buf + hdr->bHeaderLength;
   1898 	payload.size = len - hdr->bHeaderLength;
   1899 	payload.frameno = hdr->bmHeaderInfo & UV_FRAME_ID;
   1900 	payload.end_of_frame = hdr->bmHeaderInfo & UV_END_OF_FRAME;
   1901 
   1902 	video_submit_payload(vs->vs_videodev, &payload);

https://nxr.netbsd.org/xref/src/sys/dev/usb/uvideo.c?r=1.85#1897
>How-To-Repeat:
code inspection
malicious uvideo(4) device, e.g. via vhci(4)
>Fix:
change `hdr->bHeaderLength < sizeof(...)' to `hdr->bHeaderLength < len'