fix: Address P1 security audit findings
- VULN-010: Prevent CSI UDP buffer overflow with bounds-checked serialization loops and clamped pos before sendto - VULN-019: Validate probe frame sig_len before body access - VULN-017: Add NVS write throttle to config_erase_key() - VULN-009: Tighten HMAC replay window from ±30s to ±5s, add nonce dedup cache (8 entries) to reject exact replays within window - VULN-004/018: Add 50ms rate limit on command socket (20 cmd/s max) - VULN-014: Stage baseline calibration in local buffer, gate with atomic nsub write to prevent partial reads from CSI callback
This commit is contained in:
@@ -440,6 +440,7 @@ static esp_err_t config_save_blob(const char *key, const void *data, size_t len)
|
||||
|
||||
static esp_err_t config_erase_key(const char *key)
|
||||
{
|
||||
if (!nvs_write_throttle()) return ESP_ERR_INVALID_STATE;
|
||||
nvs_handle_t h;
|
||||
esp_err_t err = nvs_open("csi_config", NVS_READWRITE, &h);
|
||||
if (err != ESP_OK) return err;
|
||||
@@ -701,24 +702,29 @@ static void wifi_csi_rx_cb(void *ctx, wifi_csi_info_t *info)
|
||||
rx_ctrl->timestamp, rx_ctrl->ant, rx_ctrl->sig_len, rx_ctrl->rx_state);
|
||||
#endif
|
||||
|
||||
/* Helper: remaining space in s_udp_buffer (clamped to 0) */
|
||||
#define UDP_REM(p) ((p) < (int)sizeof(s_udp_buffer) ? sizeof(s_udp_buffer) - (p) : 0)
|
||||
|
||||
if (send_raw) {
|
||||
/* Raw I/Q array payload */
|
||||
#if (CONFIG_IDF_TARGET_ESP32C5 || CONFIG_IDF_TARGET_ESP32C61) && CSI_FORCE_LLTF
|
||||
int16_t csi = ((int16_t)(((((uint16_t)info->buf[1]) << 8) | info->buf[0]) << 4) >> 4);
|
||||
pos += snprintf(s_udp_buffer + pos, sizeof(s_udp_buffer) - pos,
|
||||
pos += snprintf(s_udp_buffer + pos, UDP_REM(pos),
|
||||
",%d,%d,\"[%d", (info->len - 2) / 2, info->first_word_invalid, (int16_t)(compensate_gain * csi));
|
||||
for (int i = 2; i < (info->len - 2); i += 2) {
|
||||
for (int i = 2; i < (info->len - 2) && pos < (int)sizeof(s_udp_buffer) - 8; i += 2) {
|
||||
csi = ((int16_t)(((((uint16_t)info->buf[i + 1]) << 8) | info->buf[i]) << 4) >> 4);
|
||||
pos += snprintf(s_udp_buffer + pos, sizeof(s_udp_buffer) - pos, ",%d", (int16_t)(compensate_gain * csi));
|
||||
pos += snprintf(s_udp_buffer + pos, UDP_REM(pos), ",%d", (int16_t)(compensate_gain * csi));
|
||||
}
|
||||
#else
|
||||
pos += snprintf(s_udp_buffer + pos, sizeof(s_udp_buffer) - pos,
|
||||
pos += snprintf(s_udp_buffer + pos, UDP_REM(pos),
|
||||
",%d,%d,\"[%d", info->len, info->first_word_invalid, (int16_t)(compensate_gain * info->buf[0]));
|
||||
for (int i = 1; i < info->len; i++) {
|
||||
pos += snprintf(s_udp_buffer + pos, sizeof(s_udp_buffer) - pos, ",%d", (int16_t)(compensate_gain * info->buf[i]));
|
||||
for (int i = 1; i < info->len && pos < (int)sizeof(s_udp_buffer) - 8; i++) {
|
||||
pos += snprintf(s_udp_buffer + pos, UDP_REM(pos), ",%d", (int16_t)(compensate_gain * info->buf[i]));
|
||||
}
|
||||
#endif
|
||||
pos += snprintf(s_udp_buffer + pos, sizeof(s_udp_buffer) - pos, "]\"\n");
|
||||
if (pos < (int)sizeof(s_udp_buffer) - 4) {
|
||||
pos += snprintf(s_udp_buffer + pos, UDP_REM(pos), "]\"\n");
|
||||
}
|
||||
} else {
|
||||
/* Compact feature payload */
|
||||
pos += snprintf(s_udp_buffer + pos, sizeof(s_udp_buffer) - pos,
|
||||
@@ -728,6 +734,9 @@ static void wifi_csi_rx_cb(void *ctx, wifi_csi_info_t *info)
|
||||
(unsigned)features.amp_max_idx, (unsigned long)features.energy);
|
||||
}
|
||||
|
||||
/* Clamp pos to buffer size */
|
||||
if (pos > (int)sizeof(s_udp_buffer)) pos = (int)sizeof(s_udp_buffer);
|
||||
|
||||
/* Send via UDP */
|
||||
if (s_udp_socket >= 0) {
|
||||
sendto(s_udp_socket, s_udp_buffer, pos, 0, (struct sockaddr *)&s_dest_addr, sizeof(s_dest_addr));
|
||||
@@ -992,9 +1001,14 @@ static void adaptive_task(void *arg)
|
||||
if (s_calibrating && s_calib_count >= (uint32_t)s_calib_target) {
|
||||
int nsub = s_calib_nsub;
|
||||
uint32_t count = s_calib_count;
|
||||
/* Stage baseline in local buffer to avoid partial reads from CSI callback */
|
||||
float staged[BASELINE_MAX_SUBS];
|
||||
for (int i = 0; i < nsub; i++) {
|
||||
s_baseline_amps[i] = s_calib_accum[i] / (float)count;
|
||||
staged[i] = s_calib_accum[i] / (float)count;
|
||||
}
|
||||
/* Atomically gate: zero nsub first, copy, then set nsub */
|
||||
s_baseline_nsub = 0;
|
||||
memcpy(s_baseline_amps, staged, nsub * sizeof(float));
|
||||
s_baseline_nsub = nsub;
|
||||
config_save_blob("bl_amps", s_baseline_amps, nsub * sizeof(float));
|
||||
config_save_i8("bl_nsub", (int8_t)nsub);
|
||||
@@ -1380,6 +1394,9 @@ static void wifi_promiscuous_cb(void *buf, wifi_promiscuous_pkt_type_t type)
|
||||
/* Dedup: skip if this MAC was reported recently */
|
||||
if (probe_dedup_check(hdr->addr2)) return;
|
||||
|
||||
/* Validate frame length before accessing body */
|
||||
if (pkt->rx_ctrl.sig_len < sizeof(wifi_ieee80211_mac_hdr_t) + 2) return;
|
||||
|
||||
/* Parse SSID from tagged parameters after MAC header */
|
||||
const uint8_t *body = pkt->payload + sizeof(wifi_ieee80211_mac_hdr_t);
|
||||
int body_len = pkt->rx_ctrl.sig_len - sizeof(wifi_ieee80211_mac_hdr_t);
|
||||
@@ -1429,10 +1446,19 @@ static void wifi_promiscuous_init(void)
|
||||
|
||||
/* --- HMAC command authentication --- */
|
||||
|
||||
/* Nonce dedup cache: reject exact replay of timestamp+HMAC within the window */
|
||||
#define AUTH_NONCE_CACHE_SIZE 8
|
||||
static struct {
|
||||
long ts;
|
||||
uint8_t mac_prefix[4]; /* first 4 bytes of HMAC hex for quick match */
|
||||
} s_auth_nonce_cache[AUTH_NONCE_CACHE_SIZE];
|
||||
static int s_auth_nonce_idx = 0;
|
||||
|
||||
/**
|
||||
* Verify HMAC-signed command. Format: "HMAC:<16hex>:<uptime_s>:<cmd>"
|
||||
* HMAC = truncated SHA-256(secret, "<uptime_s>:<cmd>")
|
||||
* Timestamp must be within 30s of device uptime (replay protection).
|
||||
* Timestamp must be within 5s of device uptime (replay protection).
|
||||
* Recently used timestamp+HMAC pairs are cached to reject exact replays.
|
||||
* Returns pointer to actual command on success, or NULL on failure
|
||||
* (with error message written to reply).
|
||||
*/
|
||||
@@ -1467,11 +1493,11 @@ static const char *auth_verify(const char *input, char *reply, size_t reply_size
|
||||
return NULL;
|
||||
}
|
||||
|
||||
/* Replay protection: reject stale timestamps */
|
||||
/* Replay protection: reject stale timestamps (±5s window) */
|
||||
long ts = strtol(payload, NULL, 10);
|
||||
int64_t now_s = esp_timer_get_time() / 1000000LL;
|
||||
int64_t drift = now_s - (int64_t)ts;
|
||||
if (drift < -30 || drift > 30) {
|
||||
if (drift < -5 || drift > 5) {
|
||||
snprintf(reply, reply_size, "ERR AUTH expired (drift=%llds)", (long long)drift);
|
||||
return NULL;
|
||||
}
|
||||
@@ -1505,6 +1531,20 @@ static const char *auth_verify(const char *input, char *reply, size_t reply_size
|
||||
return NULL;
|
||||
}
|
||||
|
||||
/* Nonce dedup: reject if this exact timestamp+HMAC was already used */
|
||||
for (int i = 0; i < AUTH_NONCE_CACHE_SIZE; i++) {
|
||||
if (s_auth_nonce_cache[i].ts == ts &&
|
||||
memcmp(s_auth_nonce_cache[i].mac_prefix, input + 5, 4) == 0) {
|
||||
snprintf(reply, reply_size, "ERR AUTH replay rejected");
|
||||
return NULL;
|
||||
}
|
||||
}
|
||||
|
||||
/* Record this nonce */
|
||||
s_auth_nonce_cache[s_auth_nonce_idx % AUTH_NONCE_CACHE_SIZE].ts = ts;
|
||||
memcpy(s_auth_nonce_cache[s_auth_nonce_idx % AUTH_NONCE_CACHE_SIZE].mac_prefix, input + 5, 4);
|
||||
s_auth_nonce_idx++;
|
||||
|
||||
return cmd;
|
||||
}
|
||||
|
||||
@@ -2529,6 +2569,9 @@ static void cmd_task(void *arg)
|
||||
struct sockaddr_in src_addr;
|
||||
socklen_t src_len;
|
||||
|
||||
/* Rate limiting: min 50ms between commands (20 cmd/s max) */
|
||||
int64_t last_cmd_time = 0;
|
||||
|
||||
while (1) {
|
||||
src_len = sizeof(src_addr);
|
||||
int len = recvfrom(sock, rx_buf, sizeof(rx_buf) - 1, 0,
|
||||
@@ -2539,6 +2582,13 @@ static void cmd_task(void *arg)
|
||||
continue;
|
||||
}
|
||||
|
||||
/* Rate limit: drop packets arriving faster than 50ms apart */
|
||||
int64_t now_cmd = esp_timer_get_time();
|
||||
if (now_cmd - last_cmd_time < 50000) {
|
||||
continue;
|
||||
}
|
||||
last_cmd_time = now_cmd;
|
||||
|
||||
/* Strip trailing whitespace */
|
||||
while (len > 0 && (rx_buf[len - 1] == '\n' || rx_buf[len - 1] == '\r' || rx_buf[len - 1] == ' ')) {
|
||||
len--;
|
||||
|
||||
Reference in New Issue
Block a user