device: clamp EP0 OUT data copy to the control transfer buffer (#3705)

* device: clamp EP0 OUT data copy to the control transfer buffer

usbd_control_xfer_cb() copied xferred_bytes from the EP0 bounce buffer
into the requester's buffer with no bound. A non-compliant host that
sends an OUT data packet larger than the control transfer's data_len
(= min(len, wLength), the buffer capacity) would overflow that buffer
and over-count total_xferred. Clamp xferred_bytes to the remaining
buffer space before the memcpy and accounting.

2 files changed, 53 insertions, 1 deletions

diff --git a/src/device/usbd.c b/src/device/usbd.c
index 55ad330c1..f87b63111 100644
--- a/src/device/usbd.c
+++ b/src/device/usbd.c
@@ -914,6 +914,8 @@ static bool usbd_control_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t
   // Data stage progress
   if (ctrl_xfer->request.bmRequestType_bit.direction == TUSB_DIR_OUT) {
     TU_VERIFY(ctrl_xfer->buffer);
+    // Clamp host overrun to remaining capacity (data_len) so memcpy can't overflow the caller buffer
+    xferred_bytes = tu_min32(xferred_bytes, ctrl_xfer->data_len - ctrl_xfer->total_xferred);
     if (ctrl_xfer->buffer != _ctrl_epbuf.buf) {
       memcpy(ctrl_xfer->buffer, _ctrl_epbuf.buf, xferred_bytes);
     }
diff --git a/test/unit-test/test/device/usbd/test_usbd.c b/test/unit-test/test/device/usbd/test_usbd.c
index 3a2cf3217..7f3c3f5b2 100644
--- a/test/unit-test/test/device/usbd/test_usbd.c
+++ b/test/unit-test/test/device/usbd/test_usbd.c
@@ -29,7 +29,7 @@
 #include "tusb_fifo.h"
 #include "tusb.h"
 #include "usbd.h"
-TEST_SOURCE_FILE("usbd_control.c")
+TEST_SOURCE_FILE("usbd.c")
 
 // Mock File
 #include "mock_dcd.h"
@@ -100,6 +100,16 @@ tusb_control_request_t const req_get_desc_configuration =
   .wLength = 256
 };
 
+// Vendor OUT control request (direction OUT, type Vendor, recipient Device), 8-byte data stage
+tusb_control_request_t const req_vendor_out =
+{
+  .bmRequestType = 0x40,
+  .bRequest = 0x01,
+  .wValue = 0x0000,
+  .wIndex = 0x0000,
+  .wLength = 8
+};
+
 uint8_t const* desc_device;
 uint8_t const* desc_configuration;
 
@@ -120,6 +130,19 @@ uint16_t const* tud_descriptor_string_cb(uint8_t index, uint16_t langid) {
   return NULL;
 }
 
+// Backing buffer for the vendor OUT data stage. Sized to EP0 max packet so an (untested) regression
+// that drops the clamp can't corrupt memory here; the regression is caught by the expectation below.
+static uint8_t vendor_out_buf[CFG_TUD_ENDPOINT0_SIZE];
+
+bool tud_vendor_control_xfer_cb(uint8_t rhport_, uint8_t stage, tusb_control_request_t const* request) {
+  (void) request;
+  if (stage == CONTROL_STAGE_SETUP) {
+    // Offer only an 8-byte capacity even though the data stage may receive a larger packet
+    return tud_control_xfer(rhport_, request, vendor_out_buf, 8);
+  }
+  return true;
+}
+
 void setUp(void) {
   dcd_int_disable_Ignore();
   dcd_int_enable_Ignore();
@@ -246,3 +269,30 @@ void test_usbd_control_in_zlp(void)
 
   tud_task();
 }
+
+//--------------------------------------------------------------------+
+// Control OUT data stage host overrun
+//--------------------------------------------------------------------+
+
+// A non-compliant host sends an OUT data packet larger than the buffer the class offered:
+// wLength = 8, but the DCD reports a full CFG_TUD_ENDPOINT0_SIZE packet. usbd must clamp the
+// copy/accounting to the 8-byte capacity so total_xferred reaches wLength, ends the data stage,
+// and queues the IN status stage. Without the clamp total_xferred overshoots wLength and usbd
+// re-arms an OUT data packet (EDPT_CTRL_OUT) instead, failing the EDPT_CTRL_IN expectation below.
+void test_usbd_control_out_overrun_clamp(void)
+{
+  dcd_event_setup_received(rhport, (uint8_t*) &req_vendor_out, false);
+
+  // Data stage: usbd arms an 8-byte OUT into its internal bounce buffer (buffer ptr is internal)
+  dcd_edpt_xfer_ExpectAndReturn(rhport, EDPT_CTRL_OUT, NULL, 8, false, true);
+  dcd_edpt_xfer_IgnoreArg_buffer();
+  // Host overrun: DCD reports a full max packet, larger than the 8-byte capacity
+  dcd_event_xfer_complete(rhport, EDPT_CTRL_OUT, CFG_TUD_ENDPOINT0_SIZE, XFER_RESULT_SUCCESS, false);
+
+  // Clamp -> total_xferred == wLength -> data stage done -> IN status stage queued
+  dcd_edpt_xfer_ExpectAndReturn(rhport, EDPT_CTRL_IN, NULL, 0, false, true);
+  dcd_event_xfer_complete(rhport, EDPT_CTRL_IN, 0, 0, false);
+  dcd_edpt0_status_complete_ExpectWithArray(rhport, &req_vendor_out, 1);
+
+  tud_task();
+}