fix(nordic): bound the GATT write length in bt_receive_cb()
What the developer wrote
Authored by tychovrahe
fix(nordic): bound the GATT write length in bt_receive_cb()
The receive callback copied the GATT write payload into a fixed 251-byte
stack buffer without checking its length. It does not overflow today, but
only because the numbers happen to line up exactly: one L2CAP RX buffer
holds a 247-byte ATT PDU, an ATT write spends 3 bytes on the opcode and the
handle, and 244 plus the 7-byte address prefix is exactly the buffer size.
Nothing tied those numbers together. Raising CONFIG_BT_BUF_ACL_RX_SIZE on
its own is enough to let a peer deliver more, because the delivered length
is bounded by the L2CAP RX MTU rather than by the negotiated ATT MTU -
att_write_req() and att_write_cmd() pass the received length straight to the
callback.
Reject anything longer than BLE_RX_PACKET_SIZE, and add a BUILD_ASSERT so
that the Bluetooth buffer configuration and BLE_RX_PACKET_SIZE can no longer
drift apart silently.
[no changelog]
What changed, and why it matters
This commit fixes a Bluetooth code flaw in Trezor's Nordic chip firmware. A function that receives data from another device over Bluetooth was copying that data into a fixed-size on-chip memory buffer without first checking how long the incoming data was. Right now the sizes happen to match, so no overflow occurs, but a configuration change or a connected peer sending a specially large write could have overflowed the buffer. The patch now rejects oversized incoming packets and adds a compile-time check so the buffer sizes cannot silently drift apart in future builds.
Treat this as a security fix and include it in the next firmware release. Review whether other Bluetooth callbacks in the same file or related Zephyr-based firmware use similar unchecked copies. Verify that BLE_RX_PACKET_SIZE and CONFIG_BT_BUF_ACL_RX_SIZE are kept in sync in all build configurations, including debug/internal builds where ACL RX size may be increased.
Security signals we found
Stack buffer overflow risk in Bluetooth GATT receive callback
Missing length validation before memcpy into fixed-size buffer
Reachable from a connected Bluetooth peer (remote attack surface)
Build-time assertion added to tie buffer sizes together
Patch rejects oversized packets with a warning log
Evidence from the diff
In nordic/trezor/trezor-ble/src/ble/ble.c, bt_receive_cb() previously declared uint8_t data_copy[BLE_RX_PACKET_SIZE + 7] and copied len bytes from the GATT write payload without validation. The commit adds a length check (if len > BLE_RX_PACKET_SIZE, drop), replaces the magic 7 with BLE_RX_ADDR_PREFIX_SIZE (1 + BT_ADDR_SIZE), and adds a BUILD_ASSERT that BLE_MAX_GATT_WRITE_SIZE (BT_L2CAP_RX_MTU - 3) <= BLE_RX_PACKET_SIZE. The vulnerability is a stack-based buffer overflow reachable from a paired Bluetooth peer; exploitation is currently blocked only by the exact alignment of L2CAP/ATT/buffer sizes, and raising CONFIG_BT_BUF_ACL_RX_SIZE alone would remove that protection.
Changed components
nordic/trezor/trezor-ble/src/ble/ble.cbt_receive_cb()Trezor Safe hardware wallets using Nordic nRF Bluetooth stackInspect captured patch +24 / −3
### nordic/trezor/trezor-ble/src/ble/ble.c
@@ -27,6 +27,7 @@
#include <zephyr/bluetooth/gatt.h>
#include <zephyr/bluetooth/hci.h>
#include <zephyr/bluetooth/hci_vs.h>
+#include <zephyr/bluetooth/l2cap.h>
#include <zephyr/bluetooth/uuid.h>
#include <zephyr/settings/settings.h>
@@ -57,6 +58,21 @@ static int8_t g_act_tx_power_level = 0;
static int8_t g_set_tx_power_level = 0;
#endif
+// Address type and address prepended to every received packet
+#define BLE_RX_ADDR_PREFIX_SIZE (1 + BT_ADDR_SIZE)
+
+// Largest GATT write payload a peer can deliver: the ATT PDU that fits into a
+// single L2CAP RX buffer, less the ATT write opcode and the attribute handle.
+#define BLE_MAX_GATT_WRITE_SIZE (BT_L2CAP_RX_MTU - 3)
+
+// `BLE_RX_PACKET_SIZE` has to keep up with the Bluetooth buffer configuration,
+// which is what actually bounds how much a peer can write. If a peer could
+// deliver more, `bt_receive_cb()` would have to drop valid packets to keep
+// `data_copy` from overflowing.
+BUILD_ASSERT(BLE_MAX_GATT_WRITE_SIZE <= BLE_RX_PACKET_SIZE,
+ "CONFIG_BT_BUF_ACL_RX_SIZE allows a GATT write larger than "
+ "BLE_RX_PACKET_SIZE");
+
static void bt_receive_cb(struct bt_conn *conn, const uint8_t *const data,
uint16_t len) {
if (atomic_get(&g_busy_flag) != 0) {
@@ -71,13 +87,18 @@ static void bt_receive_cb(struct bt_conn *conn, const uint8_t *const data,
LOG_DBG("Received data from: %s, %d", addr, len);
- uint8_t data_copy[BLE_RX_PACKET_SIZE + 7] = {0};
+ if (len > BLE_RX_PACKET_SIZE) {
+ LOG_WRN("Received data too long (%u bytes), dropping", len);
+ return;
+ }
+
+ uint8_t data_copy[BLE_RX_PACKET_SIZE + BLE_RX_ADDR_PREFIX_SIZE] = {0};
data_copy[0] = bt_conn_get_dst(conn)->type;
memcpy(data_copy + 1, bt_conn_get_dst(conn)->a.val, BT_ADDR_SIZE);
- memcpy(data_copy + 1 + BT_ADDR_SIZE, data, len);
+ memcpy(data_copy + BLE_RX_ADDR_PREFIX_SIZE, data, len);
- trz_comm_send_msg(NRF_SERVICE_BLE, data_copy, len + 1 + BT_ADDR_SIZE);
+ trz_comm_send_msg(NRF_SERVICE_BLE, data_copy, len + BLE_RX_ADDR_PREFIX_SIZE);
}
bool ble_init(void) {Why this scored 64/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.