NetBSD Problem Report #58095

From www@netbsd.org  Mon Apr  1 02:05:48 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 79A8F1A9239
	for <gnats-bugs@gnats.NetBSD.org>; Mon,  1 Apr 2024 02:05:48 +0000 (UTC)
Message-Id: <20240401020546.0CE271A923B@mollari.NetBSD.org>
Date: Mon,  1 Apr 2024 02:05:46 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: possible device-controlled buffer overrun in uvideo(4)
X-Send-Pr-Version: www-1.0

>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'

Home	PR Database Search