Fix out of bounds write when a HID descriptor has a large report count (#332) - #361
Draft
mglushko wants to merge 1 commit into
Draft
Fix out of bounds write when a HID descriptor has a large report count (#332)#361mglushko wants to merge 1 commit into
mglushko wants to merge 1 commit into
Conversation
handle_main_input() loops report_count times per main item and uses the loop index to index parser->p_usage[i]. The only guard, i < HID_MAX_USAGES in update_usage(), bounds that index relative to p_usage rather than against the end of usages[], and p_usage has usually moved on by then. store_element() had no guard at all. The Gameball trackball from issue hrvach#332 carries a vendor "gesture" collection on its second interface that declares 8-bit fields with report counts of 64, 256, 1024 and 2048. Parsing it walks to usages[2051] in a 128 entry array: reads run 3848 bytes past the end, and writes reach usages[131]. On the RP2040 usages[] ends at byte 271 of parser_state and p_usage sits at byte 272, so the first out-of-bounds write lands on the parser's own cursor and fills it with usage data. At -Ofast that pointer is reloaded from memory on every iteration, so the next element dereferences an unaligned address in unmapped space - a hard fault, and a watchdog reset for as long as the device stays plugged in. The trackball interface itself parses correctly (buttons, X, Y, wheel and AC pan all come out right, so both scroll axes are covered); the board just never survives long enough to use it. Bound every access against the end of the array. Past the last stored usage the same usage keeps applying, which is what the spec says happens anyway, so reuse the last slot instead of running off the end. Verified by parsing both Gameball descriptors plus eight ordinary devices (boot mouse, hi-res mouse with report ID, boot keyboard, NKRO keyboard, consumer, system, composite dongle, and a descriptor declaring more usages than the array holds): all previously working descriptors produce byte-identical results. A fuzz run over 750k generated descriptors records zero accesses outside usages[], against roughly 125M per 40k descriptors before the change.
This was referenced Aug 13, 2026
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
1 participant
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.
Plugging the Gameball trackball from #332 into a board stops that board working. The reporter has it on board B and the cursor never moves, while board A carries on normally.
The parser stores the usages it reads in a fixed array of 128 slots. When a descriptor says a field repeats N times, it writes one entry per repeat without ever checking N against the size of that array. The Gameball has a second interface for gestures with repeat counts of 64, 256, 1024 and 2048, so the parser runs well past the end.
In memory the array is followed immediately by the pointer the parser uses to walk it, so the first write past the end lands on that pointer. Everything written after that goes wherever the corrupted pointer happens to point, which is why the board the trackball is plugged into stops working.
The descriptor itself is valid. One usage with a large repeat count is a usage value array, and the HID spec (1.11, section 6.2.2.8) says the last usage applies to any remaining controls, so the device is fine and the parser was reading it wrong. It is not specific to this device either. Any descriptor with a repeat count over 128 does it, and there is no way around it in the config because the descriptor is parsed before the code looks at the interface type.
The fix checks every access against the end of the array and reuses the last usage slot instead of walking past it, which is the behaviour the spec describes anyway.
Two things this does not address. The usage pointer still never resets, so a device with more than about 126 usages ahead of its pointer collection would enumerate without the cursor moving. Report Count is a 32 bit field, so a large enough count still outruns the 500 ms watchdog even though memory is no longer corrupted. Neither affects this device, whose largest count is 2048.
Marking this a draft until someone with a Gameball confirms it on hardware.