ALSA: caiaq: possible out of bounds read in Traktor Kontrol S4 dispatch

[Maoyi Xie]

Hi all,

I think the Traktor Kontrol S4 input path in sound/usb/caiaq/input.c can read
past the end of the URB buffer when the device sends a reply whose length is
not a multiple of 16. I would appreciate it if you could take a look.

The dispatch loop walks the reply in fixed 16 byte blocks.

	while (len) {
		unsigned int i, block_id = (buf[0] << 8) | buf[1];
		...
		len -= TKS4_MSGBLOCK_SIZE;
		buf += TKS4_MSGBLOCK_SIZE;
	}

Each pass reads up to buf[15] and then does len -= 16. But len is the raw
device length. It comes straight from urb->actual_length in
snd_usb_caiaq_ep4_reply_dispatch and there is no check that it is a multiple
of 16. The X1 and Maschine arms in that same handler do bound check the
length, but the S4 arm does not.

If len is not a multiple of 16, say 8, the final len -= 16 underflows because
len is unsigned. The loop condition while (len) then stays true and the walk
keeps consuming 16 byte blocks far past the end of the buffer. That is an out
of bounds read. The buffer is cdev->ep4_in_buf of size EP4_BUFSIZE.

The attacker model is a malicious or emulated USB device that claims the
Native Instruments Traktor Kontrol S4 id and returns a short reply on its bulk
in endpoint once the input device is opened.

I reproduced this under KASAN on 7.1-rc7. A length of one whole block runs the
loop once and reads nothing past the buffer. A length of 8 underflows and
KASAN reports a slab out of bounds read just past the allocation.

The fix I tried is to only consume a block when a whole block is present.

	-	while (len) {
	+	while (len >= TKS4_MSGBLOCK_SIZE) {

That stops the decrement from underflowing and drops any trailing partial
block.

Does this look like a real bug to you? If the direction is right I am happy to
send a proper patch with a Fixes tag pointing at 15c5ab607045 ("ALSA:
snd-usb-caiaq: Add support for Traktor Kontrol S4").

Thanks,
Maoyi
https://maoyixie.com/