Merge pull request #3741 from dxbjavid/ncm-ndp-length-bound

bound ndp16 wLength against received ntb in recv_validate_datagram

5 files changed, 214 insertions, 1 deletions

diff --git a/src/class/net/ncm_device.c b/src/class/net/ncm_device.c
index d828ab97c..84a524f49 100644
--- a/src/class/net/ncm_device.c
+++ b/src/class/net/ncm_device.c
@@ -602,7 +602,7 @@ static bool recv_validate_datagram(const recv_ntb_t *ntb, uint32_t len) {
     TU_LOG_DRV("(EE) ill block length2: %d > %d\n", nth16->wBlockLength, CFG_TUD_NCM_OUT_NTB_MAX_SIZE);
     return false;
   }
-  if (nth16->wNdpIndex < sizeof(nth16) || nth16->wNdpIndex > len - (sizeof(ndp16_t) + 2 * sizeof(ndp16_datagram_t))) {
+  if (nth16->wNdpIndex < sizeof(nth16_t) || nth16->wNdpIndex > len - (sizeof(ndp16_t) + 2 * sizeof(ndp16_datagram_t))) {
     TU_LOG_DRV("(EE) ill position of first ndp: %d (%lu)\n", nth16->wNdpIndex, len);
     return false;
   }
@@ -614,6 +614,12 @@ static bool recv_validate_datagram(const recv_ntb_t *ntb, uint32_t len) {
     TU_LOG_DRV("(EE) ill ndp16 length: %d\n", ndp16->wLength);
     return false;
   }
+  // the NDP block (wLength bytes from wNdpIndex) must fit within the received NTB, otherwise the
+  // datagram pointer array walked below (max_ndx is derived from wLength) runs past ntb->data
+  if ((uint32_t) nth16->wNdpIndex + ndp16->wLength > len) {
+    TU_LOG_DRV("(EE) ill ndp16 length: %d (%lu)\n", ndp16->wLength, len);
+    return false;
+  }
   if (ndp16->dwSignature != NDP16_SIGNATURE_NCM0 && ndp16->dwSignature != NDP16_SIGNATURE_NCM1) {
     TU_LOG_DRV("(EE) ill signature: 0x%08x\n", (unsigned) ndp16->dwSignature);
     return false;
diff --git a/test/fuzz/device/net_ncm/Makefile b/test/fuzz/device/net_ncm/Makefile
new file mode 100644
index 000000000..df5a61324
--- /dev/null
+++ b/test/fuzz/device/net_ncm/Makefile
@@ -0,0 +1,43 @@
+# Focused fuzz harness for the NCM receive path (recv_validate_datagram).
+#
+# It is self-contained: it #includes src/class/net/ncm_device.c and stubs the
+# few usbd/glue symbols, so it does not link the whole device stack.
+#
+#   make            # build the libFuzzer target: _build/net_ncm
+#   make regression # build + replay the crafted seed under ASan (no engine)
+#   make clean
+
+TOP := ../../../..
+BUILD := _build
+
+CC ?= clang
+
+INC += \
+  $(TOP)/src \
+  .
+
+CFLAGS += $(addprefix -I,$(INC)) -g -O1 -fsanitize=address
+
+FUZZ_FLAGS := -fsanitize=fuzzer
+
+.PHONY: all regression clean get-deps
+
+all: $(BUILD)/net_ncm
+
+# No external dependencies (the harness only needs the in-tree NCM driver).
+get-deps:
+
+# libFuzzer supplies its own main(), so drop the standalone driver here.
+$(BUILD)/net_ncm: fuzz.c
+	mkdir -p $(@D)
+	$(CC) $(CFLAGS) $(FUZZ_FLAGS) -DNO_MAIN -o $@ $<
+
+# Standalone build with a plain main(): replays the crafted seed and any extra
+# corpus files given as arguments. Used as a deterministic regression.
+regression: fuzz.c
+	mkdir -p $(BUILD)
+	$(CC) $(CFLAGS) -o $(BUILD)/net_ncm_regression $<
+	./$(BUILD)/net_ncm_regression
+
+clean:
+	$(RM) -rf $(BUILD) *.dSYM
diff --git a/test/fuzz/device/net_ncm/fuzz.c b/test/fuzz/device/net_ncm/fuzz.c
new file mode 100644
index 000000000..a93c144f7
--- /dev/null
+++ b/test/fuzz/device/net_ncm/fuzz.c
@@ -0,0 +1,140 @@
+/*
+ * SPDX-FileCopyrightText: Copyright (c) 2026 Ha Thach (tinyusb.org)
+ * SPDX-License-Identifier: MIT
+ *
+ * This file is part of the TinyUSB stack.
+ */
+
+// Focused fuzz harness for the NCM receive path. It feeds a raw NCM Transfer
+// Block (NTB), i.e. host-controlled bytes off the OUT endpoint, straight into
+// recv_validate_datagram() which is the function that decides whether an
+// incoming NTB is well formed before the driver walks its datagram array.
+//
+// recv_validate_datagram() is static, so the driver is pulled in by #include so
+// the harness can reach it. The referenced usbd/glue symbols are stubbed below;
+// none of them are exercised by the validation path.
+//
+// The seed in net_ncm_seed_corpus.zip crafts an NTB whose first NDP carries a
+// wLength of 0xfff0 while the transfer itself is only 64 bytes. Before the
+// wNdpIndex + wLength bound was added, max_ndx was derived from that wLength and
+// the ndp16_datagram[] walk ran tens of kB past ntb->data (a 3200 byte buffer);
+// this reproduces that out-of-bounds read under -fsanitize=address.
+
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "class/net/ncm_device.c"
+
+//--------------------------------------------------------------------+
+// Stubs: referenced by the NCM driver, unused by recv_validate_datagram
+//--------------------------------------------------------------------+
+bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_t total_bytes, bool is_isr) {
+  (void) rhport; (void) ep_addr; (void) buffer; (void) total_bytes; (void) is_isr;
+  return true;
+}
+bool usbd_edpt_busy(uint8_t rhport, uint8_t ep_addr) {
+  (void) rhport; (void) ep_addr;
+  return false;
+}
+bool usbd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const *desc_ep) {
+  (void) rhport; (void) desc_ep;
+  return true;
+}
+bool usbd_open_edpt_pair(uint8_t rhport, uint8_t const *p_desc, uint8_t ep_count, uint8_t xfer_type,
+                         uint8_t *ep_out, uint8_t *ep_in) {
+  (void) rhport; (void) p_desc; (void) ep_count; (void) xfer_type; (void) ep_out; (void) ep_in;
+  return true;
+}
+bool tud_control_xfer(uint8_t rhport, tusb_control_request_t const *request, void *buffer, uint16_t len) {
+  (void) rhport; (void) request; (void) buffer; (void) len;
+  return true;
+}
+bool tud_control_status(uint8_t rhport, tusb_control_request_t const *request) {
+  (void) rhport; (void) request;
+  return true;
+}
+tusb_speed_t tud_speed_get(void) {
+  return TUSB_SPEED_FULL;
+}
+bool tud_network_recv_cb(const uint8_t *src, uint16_t size) {
+  (void) src; (void) size;
+  return true;
+}
+uint16_t tud_network_xmit_cb(uint8_t *dst, void *ref, uint16_t arg) {
+  (void) dst; (void) ref; (void) arg;
+  return 0;
+}
+
+//--------------------------------------------------------------------+
+// Fuzz entry
+//--------------------------------------------------------------------+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
+  // Give the NTB a heap allocation of the exact receive-buffer size so any read
+  // past ntb->data is caught by AddressSanitizer.
+  recv_ntb_t *ntb = (recv_ntb_t *) malloc(sizeof(recv_ntb_t));
+  if (ntb == NULL) {
+    return 0;
+  }
+  size_t n = size < sizeof(ntb->data) ? size : sizeof(ntb->data);
+  memset(ntb->data, 0, sizeof(ntb->data));
+  memcpy(ntb->data, data, n);
+  recv_validate_datagram(ntb, (uint32_t) n);
+  free(ntb);
+  return 0;
+}
+
+//--------------------------------------------------------------------+
+// Standalone driver (built when there is no libFuzzer engine): replays the
+// crafted NTB, or any corpus files passed on the command line.
+//--------------------------------------------------------------------+
+#ifndef NO_MAIN
+static void run_crafted_seed(void) {
+  uint8_t buf[64];
+  memset(buf, 0, sizeof(buf));
+
+  // NTH16: signature, wHeaderLength = sizeof(nth16_t), wSequence, wBlockLength, wNdpIndex
+  uint32_t nth_sig = NTH16_SIGNATURE;
+  memcpy(buf + 0, &nth_sig, 4);
+  uint16_t v;
+  v = sizeof(nth16_t); memcpy(buf + 4, &v, 2);
+  v = 0;               memcpy(buf + 6, &v, 2);
+  v = sizeof(buf);     memcpy(buf + 8, &v, 2);
+  v = sizeof(nth16_t); memcpy(buf + 10, &v, 2);
+
+  // NDP16 at wNdpIndex: signature, wLength (unbounded pre-fix), wNextNdpIndex
+  uint32_t ndp_sig = NDP16_SIGNATURE_NCM0;
+  memcpy(buf + sizeof(nth16_t) + 0, &ndp_sig, 4);
+  v = 0xFFF0; memcpy(buf + sizeof(nth16_t) + 4, &v, 2);
+  v = 0;      memcpy(buf + sizeof(nth16_t) + 6, &v, 2);
+
+  LLVMFuzzerTestOneInput(buf, sizeof(buf));
+}
+
+int main(int argc, char **argv) {
+  if (argc < 2) {
+    run_crafted_seed();
+    return 0;
+  }
+  for (int i = 1; i < argc; i++) {
+    FILE *f = fopen(argv[i], "rb");
+    if (f == NULL) {
+      continue;
+    }
+    fseek(f, 0, SEEK_END);
+    long len = ftell(f);
+    fseek(f, 0, SEEK_SET);
+    if (len > 0) {
+      uint8_t *buf = (uint8_t *) malloc((size_t) len);
+      if (buf != NULL && fread(buf, 1, (size_t) len, f) == (size_t) len) {
+        LLVMFuzzerTestOneInput(buf, (size_t) len);
+      }
+      free(buf);
+    }
+    fclose(f);
+  }
+  return 0;
+}
+#endif
diff --git a/test/fuzz/device/net_ncm/net_ncm_seed_corpus.zip b/test/fuzz/device/net_ncm/net_ncm_seed_corpus.zip
new file mode 100644
index 000000000..a4b481d0b
--- /dev/null
+++ b/test/fuzz/device/net_ncm/net_ncm_seed_corpus.zip
diff --git a/test/fuzz/device/net_ncm/tusb_config.h b/test/fuzz/device/net_ncm/tusb_config.h
new file mode 100644
index 000000000..b5e3a1fb4
--- /dev/null
+++ b/test/fuzz/device/net_ncm/tusb_config.h
@@ -0,0 +1,24 @@
+/*
+ * SPDX-FileCopyrightText: Copyright (c) 2026 Ha Thach (tinyusb.org)
+ * SPDX-License-Identifier: MIT
+ *
+ * This file is part of the TinyUSB stack.
+ */
+
+#ifndef _TUSB_CONFIG_H_
+#define _TUSB_CONFIG_H_
+
+// Minimal configuration to build the NCM device driver for the receive-path
+// fuzz harness. Only the NCM class is enabled; no root hub / DCD is needed
+// because the harness drives recv_validate_datagram() directly.
+#define CFG_TUSB_MCU            OPT_MCU_NONE
+#define CFG_TUSB_OS             OPT_OS_NONE
+
+#define CFG_TUD_ENABLED        1
+#define CFG_TUD_ENDPOINT0_SIZE  64
+
+// Network class has 2 drivers: ECM/RNDIS and NCM. Fuzz the NCM parser here.
+#define CFG_TUD_ECM_RNDIS      0
+#define CFG_TUD_NCM            1
+
+#endif