From 481d53c616d00511efc4ea28d869e0fccb1df0a7 Mon Sep 17 00:00:00 2001 From: Alan Carvalho de Assis Date: Thu, 17 Sep 2026 10:14:40 -0300 Subject: [PATCH] wireless/bluetooth: Validate lengths when parsing advertising reports. le_adv_report() took the report count and each report's data length from the event and used them without checking either against the data that was actually received: - the declared data length indexes the RSSI octet, so a length larger than the event reads past the end of the buffer; - the loop was bounded only by the report count, so a count larger than the payload walks off the end of it; - bt_buf_consume() only checks its bound with DEBUGASSERT(), so on a build without assertions the buffer length underflows rather than reporting the problem. Check that the event is long enough for the count, then check each report against the remaining length before reading its data or its RSSI, and stop parsing when a report does not fit. While here, include the RSSI octet when advancing to the next report. sizeof() of the report structure does not account for it, because the data member is a zero-length array, so every report after the first started one octet early. Ref: Core v6.0, Vol 4, Part E, 7.7.65.2 (LE Advertising Report event) Testing: builds for sim:bluetooth with Make; every commit in this series verified to build individually. Not yet exercised at runtime - the scriptable controller that can inject a malformed report is added separately. Signed-off-by: Alan C. Assis Assisted-by: Claude Code Opus 5 --- wireless/bluetooth/bt_hcicore.c | 56 ++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 12 deletions(-) diff --git a/wireless/bluetooth/bt_hcicore.c b/wireless/bluetooth/bt_hcicore.c index ce6c85d8d35eb..08089e587d2fc 100644 --- a/wireless/bluetooth/bt_hcicore.c +++ b/wireless/bluetooth/bt_hcicore.c @@ -792,18 +792,52 @@ static void check_pending_conn(FAR const bt_addr_le_t *addr, uint8_t evtype, static void le_adv_report(FAR struct bt_buf_s *buf) { - FAR struct bt_hci_ev_le_advertising_report_s *info; - uint8_t num_reports = buf->data[0]; + uint8_t num_reports; + + if (buf->len < sizeof(num_reports)) + { + wlerr("ERROR: Truncated advertising report event\n"); + return; + } + + num_reports = buf->data[0]; wlinfo("Adv number of reports %u\n", num_reports); - info = bt_buf_consume(buf, sizeof(num_reports)); + bt_buf_consume(buf, sizeof(num_reports)); while (num_reports--) { - int8_t rssi = info->data[info->length]; + FAR struct bt_hci_ev_le_advertising_report_s *info; FAR struct bt_keys_s *keys; bt_addr_le_t addr; + size_t reportlen; + int8_t rssi; + + /* A report is the fixed fields, the advertising data whose length + * they declare, and one octet of RSSI. Both lengths come from the + * controller, so check them against what was actually received + * before using them: the data length indexes the RSSI octet, and + * the total is what locates the next report. + */ + + if (buf->len < sizeof(*info)) + { + wlerr("ERROR: Truncated advertising report\n"); + return; + } + + info = (FAR void *)buf->data; + reportlen = sizeof(*info) + info->length + sizeof(rssi); + + if (buf->len < reportlen) + { + wlerr("ERROR: Advertising report data length %u exceeds event\n", + info->length); + return; + } + + rssi = info->data[info->length]; wlinfo("%s event %u, len %u, rssi %d dBm\n", bt_addr_le_str(&info->addr), info->evt_type, info->length, @@ -829,16 +863,13 @@ static void le_adv_report(FAR struct bt_buf_s *buf) check_pending_conn(&info->addr, info->evt_type, keys); - /* Get next report iteration by moving pointer to right offset in buf - * according to spec 4.2, Vol 2, Part E, 7.7.65.2. - * - * TODO: multiple reports are stored as multiple arrays not one array - * of structs. If num_reports > 0 this will not WORK! + /* Advance to the next report. The RSSI octet sits after the + * advertising data and belongs to the report, which the previous + * advance left out, so every report after the first started one + * octet early. */ - /* Note that info already contains one byte which accounts for RSSI */ - - info = bt_buf_consume(buf, sizeof(*info) + info->length); + bt_buf_consume(buf, reportlen); } } @@ -2191,6 +2222,7 @@ int bt_start_advertising(uint8_t type, FAR const struct bt_eir_s *ad, int bt_stop_advertising(void) { FAR struct bt_buf_s *buf; + if (!g_btdev.adv_enable) { wlwarn("WARNING: Already advertising\n");