Skip to content

Commit

Permalink
[Tizen] Fix memory out-of-bound reads when logging (#19157)
Browse files Browse the repository at this point in the history 
* Add more feature check to Tizen app manifest

* Use C++ casting instead of strongest C casting

* Do not log hex data byte by byte - use ChipLogByteSpan

* Use dedicated type for GATT type instead of int

* Do not return NULL in "to-string" functions

* Use "%.*s" specifier when logging non-string data

Fixes #15777

* Small cleanups in Tizen DNSsd logging

* Use sizeof(dest) when using strlcpy function
  • Loading branch information
@arkq @pull
arkq authored and pull[bot] committed Aug 2, 2023
1 parent 358a9b7 commit 2232064
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 148 deletions.
3 changes: 3 additions & 0 deletions examples/lighting-app/tizen/tizen-manifest.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
<privilege>http://tizen.org/privilege/internet</privilege>
<privilege>http://tizen.org/privilege/network.get</privilege>
</privileges>
<feature name="http://tizen.org/feature/network.bluetooth">true</feature>
<feature name="http://tizen.org/feature/network.bluetooth.le">true</feature>
<feature name="http://tizen.org/feature/network.bluetooth.le.gatt.server">true</feature>
<feature name="http://tizen.org/feature/network.service_discovery.dnssd">true</feature>
<feature name="http://tizen.org/feature/network.wifi">true</feature>
</manifest>
2 changes: 1 addition & 1 deletion src/platform/EFR32/NetworkCommissioningWiFiDriver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ CHIP_ERROR SlWiFiDriver::ConnectWiFiNetwork(const char * ssid, uint8_t ssidLen,
memcpy(wifiConfig.passkey, key, keyLen);
wifiConfig.security = WFX_SEC_WPA2;

ChipLogProgress(NetworkProvisioning, "Setting up connetion for WiFi SSID: %s", wifiConfig.ssid);
ChipLogProgress(NetworkProvisioning, "Setting up connection for WiFi SSID: %.*s", static_cast<int>(ssidLen), ssid);
// Configure the WFX WiFi interface.
wfx_set_wifi_provision(&wifiConfig);
ReturnErrorOnFailure(ConnectivityMgr().SetWiFiStationMode(ConnectivityManager::kWiFiStationMode_Disabled));
Expand Down
99 changes: 52 additions & 47 deletions src/platform/Tizen/BLEManagerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,14 @@ gboolean BLEManagerImpl::_BleInitialize(void * userData)
return false;
}

static int __GetAttInfo(bt_gatt_h gattHandle, char ** uuid, int * type)
static int __GetAttInfo(bt_gatt_h gattHandle, char ** uuid, bt_gatt_type_e * type)
{
int ret;

ret = bt_gatt_get_type(gattHandle, (bt_gatt_type_e *) type);
int ret = bt_gatt_get_type(gattHandle, type);
VerifyOrReturnError(ret == BT_ERROR_NONE, ret);

return bt_gatt_get_uuid(gattHandle, uuid);
}

static const char * __ConvertAttTypeToStr(int type)
static constexpr const char * __ConvertAttTypeToStr(bt_gatt_type_e type)
{
switch (type)
{
Expand All @@ -143,24 +140,27 @@ static const char * __ConvertAttTypeToStr(int type)
case BT_GATT_TYPE_DESCRIPTOR:
return "Descriptor";
default:
return nullptr;
return "(unknown)";
}
}

static void __ReadValueRequestedCb(const char * remoteAddress, int requestId, bt_gatt_server_h server, bt_gatt_h gattHandle,
int offset, void * userData)
{
int ret, len = 0, type = 0;
char *value = nullptr, *uuid = nullptr;
int ret, len = 0;
bt_gatt_type_e type;
char * uuid = nullptr;
char * value = nullptr;

__GetAttInfo(gattHandle, &uuid, &type);
ChipLogProgress(DeviceLayer, "Read Requested on %s(%s)", __ConvertAttTypeToStr(type), uuid);
VerifyOrReturn(__GetAttInfo(gattHandle, &uuid, &type) == BT_ERROR_NONE,
ChipLogError(DeviceLayer, "Failed to fetch GATT Attribute from GATT handle"));
ChipLogProgress(DeviceLayer, "Read Requested on %s: %s", __ConvertAttTypeToStr(type), uuid);
g_free(uuid);

ret = bt_gatt_get_value(gattHandle, &value, &len);
VerifyOrReturn(ret == BT_ERROR_NONE, ChipLogError(DeviceLayer, "bt_gatt_get_value() failed. ret: %d", ret));

ChipLogProgress(DeviceLayer, "Read Value: %s(len: %d)", value, len);
ChipLogProgress(DeviceLayer, "Read Value (len: %d): %.*s", len, len, value);

ret = bt_gatt_server_send_response(requestId, BT_GATT_REQUEST_TYPE_READ, offset, 0x00, value, len);
g_free(value);
Expand All @@ -170,18 +170,19 @@ static void __ReadValueRequestedCb(const char * remoteAddress, int requestId, bt
void BLEManagerImpl::WriteValueRequestedCb(const char * remoteAddress, int requestId, bt_gatt_server_h server, bt_gatt_h gattHandle,
bool responseNeeded, int offset, const char * value, int len, void * userData)
{
int ret, type = 0;
int ret;
char * uuid = nullptr;
BLEConnection * conn = nullptr;
bt_gatt_type_e type;

conn = static_cast<BLEConnection *>(g_hash_table_lookup(sInstance.mConnectionMap, remoteAddress));
VerifyOrReturn(conn != nullptr, ChipLogError(DeviceLayer, "Failed to find connection info"));

VerifyOrReturn(__GetAttInfo(gattHandle, &uuid, &type) == BT_ERROR_NONE,
ChipLogError(DeviceLayer, "Failed to fetch GATT Attribute from GATT handle"));

ChipLogProgress(DeviceLayer, "Write Requested on %s(%s)", __ConvertAttTypeToStr(type), uuid);
ChipLogProgress(DeviceLayer, "Write Value: %s(len: %d)", value, len);
ChipLogProgress(DeviceLayer, "Write Requested on %s: %s", __ConvertAttTypeToStr(type), uuid);
ChipLogProgress(DeviceLayer, "Write Value (len: %d): %.*s ", len, len, value);
g_free(uuid);

ret = bt_gatt_set_value(gattHandle, value, len);
Expand All @@ -190,14 +191,14 @@ void BLEManagerImpl::WriteValueRequestedCb(const char * remoteAddress, int reque
ret = bt_gatt_server_send_response(requestId, BT_GATT_REQUEST_TYPE_WRITE, offset, 0x00, nullptr, 0);
VerifyOrReturn(ret == BT_ERROR_NONE, ChipLogError(DeviceLayer, "bt_gatt_server_send_response() failed. ret: %d", ret));

sInstance.HandleC1CharWriteEvent(conn, (const uint8_t *) value, len);
sInstance.HandleC1CharWriteEvent(conn, reinterpret_cast<const uint8_t *>(value), len);
}

void BLEManagerImpl::NotificationStateChangedCb(bool notify, bt_gatt_server_h server, bt_gatt_h gattHandle, void * userData)
{
int type = 0;
char * uuid = nullptr;
BLEConnection * conn = nullptr;
bt_gatt_type_e type;
GHashTableIter iter;
gpointer key, value;

Expand All @@ -214,7 +215,7 @@ void BLEManagerImpl::NotificationStateChangedCb(bool notify, bt_gatt_server_h se
VerifyOrReturn(__GetAttInfo(gattHandle, &uuid, &type) == BT_ERROR_NONE,
ChipLogError(DeviceLayer, "Failed to fetch GATT Attribute from GATT handle"));

ChipLogProgress(DeviceLayer, "Notification State Changed %d on %s(%s)", notify, __ConvertAttTypeToStr(type), uuid);
ChipLogProgress(DeviceLayer, "Notification State Changed %d on %s: %s", notify, __ConvertAttTypeToStr(type), uuid);
g_free(uuid);
sInstance.NotifyBLESubscribed(notify ? true : false, conn);
}
Expand All @@ -240,7 +241,7 @@ void BLEManagerImpl::CharacteristicNotificationCb(bt_gatt_h characteristic, char
VerifyOrReturn(conn->gattCharC2Handle == characteristic, ChipLogError(DeviceLayer, "Gatt characteristic handle did not match"));

ChipLogProgress(DeviceLayer, "Notification Received from CHIP peripheral [%s]", conn->peerAddr);
sInstance.HandleRXCharChanged(conn, (const uint8_t *) value, len);
sInstance.HandleRXCharChanged(conn, reinterpret_cast<const uint8_t *>(value), len);
}

void BLEManagerImpl::IndicationConfirmationCb(int result, const char * remoteAddress, bt_gatt_server_h server,
Expand Down Expand Up @@ -419,7 +420,7 @@ gboolean BLEManagerImpl::ConnectChipThing(gpointer userData)
{
int ret = BT_ERROR_NONE;

char * address = (char *) userData;
char * address = reinterpret_cast<char *>(userData);
ChipLogProgress(DeviceLayer, "ConnectRequest: Addr [%s]", address);

ret = bt_gatt_client_create(address, &sInstance.mGattClient);
Expand Down Expand Up @@ -450,7 +451,7 @@ void BLEManagerImpl::ConnectHandler(const char * address)

void BLEManagerImpl::OnChipDeviceScanned(void * device, const chip::Ble::ChipBLEDeviceIdentificationInfo & info)
{
bt_adapter_le_device_scan_result_info_s * deviceInfo = (bt_adapter_le_device_scan_result_info_s *) device;
auto deviceInfo = reinterpret_cast<bt_adapter_le_device_scan_result_info_s *>(device);
VerifyOrReturn(deviceInfo != nullptr, ChipLogError(DeviceLayer, "Invalid Device Info"));

ChipLogProgress(DeviceLayer, "New device scanned: %s", deviceInfo->remote_address);
Expand Down Expand Up @@ -678,50 +679,54 @@ void BLEManagerImpl::InitConnectionData(void)

static bool __GattClientForeachCharCb(int total, int index, bt_gatt_h charHandle, void * data)
{
int type;
bt_gatt_type_e type;
char * uuid = nullptr;
auto conn = static_cast<BLEConnection *>(data);

if (__GetAttInfo(charHandle, &uuid, &type) == BT_ERROR_NONE)
VerifyOrExit(__GetAttInfo(charHandle, &uuid, &type) == BT_ERROR_NONE,
ChipLogError(DeviceLayer, "Failed to fetch GATT Attribute from CHAR handle"));

if (strcasecmp(uuid, chip_ble_char_c1_tx_uuid) == 0)
{
if (strcasecmp(uuid, chip_ble_char_c1_tx_uuid) == 0)
{
ChipLogProgress(DeviceLayer, "CHIP Char C1 TX Found [%s]", uuid);
conn->gattCharC1Handle = charHandle;
}
else if (strcasecmp(uuid, chip_ble_char_c2_rx_uuid) == 0)
{
ChipLogProgress(DeviceLayer, "CHIP Char C2 RX Found [%s]", uuid);
conn->gattCharC2Handle = charHandle;
}
g_free(uuid);
ChipLogProgress(DeviceLayer, "CHIP Char C1 TX Found [%s]", uuid);
conn->gattCharC1Handle = charHandle;
}
else if (strcasecmp(uuid, chip_ble_char_c2_rx_uuid) == 0)
{
ChipLogProgress(DeviceLayer, "CHIP Char C2 RX Found [%s]", uuid);
conn->gattCharC2Handle = charHandle;
}
g_free(uuid);

exit:
/* Try next Char UUID */
return true;
}

static bool __GattClientForeachServiceCb(int total, int index, bt_gatt_h svcHandle, void * data)
{
int type;
bt_gatt_type_e type;
char * uuid = nullptr;
auto conn = static_cast<BLEConnection *>(data);
ChipLogProgress(DeviceLayer, "__GattClientForeachServiceCb");

if (__GetAttInfo(svcHandle, &uuid, &type) == BT_ERROR_NONE)
VerifyOrExit(__GetAttInfo(svcHandle, &uuid, &type) == BT_ERROR_NONE,
ChipLogError(DeviceLayer, "Failed to fetch GATT Attribute from SVC handle"));

if (strcasecmp(uuid, chip_ble_service_uuid) == 0)
{
if (strcasecmp(uuid, chip_ble_service_uuid) == 0)
{
ChipLogProgress(DeviceLayer, "CHIP Service UUID Found [%s]", uuid);
ChipLogProgress(DeviceLayer, "CHIP Service UUID Found [%s]", uuid);

if (bt_gatt_service_foreach_characteristics(svcHandle, __GattClientForeachCharCb, conn) == BT_ERROR_NONE)
conn->isChipDevice = true;
if (bt_gatt_service_foreach_characteristics(svcHandle, __GattClientForeachCharCb, conn) == BT_ERROR_NONE)
conn->isChipDevice = true;

/* Got CHIP Device, no need to process further service */
g_free(uuid);
return false;
}
/* Got CHIP Device, no need to process further service */
g_free(uuid);
return false;
}
g_free(uuid);

exit:
/* Try next Service UUID */
return true;
}
Expand Down Expand Up @@ -1242,7 +1247,7 @@ bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUU
conn = static_cast<BLEConnection *>(g_hash_table_lookup(sInstance.mConnectionMap, conn->peerAddr));
VerifyOrExit(conn != nullptr, ChipLogError(DeviceLayer, "Failed to find connection info"));

ret = bt_gatt_set_value(mGattCharC2Handle, (const char *) pBuf->Start(), pBuf->DataLength());
ret = bt_gatt_set_value(mGattCharC2Handle, reinterpret_cast<const char *>(pBuf->Start()), pBuf->DataLength());
VerifyOrExit(ret == BT_ERROR_NONE, ChipLogError(DeviceLayer, "bt_gatt_set_value() failed. ret: %d", ret));

ChipLogProgress(DeviceLayer, "Sending indication for CHIPoBLE RX characteristic (con %s, len %u)", conn->peerAddr,
Expand Down Expand Up @@ -1277,7 +1282,7 @@ bool BLEManagerImpl::SendWriteRequest(BLE_CONNECTION_OBJECT conId, const Ble::Ch
ChipLogError(DeviceLayer, "SendWriteRequest() called with invalid characteristic ID"));
VerifyOrExit(conn->gattCharC1Handle != nullptr, ChipLogError(DeviceLayer, "Char C1 is null"));

ret = bt_gatt_set_value(conn->gattCharC1Handle, (const char *) pBuf->Start(), pBuf->DataLength());
ret = bt_gatt_set_value(conn->gattCharC1Handle, reinterpret_cast<const char *>(pBuf->Start()), pBuf->DataLength());
VerifyOrExit(ret == BT_ERROR_NONE, ChipLogError(DeviceLayer, "bt_gatt_set_value() failed. ret: %d", ret));

ChipLogProgress(DeviceLayer, "Sending Write Request for CHIPoBLE TX characteristic (con %s, len %u)", conn->peerAddr,
Expand Down
52 changes: 20 additions & 32 deletions src/platform/Tizen/ChipDeviceScanner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,58 +56,47 @@ std::unique_ptr<ChipDeviceScanner> ChipDeviceScanner::Create(ChipDeviceScannerDe
return std::make_unique<ChipDeviceScanner>(delegate);
}

static void __CleanupServiceData(bt_adapter_le_service_data_s * dataList, int count)
static void __CleanupServiceData(bt_adapter_le_service_data_s * dataList, size_t count)
{
int i;
if (!dataList || count == 0)
return;
VerifyOrReturn(dataList != nullptr);
VerifyOrReturn(count != 0);

for (i = 0; i < count; i++)
for (size_t i = 0; i < count; i++)
{
g_free(dataList[i].service_uuid);
g_free(dataList[i].service_data);
}
g_free(dataList);
}

static void __PrintLEScanData(bt_adapter_le_service_data_s * dataList, int idx)
static void __PrintLEScanData(bt_adapter_le_service_data_s * dataList, size_t idx)
{
int i = 0;
if (!dataList)
return;
VerifyOrReturn(dataList != nullptr);

// Print Service UUID in the Service Data
ChipLogProgress(DeviceLayer, "======Service UUID========\n");
ChipLogProgress(DeviceLayer, " UUID[%s]\n", dataList[idx].service_uuid);

ChipLogProgress(DeviceLayer, "Service Data Length::[%d]\n", dataList[idx].service_data_len);
ChipLogDetail(DeviceLayer, "======Service UUID========");
ChipLogDetail(DeviceLayer, "Service UUID::[%s]", dataList[idx].service_uuid);

// Print Service Data
if (dataList[idx].service_data_len > 0)
ChipLogProgress(DeviceLayer, "=====Service Data====\n");

for (i = 0; i < dataList[idx].service_data_len; i++)
{
ChipLogProgress(DeviceLayer, "Service Data[%d] = [0x%x]\n", i, dataList[idx].service_data[i]);
}
ChipLogDetail(DeviceLayer, "======Service Data========");
ChipLogDetail(DeviceLayer, "Service Data Length::[%d]", dataList[idx].service_data_len);
ChipLogByteSpan(DeviceLayer, ByteSpan(reinterpret_cast<uint8_t *>(dataList[idx].service_data), dataList[idx].service_data_len));
}

static bool __IsChipThingDevice(bt_adapter_le_device_scan_result_info_s * info,
chip::Ble::ChipBLEDeviceIdentificationInfo & aDeviceInfo)
{
VerifyOrReturnError(info != nullptr, false);

int count = 0;
bt_adapter_le_service_data_s * dataList = nullptr;
bool isChipDevice = false;

if (!info)
return false;

ChipLogProgress(DeviceLayer, "Is [%s] ChipThingDevice ?: Check now", info->remote_address);

if (bt_adapter_le_get_scan_result_service_data_list(info, BT_ADAPTER_LE_PACKET_ADVERTISING, &dataList, &count) == BT_ERROR_NONE)
{
int i;
for (i = 0; i < count; i++)
for (int i = 0; i < count; i++)
{
if (g_strcmp0(dataList[i].service_uuid, chip_service_uuid) == 0 ||
g_strcmp0(dataList[i].service_uuid, chip_service_uuid_short) == 0)
Expand All @@ -127,11 +116,10 @@ static bool __IsChipThingDevice(bt_adapter_le_device_scan_result_info_s * info,

void ChipDeviceScanner::LeScanResultCb(int result, bt_adapter_le_device_scan_result_info_s * info, void * userData)
{
ChipDeviceScanner * self = (ChipDeviceScanner *) userData;
chip::Ble::ChipBLEDeviceIdentificationInfo deviceInfo;
VerifyOrReturn(info != nullptr);

if (!info)
return;
auto self = reinterpret_cast<ChipDeviceScanner *>(userData);
chip::Ble::ChipBLEDeviceIdentificationInfo deviceInfo;

ChipLogProgress(DeviceLayer, "LE Device Reported!! remote addr [%s]", info->remote_address);

Expand All @@ -148,16 +136,16 @@ void ChipDeviceScanner::LeScanResultCb(int result, bt_adapter_le_device_scan_res

gboolean ChipDeviceScanner::TimerExpiredCb(gpointer userData)
{
ChipDeviceScanner * self = (ChipDeviceScanner *) userData;
auto self = reinterpret_cast<ChipDeviceScanner *>(userData);
ChipLogProgress(DeviceLayer, "Scan Timer expired!!");
self->StopChipScan();
return G_SOURCE_REMOVE;
}

gboolean ChipDeviceScanner::TriggerScan(GMainLoop * mainLoop, gpointer userData)
{
ChipDeviceScanner * self = (ChipDeviceScanner *) userData;
int ret = BT_ERROR_NONE;
auto self = reinterpret_cast<ChipDeviceScanner *>(userData);
int ret = BT_ERROR_NONE;
GSource * idleSource;

self->mAsyncLoop = mainLoop;
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Tizen/ChipDeviceScanner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ struct ScanFilterData
char address[CHIP_TIZEN_BLE_ADDRESS_STRING_LEN];
char service_uuid[CHIP_TIZEN_BLE_SERVICE_UUID_STRING_LEN];
char service_data[CHIP_TIZEN_BLE_SERVICE_DATA_MAX_LEN];
int service_data_len;
unsigned int service_data_len;
};

// Receives callbacks when chip devices are being scanned
Expand Down
Loading

0 comments on commit 2232064

Please sign in to comment.