Skip to content

Commit 32ac770

Browse files
committed
Bluetooth: GATT: Check len of response before parsing response PDU
In function `parse_read_by_uuid()`, the response length is not checked before parsing the response PDU. There is a potential issue that the `len` will be underflowed if the `len` is less than the size of `struct bt_att_data`. Check the length before parsing the response PDU. If the length is less then the size of `struct bt_att_data`, notify the upper layer with the error `BT_ATT_ERR_INVALID_ATTRIBUTE_LEN` and stop the parsing. Signed-off-by: Lyle Zhu <[email protected]>
1 parent f4a85bb commit 32ac770

File tree

2 files changed

+15
-6
lines changed

2 files changed

+15
-6
lines changed

include/zephyr/bluetooth/gatt.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1994,10 +1994,12 @@ struct bt_gatt_read_params {
19941994
* the callback being called with @p data set to @c NULL.
19951995
*
19961996
* Note that returning @ref BT_GATT_ITER_CONTINUE is really starting a new ATT
1997-
* operation, so this can fail to allocate resources. However, all API errors
1998-
* are reported as if the server returned @ref BT_ATT_ERR_UNLIKELY. There is no
1999-
* way to distinguish between this condition and a @ref BT_ATT_ERR_UNLIKELY
2000-
* response from the server itself.
1997+
* operation, so this can fail to allocate resources. However, if the received
1998+
* data length is invalid, the error @ref BT_ATT_ERR_INVALID_ATTRIBUTE_LEN
1999+
* will be returned. For all other API errors are reported as if the server
2000+
* returned @ref BT_ATT_ERR_UNLIKELY. There is no way to distinguish between
2001+
* this condition and a @ref BT_ATT_ERR_UNLIKELY response from the server
2002+
* itself.
20012003
*
20022004
* Note that the effect of returning @ref BT_GATT_ITER_CONTINUE from the
20032005
* callback varies depending on the type of read operation.

subsys/bluetooth/host/gatt.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4761,6 +4761,15 @@ static void parse_read_by_uuid(struct bt_conn *conn,
47614761
uint16_t handle;
47624762
uint16_t len;
47634763

4764+
len = MIN(rsp->len, length);
4765+
if (len < sizeof(struct bt_att_data)) {
4766+
LOG_WRN("Bad peer: ATT read-by-uuid rsp: invalid ATTR data len %u", len);
4767+
params->func(conn, BT_ATT_ERR_INVALID_ATTRIBUTE_LEN, params, NULL, 0);
4768+
return;
4769+
}
4770+
4771+
len -= sizeof(struct bt_att_data);
4772+
47644773
handle = sys_le16_to_cpu(data->handle);
47654774

47664775
/* Handle 0 is invalid */
@@ -4769,8 +4778,6 @@ static void parse_read_by_uuid(struct bt_conn *conn,
47694778
return;
47704779
}
47714780

4772-
len = rsp->len > length ? length - 2 : rsp->len - 2;
4773-
47744781
LOG_DBG("handle 0x%04x len %u value %u", handle, rsp->len, len);
47754782

47764783
if (!IN_RANGE(handle, req_start_handle, req_end_handle)) {

0 commit comments

Comments
 (0)