Skip to content

Commit 3ed9249

Browse files
author
0ldev
committed
fix: two bugs found in code review
Bug 1 — soft_ap_ssid buffer overflow (Politician.cpp begin()) wifi_ap_config_t::ssid is uint8_t[32]. The previous memcpy used raw strlen() without any guard, so a caller-supplied SSID longer than 32 bytes would write past the ssid[] field into the adjacent password[] field of the stack-allocated wifi_config_t. Added: if (ap_ssid_len > 32) ap_ssid_len = 32; before the memcpy. Bug 2 — TOCTOU race in setTargetBySsid() (Politician.cpp) The function released _lock before dereferencing _apCache[best].bssid and _apCache[best].channel. On a dual-core ESP32, the worker task on Core 0 can evict or overwrite that cache slot between the lock release and the setTarget() call, causing the engine to lock onto the wrong AP. Fix: copy bssid/channel into local variables before xSemaphoreGive, then call setTarget() with the local copies. Note: ClientFoundCb signature change ClientFoundCb changed from (bssid, sta, rssi) to (const ClientRecord &). Added migration note at top of README.
1 parent 8ba74e1 commit 3ed9249

2 files changed

Lines changed: 30 additions & 7 deletions

File tree

README.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,22 @@
77

88
Politician is an embedded C++ library designed for WiFi security auditing on ESP32 platforms. It provides a clean, modern API for capturing WPA/WPA2/WPA3 handshakes and harvesting enterprise credentials using advanced 802.11 protocol techniques.
99

10+
## Migration Notes
11+
12+
### ClientFoundCb signature change (develop)
13+
14+
`ClientFoundCb` was changed from `(const uint8_t *bssid, const uint8_t *sta, int8_t rssi)` to `(const ClientRecord &rec)`. Update existing callbacks:
15+
16+
```cpp
17+
// Before
18+
engine.setClientFoundCallback([](const uint8_t *bssid, const uint8_t *sta, int8_t rssi) { … });
19+
20+
// After
21+
engine.setClientFoundCallback([](const ClientRecord &rec) {
22+
// rec.bssid, rec.sta, rec.rssi — same data plus rand_mac, vendor, timestamps
23+
});
24+
```
25+
1026
## Key Capabilities
1127

1228
- **PMKID Capture** — Extract PMKIDs via fake association without disconnecting any client

src/Politician.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,10 @@ Error Politician::begin(const Config &cfg) {
146146

147147
wifi_config_t ap_cfg = {};
148148
const char *ap_ssid = _cfg.soft_ap_ssid ? _cfg.soft_ap_ssid : "WiFighter";
149-
memcpy(ap_cfg.ap.ssid, ap_ssid, strlen(ap_ssid));
150-
ap_cfg.ap.ssid_len = (uint8_t)strlen(ap_ssid);
149+
size_t ap_ssid_len = strlen(ap_ssid);
150+
if (ap_ssid_len > 32) ap_ssid_len = 32; // wifi_ap_config_t::ssid is uint8_t[32]
151+
memcpy(ap_cfg.ap.ssid, ap_ssid, ap_ssid_len);
152+
ap_cfg.ap.ssid_len = (uint8_t)ap_ssid_len;
151153
ap_cfg.ap.ssid_hidden = _cfg.soft_ap_ssid ? 0 : 1;
152154
ap_cfg.ap.max_connection = 4;
153155
ap_cfg.ap.authmode = WIFI_AUTH_OPEN;
@@ -573,22 +575,27 @@ void Politician::setChannelBands(bool ghz24, bool ghz5) {
573575
Error Politician::setTargetBySsid(const char *ssid) {
574576
if (!_initialized) return ERR_NOT_ACTIVE;
575577
uint8_t ssid_len = (uint8_t)strlen(ssid);
576-
int best = -1;
577-
int8_t best_rssi = INT8_MIN;
578+
uint8_t found_bssid[6] = {};
579+
uint8_t found_channel = 0;
580+
bool found = false;
581+
int8_t best_rssi = INT8_MIN;
578582
if (_lock && xSemaphoreTakeRecursive(_lock, pdMS_TO_TICKS(100)) == pdTRUE) {
579583
for (int i = 0; i < MAX_AP_CACHE; i++) {
580584
if (!_apCache[i].flags.active) continue;
581585
if (_apCache[i].ssid_len != ssid_len) continue;
582586
if (memcmp(_apCache[i].ssid, ssid, ssid_len) != 0) continue;
583587
if (_apCache[i].rssi > best_rssi) {
584588
best_rssi = _apCache[i].rssi;
585-
best = i;
589+
// Copy out before releasing lock — avoids TOCTOU with worker task
590+
memcpy(found_bssid, _apCache[i].bssid, 6);
591+
found_channel = _apCache[i].channel;
592+
found = true;
586593
}
587594
}
588595
xSemaphoreGiveRecursive(_lock);
589596
}
590-
if (best == -1) return ERR_NOT_FOUND;
591-
return setTarget(_apCache[best].bssid, _apCache[best].channel);
597+
if (!found) return ERR_NOT_FOUND;
598+
return setTarget(found_bssid, found_channel);
592599
}
593600

594601
void Politician::setAutoTarget(bool enable) {

0 commit comments

Comments
 (0)