Skip to content

Commit b6b087f

Browse files
sanastasiouh2zero
authored andcommitted
fix: whiteListRemove use-after-free — loop continues on invalidated iterator (#1172)
* fix: whiteListRemove use-after-free - stop iterating after erase + shrink whiteListRemove erases the matched element (invalidating the iterator) and shrink-to-fits m_whiteList (freeing the buffer the iterator points into), then continues the loop on the dangling iterator. A layout-dependent spurious re-match then calls erase() with a foreign iterator, and the vector's internal element move writes across unrelated heap memory. Observed on ESP32-C3 as intermittent heap corruption under whitelist churn (multi_heap_free bad-head asserts with address bytes in the corrupted header; a clobbered NimBLE host mutex tripping xQueueSemaphoreTake). Verified fixed under CONFIG_HEAP_POISONING_COMPREHENSIVE plus periodic heap_caps_check_integrity_all() sweeps. whiteListAdd guards duplicates via onWhiteList(), so at most one element can match - breaking out after the removal preserves semantics. * fix: use m_whiteList.data() - operator[] on an empty vector is UB After erasing the last whitelist entry, &m_whiteList[0] indexes an empty vector before ble_gap_wl_set(..., 0). data() is well-defined for empty vectors; same change applied to the whiteListAdd call site for consistency.
1 parent 3365e94 commit b6b087f

1 file changed

Lines changed: 6 additions & 2 deletions

File tree

src/NimBLEDevice.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -728,7 +728,7 @@ bool NimBLEDevice::onWhiteList(const NimBLEAddress& address) {
728728
bool NimBLEDevice::whiteListAdd(const NimBLEAddress& address) {
729729
if (!NimBLEDevice::onWhiteList(address)) {
730730
m_whiteList.push_back(address);
731-
int rc = ble_gap_wl_set(reinterpret_cast<ble_addr_t*>(&m_whiteList[0]), m_whiteList.size());
731+
int rc = ble_gap_wl_set(reinterpret_cast<ble_addr_t*>(m_whiteList.data()), m_whiteList.size());
732732
if (rc != 0) {
733733
NIMBLE_LOGE(LOG_TAG, "Failed adding to whitelist rc=%d", rc);
734734
m_whiteList.pop_back();
@@ -748,14 +748,18 @@ bool NimBLEDevice::whiteListRemove(const NimBLEAddress& address) {
748748
for (auto it = m_whiteList.begin(); it < m_whiteList.end(); ++it) {
749749
if (*it == address) {
750750
m_whiteList.erase(it);
751-
int rc = ble_gap_wl_set(reinterpret_cast<ble_addr_t*>(&m_whiteList[0]), m_whiteList.size());
751+
int rc = ble_gap_wl_set(reinterpret_cast<ble_addr_t*>(m_whiteList.data()), m_whiteList.size());
752752
if (rc != 0) {
753753
m_whiteList.push_back(address);
754754
NIMBLE_LOGE(LOG_TAG, "Failed removing from whitelist rc=%d", rc);
755755
return false;
756756
}
757757

758758
std::vector<NimBLEAddress>(m_whiteList).swap(m_whiteList);
759+
break; // `it` was invalidated by erase() and its buffer freed by the
760+
// swap above; continuing would iterate a dangling iterator.
761+
// Duplicates are impossible (whiteListAdd checks onWhiteList),
762+
// so a single match is total.
759763
}
760764
}
761765

0 commit comments

Comments
 (0)