fix: add input validation to prevent remote crash from empty/malforme…#912
fix: add input validation to prevent remote crash from empty/malforme…#912zxzxwu merged 2 commits intogoogle:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
…d PDUs Add length checks in from_bytes() for ATT and SMP protocol parsers to prevent IndexError crashes from empty PDUs sent by remote Bluetooth devices. Also add buffer size limit and UTF-8 error handling in HFP protocol to prevent memory exhaustion and decode crashes. - bumble/att.py: validate PDU is non-empty before accessing pdu[0] - bumble/smp.py: validate PDU is non-empty before accessing pdu[0] - bumble/hfp.py: limit buffer to 64KB, handle invalid UTF-8 gracefully These issues can be triggered by a remote Bluetooth device sending malformed packets, causing denial of service on the host.
ibondarenko1
force-pushed
the
fix/empty-pdu-crash
branch
from
9ca9d12 to
0a78e75
Compare
There was a problem hiding this comment.
Thanks for your PR, but I am wondering when do we need such handling?
For single packet parsing, we won't crash because there is a try-except umbrella at the packet sink root.
For HFP, despite the issue is possible, "normal" products qualified by SIG and authenticated by users should not send such invalid packets, and users can still define their own handling.
| @classmethod | ||
| def from_bytes(cls, pdu: bytes) -> ATT_PDU: | ||
| if not pdu: | ||
| raise ValueError("Empty ATT PDU") |
| @classmethod | ||
| def from_bytes(cls, pdu: bytes) -> SMP_Command: | ||
| if not pdu: | ||
| raise ValueError("Empty SMP PDU") |
| if len(self.buffer) > 65536: | ||
| logger.warning("HFP buffer overflow, truncating") | ||
| self.buffer = self.buffer[-65536:] |
There was a problem hiding this comment.
Maybe we can just abort new packets after overflow?
| if len(self.buffer) > 65536: | |
| logger.warning("HFP buffer overflow, truncating") | |
| self.buffer = self.buffer[-65536:] | |
| MAX_LINE_LENGTH = 65536 | |
| ... | |
| if len(self.buffer) > MAX_LINE_LENGTH: | |
| logger.error("HFP buffer overflow, abort") | |
| return |
…ffer overflow - att.py: raise core.InvalidPacketError instead of generic ValueError - smp.py: raise core.InvalidPacketError instead of generic ValueError - hfp.py: add MAX_BUFFER_SIZE class constant (64KB) - hfp.py: drop incoming data when it would overflow buffer instead of truncating, preserving existing partial-packet state Per review comments on PR google#912 by @zxzxwu.
|
Thanks for the review @zxzxwu — pushed follow-up commit addressing both points:
Happy to adjust naming or buffer size if preferred. |
|
@googlebot I signed it! |
3 participants
…d PDUs
Add length checks in from_bytes() for ATT and SMP protocol parsers to prevent IndexError crashes from empty PDUs sent by remote Bluetooth devices. Also add buffer size limit and UTF-8 error handling in HFP protocol to prevent memory exhaustion and decode crashes.
These issues can be triggered by a remote Bluetooth device sending malformed packets, causing denial of service on the host.