From f13be4928b1e32127ce71325fbf50f0e943c0566 Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Wed, 4 Jan 2012 23:33:07 +0100 Subject: [PATCH] Pad the transfer buffer to prevent out of bounds access. The HIDReportItem reads 32 bit chunks from the report buffer. To avoid having to check the remaining buffer space on each extraction, we pad the buffer so it is always valid to read 32 bits at a time. Also add a comment explaining why we do it that way. Thanks to Johannes Anderwald for pointing out the potential out of bound access! --- src/add-ons/kernel/drivers/input/usb_hid/HIDDevice.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/add-ons/kernel/drivers/input/usb_hid/HIDDevice.cpp b/src/add-ons/kernel/drivers/input/usb_hid/HIDDevice.cpp index 8028d4d663..aeacd6e43f 100644 --- a/src/add-ons/kernel/drivers/input/usb_hid/HIDDevice.cpp +++ b/src/add-ons/kernel/drivers/input/usb_hid/HIDDevice.cpp @@ -186,7 +186,11 @@ HIDDevice::HIDDevice(usb_device device, const usb_configuration_info *config, return; } - fTransferBuffer = (uint8 *)malloc(fTransferBufferSize); + // We pad the allocation size so that we can always read 32 bits at a time + // (as done in HIDReportItem) without the need for an additional boundary + // check. We don't increase the transfer buffer size though as to not expose + // this implementation detail onto the device when scheduling transfers. + fTransferBuffer = (uint8 *)malloc(fTransferBufferSize + 3); if (fTransferBuffer == NULL) { TRACE_ALWAYS("failed to allocate transfer buffer\n"); fStatus = B_NO_MEMORY;