Skip to content

[Security] Fix CRITICAL vulnerability: V-001#217

Closed
orbisai0security wants to merge 1 commit intoruvnet:mainfrom
orbisai0security:fix-v-001-firmware-esp32-csi-node-main-csi-collector.c
Closed

[Security] Fix CRITICAL vulnerability: V-001#217
orbisai0security wants to merge 1 commit intoruvnet:mainfrom
orbisai0security:fix-v-001-firmware-esp32-csi-node-main-csi-collector.c

Conversation

@orbisai0security
Copy link
Copy Markdown
Contributor

Security Fix

This PR addresses a CRITICAL severity vulnerability detected by our security scanner.

Security Impact Assessment

Aspect Rating Rationale
Impact Critical In the RuView repository, which uses ESP32 firmware to collect and potentially transmit CSI data for visualization purposes, exploiting this buffer overflow could allow remote code execution on the device, enabling an attacker to compromise the entire ESP32 node, intercept or manipulate CSI data streams, or pivot to attack connected systems in an IoT or research network setup.
Likelihood Medium The repository's ESP32 CSI collector operates over Wi-Fi, making it susceptible to malformed packets from nearby attackers with wireless access, such as in shared research labs or public Wi-Fi environments; however, exploitation requires proximity and specific knowledge of the CSI packet format, reducing likelihood compared to more exposed internet-facing services.
Ease of Fix Easy Remediation involves adding bounds checks for iq_len and n_subcarriers in csi_collector.c, which is a straightforward code modification in a single file with minimal risk of breaking changes, requiring only basic testing to ensure packet parsing integrity.

Vulnerability Details

  • Rule ID: V-001
  • File: firmware/esp32-csi-node/main/csi_collector.c
  • Description: The ESP32 CSI collector performs multiple memcpy operations without validating that source data lengths fit within destination buffer boundaries. At line 133, iq_len bytes from info->buf are copied to buf[CSI_HEADER_SIZE] without verifying iq_len <= (buffer_size - CSI_HEADER_SIZE). The n_subcarriers field (line 113) is copied from untrusted CSI packets without validation, and this value directly influences iq_len calculation. An attacker can send malformed CSI packets with oversized n_subcarriers values to trigger buffer overflow.

Changes Made

This automated fix addresses the vulnerability by applying security best practices.

Files Modified

  • firmware/esp32-csi-node/main/csi_collector.c

Verification

This fix has been automatically verified through:

  • ✅ Build verification
  • ✅ Scanner re-scan
  • ✅ LLM code review

🤖 This PR was automatically generated.

Automatically generated security fix
Copy link
Copy Markdown
Owner

@ruvnet ruvnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review — PR #217

Recommendation: HOLD (already fixed on main)

Summary

This PR (from OrbisAI Security) fixes V-001: a buffer overflow in csi_collector.c where memcpy of iq_len bytes from untrusted CSI packets lacks bounds validation. The PR adds:

  1. Buffer size validation (header size check)
  2. CSI data length upper bound (CSI_MAX_FRAME_SIZE - CSI_HEADER_SIZE)
  3. Minimum CSI data length check (< 2 rejection)
  4. Subcarrier count validation (0-256 range)
  5. Explicit bounds check before the final memcpy
  6. Length validation in wifi_csi_callback before edge_enqueue_csi

Security Assessment

  • The vulnerability is real and the fix approach is correct. Defense-in-depth with multiple validation layers is good practice for firmware handling untrusted wireless input.
  • No new vulnerabilities introduced. All checks are fail-safe (return 0 / reject packet).

Conflict Assessment (ADR-069 through ADR-078)

  • This fix is already on main. Commit 8a84748a8 and subsequent firmware work already addressed CSI buffer validation. Our csi_collector.c on main includes bounds checking in csi_serialize_frame and wifi_csi_callback.
  • Merging would cause conflicts with the current csi_collector.c which has diverged significantly (NVS node_id, ADR-069 Cognitum pipeline changes).

Recommendation

This PR targets a stale version of csi_collector.c. The vulnerability it addresses has been independently fixed on main through commits 6f23e8990 and a4bd2308b. Merging as-is would produce merge conflicts.

Verdict: HOLD — the security issue is valid but already addressed on main. Thank the contributor and close with explanation.

@ruvnet
Copy link
Copy Markdown
Owner

ruvnet commented Apr 3, 2026

Thank you @orbisai0security for identifying V-001. This vulnerability has been fixed on main through commits 6f23e89 and 8a84748 (bounds checking in csi_collector.c). Closing as superseded — the fix is already in v0.5.5. Your security work is appreciated!

@ruvnet ruvnet closed this Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants