Skip to content

bound ndp16 wLength against received ntb in recv_validate_datagram#3741

Open
dxbjavid wants to merge 1 commit into
hathach:masterfrom
dxbjavid:ncm-ndp-length-bound
Open

bound ndp16 wLength against received ntb in recv_validate_datagram#3741
dxbjavid wants to merge 1 commit into
hathach:masterfrom
dxbjavid:ncm-ndp-length-bound

Conversation

@dxbjavid

Copy link
Copy Markdown
Contributor

the ncm driver validates each incoming ntb in recv_validate_datagram before passing it to the glue logic. the first ndp's wLength is the size of the datagram pointer block and it comes straight from the host on the out endpoint, but the function only checks it against a minimum and never an upper bound. max_ndx is then derived from wLength and used to index ndp16_datagram[max_ndx - 1], and to walk the array, inside ntb->data which is only CFG_TUD_NCM_OUT_NTB_MAX_SIZE bytes (3200 by default). a short transfer carrying a wLength near 0xffff yields a max_ndx of roughly 16000, so the validator reads tens of kilobytes past the receive buffer while it is still deciding whether the packet is valid. i came across it while reading the bounds logic around wNdpIndex, which is capped against len whereas wLength is not. rejecting the ntb when wNdpIndex + wLength runs past the received length keeps the array within the buffer and lines up with the checks already done just above.

@github-actions

Copy link
Copy Markdown

MemBrowse Memory Report

Top 10 targets by memory change (%) (out of 2404 targets) View Project Dashboard →

target .text .rodata .data .bss total % diff
metro_m4_express/net_lwip_webserver 41,504 → 41,520 (+16) 41,708 → 41,724 (+16) +0.0%
samg55_xplained/net_lwip_webserver 41,648 → 41,664 (+16) 41,848 → 41,864 (+16) +0.0%
ea4357/net_lwip_webserver 44,656 → 44,672 (+16) 45,350 → 45,366 (+16) +0.0%
stm32c542nucleo/net_lwip_webserver 41,712 → 41,720 (+8) 48,640 → 48,648 (+8) +0.0%
stm32l412nucleo/net_lwip_webserver 43,752 → 43,760 (+8) 49,088 → 49,096 (+8) +0.0%
ch582m_evt/net_lwip_webserver 45,912 → 45,920 (+8) 49,824 → 49,832 (+8) +0.0%
frdm_rw612/net_lwip_webserver 48,968 → 48,976 (+8) 51,144 → 51,152 (+8) +0.0%
stlinkv3mini/net_lwip_webserver 46,096 → 46,104 (+8) 53,436 → 53,444 (+8) +0.0%
metro_m7_1011/net_lwip_webserver 47,084 → 47,092 (+8) 54,968 → 54,976 (+8) +0.0%
ch32v307v_r1_1v0/net_lwip_webserver 56,332 → 56,340 (+8) 57,028 → 57,036 (+8) +0.0%

@github-actions

Copy link
Copy Markdown

Hardware-in-the-loop (HIL) Test Report

hfp.json

✅ 52 passed · ❌ 0 failed · ⚪ 0 skipped · blank not run

Board cdc_dual_ports cdc_msc dfu cdc_msc_throughput audio_test_freertos dfu_runtime cdc_msc_freertos hid_boot_interface msc_dual_lun hid_generic_inout printer_to_cdc midi_test mtp
stm32l412nucleo ✅ CDC 628k/404k MSC 822k/779k
stm32f746disco ✅ CDC 13.7M/10.1M MSC 15.4M/28.7M
stm32f746disco-DMA ✅ CDC 13.1M/9.5M MSC 13.7M/31.2M
lpcxpresso43s67 ✅ CDC 12.8M/11.7M MSC 29.7M/32.2M

tinyusb.json

✅ 200 passed · ❌ 138 failed · ⚪ 11 skipped · blank not run

Board cdc_dual_ports cdc_msc dfu cdc_msc_throughput audio_test_freertos dfu_runtime cdc_msc_freertos hid_boot_interface msc_dual_lun hid_generic_inout printer_to_cdc midi_test mtp host_info_to_device_cdc cdc_msc_hid msc_file_explorer msc_file_explorer_freertos device_info hid_composite_freertos
ek_tm4c123gxl ✅ CDC 745k/797k MSC 910k/1M
espressif_p4_function_ev rd 409KB/s
espressif_p4_function_ev-DMA rd 409KB/s
espressif_s3_devkitm rd 409KB/s
espressif_s3_devkitm-DMA
feather_nrf52840_express
max32666fthr
metro_m4_express
mimxrt1015_evk
mimxrt1064_evk
lpcxpresso11u37
ra4m1_ek
raspberry_pi_pico
raspberry_pi_pico_w
raspberry_pi_pico2
adafruit_fruit_jam
stm32f072disco ✅ CDC 519k/336k MSC 630k/471k
stm32f407disco ✅ CDC 846k/641k MSC 890k/958k
stm32f723disco ✅ CDC 402k/872k MSC 765k/945k rd 17476KB/s rd 4096KB/s
stm32f723disco-DMA ✅ CDC 977k/804k MSC 1M/1M rd 20164KB/s rd 4096KB/s
stm32h743nucleo ✅ CDC 842k/953k MSC 995k/1M
stm32h743nucleo-DMA ✅ CDC 783k/244k MSC 473k/966k
stm32g0b1nucleo ✅ CDC 514k/467k MSC 492k/539k
stm32l476disco ✅ CDC 423k/507k MSC 796k/948k
stm32u083nucleo ✅ CDC 508k/487k MSC 529k/559k
nanoch32v203-fsdev ✅ CDC 358k/634k MSC 732k/1M
nanoch32v203-usbfs ✅ CDC 520k/658k MSC 875k/976k
ch32v103r_r1_1v0
ch582m_evt ✅ CDC 221k/219k MSC 408k/482k

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant