[PATCH] media: gspca: tv8532: bound packet length before frame_add

[PATCH] media: gspca: tv8532: bound packet length before frame_add

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

sd_pkt_scan() in the tv8532 sub-driver hands the two image lines of a
packet to gspca_frame_add() using gspca_dev->pixfmt.width as the copy
length, without ever looking at the actual length (len) of the received
isoc/bulk packet:

	gspca_frame_add(gspca_dev, packet_type0,
			data + 2, gspca_dev->pixfmt.width);
	gspca_frame_add(gspca_dev, packet_type1,
			data + gspca_dev->pixfmt.width + 5,
			gspca_dev->pixfmt.width);

gspca_frame_add() only bounds the destination frame buffer; it copies
'len' bytes from the source 'data' unchecked. 'data' points into the URB
transfer buffer at iso_frame_desc[i].offset, and the isoc completion
(fill_frame()) forwards whatever actual_length the device reported (it
drops only zero-length packets). A camera -- or an emulated /
USB-redirected device -- that sends a packet shorter than 2 * width + 5
bytes (709 bytes in the 352x288 mode) therefore makes the parser read up
to that many bytes from the transfer buffer, past the data actually
received for the packet. Those bytes are emitted into the userspace
video frame, disclosing stale transfer-buffer contents; for a packet
positioned at the end of the coherent URB buffer the over-read may also
reach past the allocation, depending on the endpoint's packet geometry.

Reject packets too short to contain the two lines the parser
unconditionally copies. A short packet makes the current frame
incomplete, which gspca already discards; the stream resynchronises at
the next frame boundary.

Found by code review of the sd_pkt_scan() idiom shared across the gspca
sub-drivers. Not reproduced (no hardware); analysis only.

Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 drivers/media/usb/gspca/tv8532.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/media/usb/gspca/tv8532.c b/drivers/media/usb/gspca/tv8532.c
--- a/drivers/media/usb/gspca/tv8532.c
+++ b/drivers/media/usb/gspca/tv8532.c
@@ -251,6 +251,16 @@ static void sd_pkt_scan(struct gspca_dev *gspca_dev,
 	struct sd *sd = (struct sd *) gspca_dev;
 	int packet_type0, packet_type1;

+	/*
+	 * Both gspca_frame_add() calls below read pixfmt.width bytes from
+	 * the packet without consulting len, the last one starting at
+	 * data + width + 5.  Drop packets that are too short to hold both
+	 * lines, otherwise the device can make us read past the end of the
+	 * received URB data.
+	 */
+	if (len < 2 * gspca_dev->pixfmt.width + 5)
+		return;
+
 	packet_type0 = packet_type1 = INTER_PACKET;
 	if (gspca_dev->empty_packet) {
 		gspca_dev->empty_packet = 0;