What the developer wrote
Authored by tobtoht
Merge pull request #11348
f462611 device: protect against buffer overflow (jpk68)
ACKs: selsta, PyXMR2025
What changed, and why it matters
This commit adds a safety check in Monero's hardware wallet USB communication code. Before reading another chunk of data from a Ledger/Trezor-like device, it now verifies that the next chunk will fit inside the fixed-size memory buffer. Without this check, a malicious or malfunctioning USB device could trick the software into writing past the end of the buffer, potentially crashing the wallet or corrupting memory. The fix is a straightforward bounds check, but it is only one of several reads in the same loop, so it may be a partial hardening rather than a complete fix for every overflow path.
Review the full receive loop and any other hid_read_timeout/hid_write calls in device_io_hid.cpp to ensure all buffer accesses are bounded. Consider replacing the assertion with an explicit error return path so a malicious device cannot trigger an abort-of-service. Users who pair Monero with hardware wallets should update to a release containing this commit once available.
Security signals we found
Buffer overflow / out-of-bounds write prevention
Untrusted peripheral input size validation
Assertion-based defensive check added
HID hardware wallet communication path
Evidence from the diff
In src/device/device_io_hid.cpp, inside the HID receive loop, the patch inserts ASSERT_X(offset + MAX_BLOCK <= sizeof(buffer), …) before a hid_read_timeout() call that reads up to MAX_BLOCK bytes into buffer + offset. This prevents an out-of-bounds write when offset is already near or past the end of buffer. The assertion aborts the process rather than allowing a buffer overflow. The commit message frames it as ‘protect against buffer overflow’. Because the loop can increment offset by hid_ret bytes and there are other reads in the same function, this single check may not cover all possible overflow scenarios, so the patch should be treated as conservative hardening.
Changed components
src/device/device_io_hid.cppMonero hardware wallet HID I/O layerInspect captured patch +1 / −0
### src/device/device_io_hid.cpp
@@ -219,6 +219,7 @@ namespace hw {
if (result != 0) {
break;
}
+ ASSERT_X(offset + MAX_BLOCK <= sizeof(buffer), "HID response too large for buffer");
hid_ret = hid_read_timeout(this->usb_device, buffer + offset, MAX_BLOCK, this->timeout);
ASSERT_X(hid_ret>=0, "Unable to receive hidapi response. Error "+std::to_string(result)+": "+ safe_hid_error(this->usb_device));
result = (unsigned int)hid_ret;Why this scored 65/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.