bound ndp16 wLength against received ntb in recv_validate_datagram#3741
Open
dxbjavid wants to merge 1 commit into
Open
bound ndp16 wLength against received ntb in recv_validate_datagram#3741dxbjavid wants to merge 1 commit into
dxbjavid wants to merge 1 commit into
Conversation
|
| 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% |
Hardware-in-the-loop (HIL) Test Reporthfp.json✅ 52 passed · ❌ 0 failed · ⚪ 0 skipped · blank not run
tinyusb.json✅ 200 passed · ❌ 138 failed · ⚪ 11 skipped · blank not run
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.