ttusb-dec: possible device-controlled overflow in ttusb_dec_send_command()

[Maoyi Xie]

Hi all,

I think ttusb_dec_send_command() in drivers/media/usb/ttusb-dec/ttusb_dec.c
can overflow the caller buffer when a malicious decoder returns a large
reply length. I would appreciate it if you could take a look.

The function reads the device reply into a 64 byte heap buffer, then copies
it out with a length byte that also comes from the device.

	b = kzalloc(COMMAND_PACKET_SIZE + 4, GFP_KERNEL);
	...
	usb_bulk_msg(dec->udev, dec->result_pipe, b,
		     COMMAND_PACKET_SIZE + 4, &actual_len, 1000);
	...
	if (cmd_result && b[3] > 0)
		memcpy(cmd_result, &b[4], b[3]);

b[3] is a u8 the device controls. The only check is b[3] > 0, there is no
upper bound, and actual_len is not used. Both in-file callers pass an on
stack buffer of COMMAND_PACKET_SIZE (0x3c, 60) bytes.

	u8 c[COMMAND_PACKET_SIZE];
	ttusb_dec_send_command(dec, 0x08, 0, NULL, &c_length, c);

So b[3] = 255 writes 255 bytes into a 60 byte buffer, a 195 byte stack
overflow, and also reads 195 bytes past the 64 byte b allocation. The first
caller is ttusb_dec_get_stb_state(), which runs at probe through
ttusb_dec_init_stb(), so plugging in a malicious USB DVB decoder is enough
to reach it.

The length looks like it is meant to be bounded. The 2019 fix a10feaf8c464
("media: ttusb-dec: Fix info-leak in ttusb_dec_send_command()") hardened
this function but left the reply length unchecked, and the firmware path in
the same driver clamps to COMMAND_PACKET_SIZE before copying. Clamping b[3]
to COMMAND_PACKET_SIZE before the copy would close it.

I reproduced the write on 7.1-rc7 by running the same copy with b[3] = 255
into a 60 byte stack buffer. It corrupts the stack canary and panics.

  Kernel panic - not syncing: stack-protector: Kernel stack is corrupted

Does this look like a real bug, and is clamping b[3] to COMMAND_PACKET_SIZE
the right fix? If so I am happy to send a proper patch with a Fixes tag and
Cc stable.

Thanks,
Maoyi
https://maoyixie.com/