fix: whiteListRemove use-after-free — loop continues on invalidated iterator

[sanastasiou]

whiteListRemove iterates m_whiteList and, on a match:

1. m_whiteList.erase(it) — invalidates it (and all iterators at/after it),
2. std::vector<NimBLEAddress>(m_whiteList).swap(m_whiteList) — the shrink-to-fit swap frees the ENTIRE buffer it points into,
3. the loop then continues: ++it and it < m_whiteList.end() operate on a dangling iterator into freed memory — undefined behavior on every removal that finds a match.

When the freed buffer's stale pointer happens to compare "in range" against the new allocation (pure heap-layout luck), *it == address reads freed memory, and a spurious re-match calls m_whiteList.erase(it) with a foreign iterator — the vector's internal element move then writes address-shaped data across unrelated heap memory.

Observed in the field (ESP32-C3, arduino-esp32 / IDF 5.5.4) as two distinct intermittent heap-corruption crashes under whitelist churn, both inside or adjacent to whiteListRemove:

• assert failed: multi_heap_free multi_heap_poisoning.c:279 (head != NULL) — the vector buffer's heap-block header overwritten with BLE-address bytes (CORRUPT HEAP: Bad head ... Expected 0xabba1234 got 0xbb250000, the 25 bb being the LSB-first wire bytes of a whitelisted peer address);
• assert failed: xQueueSemaphoreTake queue.c:1713 (pxQueue->uxItemSize == 0) — the NimBLE host mutex clobbered by the same wild write, tripping on the next ble_hs_lock().

Fix: break after the shrink. Since whiteListAdd guards against duplicates (onWhiteList() check before push_back), at most one element can match — breaking out after the removal preserves semantics exactly.

Verification: with this fix, the same churn workload (repeated whitelist rebuilds around pair/disconnect cycles) runs clean under CONFIG_HEAP_POISONING_COMPREHENSIVE plus periodic heap_caps_check_integrity_all() sweeps — previously ~2 crashes per 5 cycles.

Side note (not touched here to keep the diff minimal): &m_whiteList[0] on line 751 is also UB when the vector becomes empty after erasing the last entry — m_whiteList.data() would be the safe spelling.

Summary by CodeRabbit

• Bug Fixes
  • Improved Bluetooth whitelist management when adding or removing devices.
  • Fixed an issue that could cause unreliable whitelist updates or instability after removing an entry.
  • Whitelist changes now complete more consistently during device connectivity operations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 081ef182-b708-4c1e-92e8-6562ab290e60

📥 Commits

Reviewing files that changed from the base of the PR and between a9eab2f and 9477550.

📒 Files selected for processing (1)

• src/NimBLEDevice.cpp

---

📝 Walkthrough

Walkthrough

Arrr, whitelist updates now use data() for BLE API calls, while removal stops iterating after a successful erase and container swap to avoid dangling-iterator access.

Changes

Whitelist update safety

Layer / File(s)	Summary
Safe whitelist updates src/NimBLEDevice.cpp	whiteListAdd and whiteListRemove pass m_whiteList.data() to ble_gap_wl_set; successful removal now breaks immediately after the container swap, while failure recovery remains intact. Yarrr.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: h2zero

Poem

The whitelist sails with data() in sight,
No dangling iterator haunts the night.
A removed mate leaves the loop with cheer,
Failure restores what once was here.
Yarrr, safer waters for the BLE crew!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately highlights the main fix in whiteListRemove and matches the reported use-after-free issue.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}