CordioGattServer is using wrong connection handles resulting in faults / memory corruption #8880
Description
Description
I've came into this issue while working on BLE examples for FUTURE_SEQUANA target.
GattServr::write method contains the following code:
uint16_t conn_id = 0;
uint16_t conn_found = 0;
size_t updates_sent = 0;
while((conn_found < DM_CONN_MAX) && (conn_id < CONNECTION_ID_LIMIT)) {
if (DmConnInUse(conn_id) == true) {
++conn_found;
if (is_update_authorized(conn_id, att_handle)) {
uint16_t cccd_config = AttsCccEnabled(conn_id, cccd_index);
if (cccd_config & ATT_CLIENT_CFG_NOTIFY) {
AttsHandleValueNtf(conn_id, att_handle, len, (uint8_t*)buffer);
updates_sent++;
}
if (cccd_config & ATT_CLIENT_CFG_INDICATE) {
AttsHandleValueInd(conn_id, att_handle, len, (uint8_t*)buffer);
updates_sent++;
}
}
}
++conn_id;
}
The issue is that conn_id usage is inconsistent with DM API. conn_id runs here from 0 to CONNECTION_ID_LIMIT-1 (i.e. 255) while DM accepts conn_id in the range 1 .. DM_CONN_MAX (.e. 3) and uses it to index it's internal arrays (which are of size DM_CONN_MAX).
As a result random data is checked for connection attributes in DmConnInUse():
bool_t DmConnInUse(dmConnId_t connId)
{
WSF_ASSERT((connId > 0) && (connId <= DM_CONN_MAX));
return dmConnCb.ccb[connId-1].inUse;
}
If this random data happens to indicate a valid connection, subsequent functions attempt to write memory beyond DM allocated data arrays.
This could be easily caught by turning on WSF asserts, but these are off by default.
Should be easily reproducible on any target that is using CORDIO BLE stack; Sequana board is a rare beast right now.
I've found it in Mbed OS version 5.10.3/5.10.4 but it seems to be present on master branch for a few months (since commit 0f64b1c).
Issue request type
[ ] Question
[ ] Enhancement
[X] Bug