Skip to contentSkip to conversation

shared/gatt-server: Avoid memory corruption

PATCH Applied Romain Izard 2 participants 3 messages Jan 23, 2015 – Jan 26, 2015 Bluetooth drivers

lore
Share & tools
lore

Romain Izard

author
#0 lore raw

PATCH 1/2 shared/gatt-server: Avoid memory corruption

When sending notification and indication data, the size of the allocated packet is the smallest of the MTU and the payload size.

The copy procedure uses the payload size in all cases, which can lead to memory corruption. Use the packet size instead.

---
 src/shared/gatt-server.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
src/shared/gatt-server.c +2 -2
index b406ed6..dd9c88f 100644
@@ -1506,7 +1506,7 @@ bool bt_gatt_server_send_notification(struct bt_gatt_server *server,
 		return false;
 
 	put_le16(handle, pdu);
-	memcpy(pdu + 2, value, length);
+	memcpy(pdu + 2, value, pdu_len - 2);
 
 	result = !!bt_att_send(server->att, BT_ATT_OP_HANDLE_VAL_NOT, pdu,
 						pdu_len, NULL, NULL, NULL);

@@ -1571,7 +1571,7 @@ bool bt_gatt_server_send_indication(struct bt_gatt_server *server,
 	data->user_data = user_data;
 
 	put_le16(handle, pdu);
-	memcpy(pdu + 2, value, length);
+	memcpy(pdu + 2, value, pdu_len - 2);
 
 	result = !!bt_att_send(server->att, BT_ATT_OP_HANDLE_VAL_IND, pdu,
 							pdu_len, conf_cb,
Signature · 1 line
2.1.0
author
#1 lore raw

PATCH 2/2 shared/gatt-server: Fix notification & indication packet size

For both Handle Value Notification & Handle Value Indication, the size of the payload is limited to (ATT_MTU - 3) bytes, as indicated in Vol 3, part F, section 3.4.7.1 & 3.4.7.2 of the Bluetooth 4.2 specification.

In the function, the first reserved byte is implicit and corresponds to the opcode, while the 2 next bytes are explicit and contain the attribute handle.

Ensure that the truncation of large payloads is done with the correct value.

---
 src/shared/gatt-server.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
src/shared/gatt-server.c +2 -2
index dd9c88f..c7974fb 100644
@@ -1500,7 +1500,7 @@ bool bt_gatt_server_send_notification(struct bt_gatt_server *server,
 	if (!server || (length && !value))
 		return false;
 
-	pdu_len = MIN(bt_att_get_mtu(server->att), length + 2);
+	pdu_len = MIN(bt_att_get_mtu(server->att) - 1, length + 2);
 	pdu = malloc(pdu_len);
 	if (!pdu)
 		return false;

@@ -1555,7 +1555,7 @@ bool bt_gatt_server_send_indication(struct bt_gatt_server *server,
 	if (!server || (length && !value))
 		return false;
 
-	pdu_len = MIN(bt_att_get_mtu(server->att), length + 2);
+	pdu_len = MIN(bt_att_get_mtu(server->att) - 1, length + 2);
 	pdu = malloc(pdu_len);
 	if (!pdu)
 		return false;
Signature · 1 line
2.1.0
maintainer Applied
#2 lore raw
In reply to Romain Izard

Hi Romain,

Quoted 32 lines · Romain Izard

When sending notification and indication data, the size of the allocated packet is the smallest of the MTU and the payload size.

The copy procedure uses the payload size in all cases, which can lead to memory corruption. Use the packet size instead.

---
 src/shared/gatt-server.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
src/shared/gatt-server.c +2 -3
index b406ed6..dd9c88f 100644
@@ -1506,7 +1506,7 @@ bool bt_gatt_server_send_notification(struct bt_gatt_server *server,
                return false;
 
        put_le16(handle, pdu);
-       memcpy(pdu + 2, value, length);
+       memcpy(pdu + 2, value, pdu_len - 2);
 
        result = !!bt_att_send(server->att, BT_ATT_OP_HANDLE_VAL_NOT, pdu,
                                                pdu_len, NULL, NULL, NULL);

@@ -1571,7 +1571,7 @@ bool bt_gatt_server_send_indication(struct bt_gatt_server *server,
        data->user_data = user_data;
 
        put_le16(handle, pdu);
-       memcpy(pdu + 2, value, length);
+       memcpy(pdu + 2, value, pdu_len - 2);
 
        result = !!bt_att_send(server->att, BT_ATT_OP_HANDLE_VAL_IND, pdu,
                                                        pdu_len, conf_cb,
--
Quoted 1 line

2.1.0

Applied, thanks.

Signature · 1 line
Luiz Augusto von Dentz