[modem]: Harden the lib after security audit

[david-cermak]

esp_modem Deep Security Assessment

Date: 2026-02-23
Component: esp_modem (ESP-IDF protocols repository)
Threat Model: Compromised or malicious modem sending arbitrary data over the serial link; network-based attacks on socket terminals. API parameters (SMS number, PIN, APN, etc.) are considered developer-controlled and their validation is the application's responsibility.

---

Executive Summary

This assessment covers 6 primary attack surfaces of the esp_modem component. A total of 51 findings were identified across all areas:

Severity	Count
Critical	2
High	10
Medium	19
Low	14
Informational	6

Top critical/high findings requiring immediate attention:

1. AT response parsing bugs (AT-001/003/004/005/006) — a malicious modem can trigger undefined behavior, return wrong parsed values, or cause inverted pointer ranges in from_chars via crafted AT responses. [FIXED]
2. URC stale state → OOB access (URC-002) — buffer_state.last_urc_processed is never reset between commands, causing unsigned underflow and out-of-bounds pointers in the enhanced URC handler. [FIXED]
3. URC deadlock (URC-001) — a URC handler calling DTE::command() causes a cross-thread deadlock.
4. No CMUX CRC validation (CMUX-001) — in the default configuration, CMUX frames are accepted without any integrity checking.
5. Netif destructor use-after-free (PPP-004) — the DTE read callback captures a dangling this pointer after Netif destruction.
6. Unbounded inflatable buffer (UART-001) — a malicious modem can cause unbounded heap growth by never completing a response.

---

1. UART / Terminal Read Paths (Modem I/O)

Files: src/esp_modem_uart.cpp, src/esp_modem_term_fs.cpp, src/esp_modem_dte.cpp

UART-001: Unbounded Inflatable Buffer Growth — Memory Exhaustion DoS

Severity: High | esp_modem_dte.cpp:81-97, 119-132, 457-467

When CONFIG_ESP_MODEM_USE_INFLATABLE_BUFFER_IF_NEEDED is enabled, extra_buffer::grow() calls std::vector::resize() with no upper bound. A malicious modem can trigger unbounded heap growth by sending continuous data without a command separator. At 921600 baud with a 30-second command timeout, ~3.4 MB can be allocated — enough to exhaust ESP32 SRAM.

Recommendation: Add a configurable maximum size (e.g., CONFIG_ESP_MODEM_INFLATABLE_BUFFER_MAX_SIZE). In grow(), reject sizes exceeding the limit and call give_up().

UART-002: Negative read() Return → SIZE_MAX Length

Severity: High | esp_modem_uart.cpp:173-186, esp_modem_dte.cpp:111,127

UartTerminal::read() returns int (can be -1 on error). All DTE callers assign to size_t len, converting -1 to SIZE_MAX. Subsequent memchr(data, separator, SIZE_MAX) reads far past the buffer.

Recommendation: Check read_len < 0 and return 0, matching FdTerminal's behavior.

UART-003: Data Race on on_read Callback in UartTerminal

Severity: Medium | esp_modem_uart.cpp:65-68, 124-126

std::function on_read is written from the caller's task and read from the UART FreeRTOS task with no synchronization. On dual-core ESP32, concurrent access to std::function internals is undefined behavior.

Recommendation: Protect with a mutex or use an atomic flag + signaling pattern.

UART-004: Data Race on on_read Callback in FdTerminal

Severity: Medium | esp_modem_term_fs.cpp:55-59, 113-114

Similar to UART-003. The TASK_PARAMS signal mitigation is incomplete — the std::function copy at line 114 can race with set_read_cb().

UART-005: No CRC Validation for Standard CMUX Payloads

Severity: Medium | esp_modem_cmux.cpp:300-306

(Cross-reference with CMUX-001.) CRC is only validated under ESP_MODEM_CMUX_USE_SHORT_PAYLOADS_ONLY.

UART-006: Use-After-Free Risk During DTE Destruction

Severity: Medium | esp_modem_dte.hpp:56, esp_modem_dte.cpp:64-151

DTE::~DTE() = default relies on member destruction order, but terminal tasks capture this in lambdas. A task executing the lambda during destruction accesses already-destroyed members.

Recommendation: Explicitly stop terminals and clear callbacks in ~DTE().

UART-007: Unsigned Underflow in URC CONSUME_PARTIAL Handler

Severity: Low | esp_modem_dte.cpp:399-405

If consume_info.consume_size > consumed + len, the subtraction underflows. Currently benign due to len = 0, but a latent vulnerability.

UART-008: FdTerminal Inflatable Buffer Path Reads 0 Bytes

Severity: Low | esp_modem_dte.cpp:119-132, esp_modem_term_fs.cpp:124-125

FdTerminal passes len=0 to the callback, causing the inflatable buffer path to read 0 bytes and stall until timeout.

UART-009: TOCTOU Between uart_get_buffered_data_len and uart_read_bytes

Severity: Informational | esp_modem_uart.cpp:173-186

Classic TOCTOU but not exploitable — reads are bounded by the caller's buffer capacity.

UART-010: No Overflow Guard on consumed + len Arithmetic

Severity: Informational | esp_modem_dte.cpp:112,115,429-430

size_t addition without overflow check. Not practically reachable given value constraints.

---

2. CMUX Frame Parsing / De-fragmentation

Files: src/esp_modem_cmux.cpp, include/cxx_include/esp_modem_cmux.hpp

CMUX-001: No CRC Validation in Default Configuration

Severity: High | esp_modem_cmux.cpp:300-306

FCS CRC validation is only compiled when ESP_MODEM_CMUX_USE_SHORT_PAYLOADS_ONLY is enabled (default: disabled). The parser accepts any frame matching basic structural checks (SOF markers, DLCI range, frame type). A malicious modem or line injector can forge arbitrary CMUX frames.

Recommendation: Always validate FCS CRC regardless of configuration.

CMUX-003: Unbounded payload_len Enables Soft DoS

Severity: Medium | esp_modem_cmux.cpp:238, 255

Extended-length frames allow payload_len up to 32767 bytes. No validation against buffer size. Forces the parser into extended blocking reads.

Recommendation: Enforce payload_len <= buffer.size - header_overhead.

CMUX-004: Defragmentation Buffer Arithmetic Assumptions

Severity: Medium | esp_modem_cmux.cpp:324-348

buffer.size - 128 underflows if buffer.size < 128. Pointer comparison at line 329 assumes no wrap.

Recommendation: Add assert(buffer.size >= 128) or a runtime check.

CMUX-007: Header Sanity Check Without CRC Allows Frame Injection

Severity: Medium | esp_modem_cmux.cpp:246-258

Without CRC (CMUX-001), the weak structural check allows forged UA|PF frames (spoofing SABM acks) and UIH frames with arbitrary payload.

CMUX-008: Defragmentation Fallback Delivers Inconsistent Payload

Severity: Medium | esp_modem_cmux.cpp:337-348

When the buffer is insufficient, partial delivery followed by fragment delivery for the same frame causes upper layers to misparse data.

CMUX-002: Unsigned Subtraction in Extended Header Path

Severity: Low | esp_modem_cmux.cpp:236, 239

5 - frame_header_offset and payload_offset - 1 lack defensive guards. Currently safe due to control-flow invariants, but fragile.

CMUX-005: Null payload_start Delivered to Callback on Zero-Length Frames

Severity: Low | esp_modem_cmux.cpp:155-157

With defragmentation enabled, zero-length UIH frames invoke the callback with nullptr, 0.

CMUX-006: advance() Uses Signed int for size_t Operations

Severity: Low | esp_modem_cmux.cpp:115-119

Narrowing conversion risk. Change to void advance(size_t size = 1) with an assertion.

CMUX-009: Footer memcpy Relies on Implicit Invariant

Severity: Low | esp_modem_cmux.cpp:289-295

Safety depends on frame_header_offset ∈ [4,5] — add a defensive check.

---

3. PPP Data Injection into Netif

Files: src/esp_modem_netif.cpp, src/esp_modem_dce.cpp

PPP-002: exit_data() Forwards Non-PPP Data to Network Stack

Severity: High | esp_modem_dce.cpp:23-43

During DATA→COMMAND transition, any data chunk containing a 0x7E byte anywhere is forwarded to netif.receive(), even if it's not a valid PPP frame. The two if blocks (PPP check and command check) are independent, causing dual-forwarding of ambiguous data.

Recommendation: Make if/else if; validate that data starts with 0x7E; only forward before netif.stop().

PPP-004: Destructor Does Not Reset DTE Read Callback — Use-After-Free

Severity: High | esp_modem_netif.cpp:121-133

The destructor never clears the DTE read callback, which captures this. After destruction, incoming data invokes the lambda with a dangling pointer.

Recommendation: Add ppp_dte->set_read_cb(nullptr); as the first line of ~Netif().

PPP-001: No Input Validation in Netif::receive()

Severity: Medium | esp_modem_netif.cpp:67-70

No null-pointer check, no zero-length check, no upper bound on len. The size_t → int narrowing chain through esp_netif_receive → pppos_input_tcpip_as_ram_pbuf could theoretically cause heap corruption.

Recommendation: Validate data != nullptr && len > 0 && len <= buffer_size before forwarding.

PPP-003: Race Between stop() and Active Read Callback

Severity: Medium | esp_modem_netif.cpp:89-105

The read callback unconditionally forwards data; stop() does not clear it. After ppp_close() is posted but before it executes, data can be processed by a closing PPP session.

PPP-005: Destructor Timeout Leaves Driver Handle Dangling

Severity: Medium | esp_modem_netif.cpp:123-127

If signal.wait(PPP_EXIT, 30000) times out, the destructor proceeds. lwIP may still reference the destroyed Netif through driver_ifconfig.handle.

PPP-006: resume() Without Guard Can Feed Uninitialized PPP

Severity: Low | esp_modem_netif.cpp:107-114

Calling resume() without a prior start() + pause() feeds data to an uninitialized PPP session.

PPP-007: esp_netif_receive Has No Input Validation (By Design)

Severity: Informational | ext/idf/.../esp_netif_lwip.c:1254-1262

lwIP PPP's pppos_input() is a robust byte-by-byte parser with FCS validation. Malformed frames are reliably dropped. The PPP trust model inherently trusts the modem as a network interface.

PPP-008: exit_data() Wastes pbufs on Stopped PPP Interface

Severity: Low | esp_modem_dce.cpp:46-52

After netif.stop(), forwarded data allocates pbufs that are immediately discarded.

---

4. AT Command Response Parsing

File: src/esp_modem_command_library.cpp

AT-001: Undefined Behavior on Empty Token in generic_get_string [FIXED]

Severity: Medium | esp_modem_command_library.cpp:92

A malicious modem sending \n\n as a response creates an empty string_view token. The original code computed token.end() - 1 on an empty view — undefined behavior. A conforming compiler may optimize away the loop guard under UB assumptions.

Fix applied: Replaced the reverse-iterator loop with while (!token.empty() && (token.back() == '\r' || token.back() == '\n')).

AT-003: Wrong substr Length in Quote Stripping [FIXED]

Severity: Medium | esp_modem_command_library.cpp:266

A malicious modem sending +COPS: 1,0,X"Attacker",7 causes get_operator_name to extract extra characters from the response. operator_name.substr(quote1 + 1, quote2 - 1) uses quote2 - 1 as count, which is only correct when quote1 == 0. When the modem places characters before the first quote, the extracted name includes trailing data.

Fix applied: Changed to substr(quote1 + 1, quote2 - quote1 - 1) with a quote2 > quote1 guard.

AT-004: Missing Comma in get_network_system_mode Returns Wrong Value [FIXED]

Severity: Medium | esp_modem_command_library.cpp:573-578

A malicious modem sending +CNSMOD: 7 (no comma) causes int mode_pos = out.find(",") + 1 to yield mode_pos = 0 (since npos + 1 = 0). from_chars then parses from position 0, and if the modem crafts the line to start with a digit (e.g., by including it in a previous URC), an attacker-controlled value is returned.

Fix applied: Added explicit comma-not-found check; return FAIL if no comma.

AT-005: from_chars Out-of-Range Not Checked [FIXED]

Severity: Medium | Multiple locations

A malicious modem sending very large numeric values (e.g., +CSQ: 99999999999999999,3) triggers std::errc::result_out_of_range, but only errc::invalid_argument was checked. The output parameter is left unmodified per the C++17 spec — returning whatever stale/uninitialized value was in the caller's variable as if parsing succeeded.

Fix applied: Changed all from_chars error checks from == std::errc::invalid_argument to != std::errc{} to catch all error codes.

AT-006: Inverted from_chars Range on Truncated Battery Response [FIXED]

Severity: Medium | esp_modem_command_library.cpp:230

A malicious modem sending +CBC: 3. (truncated after the dot) causes from_chars(out.data() + dot_pos + 1, out.data() + out.size() - 1, ...) to receive an inverted range (begin > end) — undefined behavior.

Fix applied: Added range validation (dot_pos + 2 > out.size() → return FAIL) before the from_chars call.

AT-007/008/009: API Parameter Concatenation (Developer Responsibility)

Severity: Informational

Parameters like SMS number, PIN, and APN are concatenated into AT command strings. These are developer-supplied API parameters, not modem-originated data. Input validation is the application's responsibility if these values originate from untrusted sources.

AT-011: Signed Shift Overflow in set_network_bands_sim76xx

Severity: Medium | esp_modem_command_library.cpp:554-555

1 << band with band >= 31 is UB. Should be 1ULL << band with bounds checking.

AT-002: int Stores size_t from find() in Multiple Functions

Severity: Low | Lines 188, 444, 220, 222

Signed/unsigned mismatch. Works in practice but is implementation-defined.

AT-010: Pattern Position Assumed to Be 0

Severity: Low | Lines 443, 480, 506, 599

rssi_pos = pattern.size() hardcodes offset 0 instead of using find()'s return value.

AT-012: Comma Iteration Bounded by Early Return

Severity: Informational | esp_modem_command_library.cpp:254-271

Loop is bounded to 3 iterations by early return. No issue.

---

5. URC Handling / Callbacks

Files: src/esp_modem_dte.cpp, include/cxx_include/esp_modem_dte.hpp

URC-001: Cross-Thread Deadlock When URC Handler Calls command()

Severity: Critical | esp_modem_dte.cpp:67, 156

The UART task holds line_lock and calls the URC handler. If the handler calls DTE::command(), it tries to acquire internal_lock (held by the user task), while the user task is blocked in wait_for_line() waiting for the UART task. Classic ABBA deadlock.

Recommendation: Document constraint; add a runtime guard flag; consider deferred-dispatch via queue.

URC-002: Stale last_urc_processed → Unsigned Underflow + OOB Access

Severity: Critical | esp_modem_dte.cpp:170, 488-489

buffer_state.last_urc_processed is never reset when buffer.consumed resets to 0 between commands. create_urc_info() computes new_data_size = (consumed + len) - last_urc_processed which underflows, and new_data_start = data + last_urc_processed points out-of-bounds.

Recommendation: Reset last_urc_processed = 0 alongside buffer.consumed = 0 at line 170.

URC-003: Unbounded consume_size → Underflow + Pointer Overrun

Severity: High | esp_modem_dte.cpp:399-406

User-provided consume_info.consume_size is applied without bounds check. If it exceeds consumed + len, the subtraction underflows and data pointer advances past the buffer.

Recommendation: Validate consume_size <= consumed + len.

URC-004: CONSUME_ALL Silently Swallows Command Responses

Severity: High | esp_modem_dte.cpp:408-412

The enhanced URC handler is called before command processing. CONSUME_ALL discards the buffer without signaling GOT_LINE, causing the command to time out.

Recommendation: Warn/prevent CONSUME_ALL when is_command_active is true.

URC-005: Both Handlers Execute on Same Data Without Mutual Exclusion

Severity: High | esp_modem_dte.cpp:386-421

When both enhanced_urc_handler and urc_handler are registered, they both process the same buffer. After CONSUME_PARTIAL, the legacy handler receives partially-consumed remnants.

Recommendation: Make handlers mutually exclusive with if/else if.

URC-006: Legacy Handler Gets Mutable Buffer Pointer

Severity: Medium | esp_modem_dte.cpp:417

The legacy urc_handler receives a non-const uint8_t*. If it modifies the buffer, subsequent command response parsing operates on corrupted data.

URC-007: CONSUME_PARTIAL Sets len=0, Suppresses Command Processing

Severity: Medium | esp_modem_dte.cpp:405, 429

After partial consumption, len = 0 causes memchr to always return nullptr, skipping command response processing even if the separator is present in the remaining data.

URC-008: total_processed Wraps After ~4 Days at 115200 Baud

Severity: Medium | esp_modem_dte.cpp:479

32-bit size_t counter wraps after ~4 GB. Limited impact since command_start_offset isn't used critically.

URC-009: Legacy Handler Controls Buffer Lifetime for Post-Command Data

Severity: Medium | esp_modem_dte.cpp:416-421

Ambiguous semantics when a command has completed but trailing data arrives.

---

6. VFS Socket-Based Terminal

Files: src/esp_modem_vfs_socket_creator.cpp, src/esp_modem_term_fs.cpp

VFS-002: No TLS / Authentication on TCP Socket

Severity: High | esp_modem_vfs_socket_creator.cpp:44,64

The socket is plaintext TCP. Anyone on the network can eavesdrop, MITM, or impersonate the modem endpoint. All AT commands and PPP data traverse in cleartext.

Recommendation: Wrap in TLS for any non-localhost deployment; prominently document the trust boundary.

VFS-001: File Descriptor Leak on connect() Failure

Severity: Medium | esp_modem_vfs_socket_creator.cpp:64-66

When connect() fails, the socket FD created at line 44 is not closed. Repeated failures exhaust the FD table.

Recommendation: Add close(*fd); before return ESP_FAIL;.

VFS-003: DNS Spoofing — No DNSSEC Validation

Severity: Medium | esp_modem_vfs_socket_creator.cpp:38

getaddrinfo() with no DNS response validation. Combined with VFS-002, enables silent redirect to attacker server.

VFS-004: Arbitrary Host/Port from Configuration (SSRF-like)

Severity: Medium | esp_modem_vfs_socket_creator.cpp:86

If host_name/port are externally settable, an attacker could redirect the modem terminal to any TCP server.

VFS-005: TCP Close Causes Busy-Loop and Silent Failure

Severity: Medium | esp_modem_term_fs.cpp:133-143

read() == 0 (connection closed) is indistinguishable from "no data." The task enters a CPU-burning busy-loop: select() returns immediately, read() returns 0, repeat.

Recommendation: Treat read() == 0 as connection-closed; report terminal_error.

VFS-006: Copy-Paste Error in write() Log Message

Severity: Low | esp_modem_term_fs.cpp:150

Logs "Error occurred during read" on write failure.

VFS-007: Partial Writes Silently Dropped

Severity: Low | esp_modem_term_fs.cpp:146-154

Non-blocking socket may short-write; remaining bytes are silently lost.

VFS-008: sizeof(sockaddr) Mismatch in memcpy/connect

Severity: Low | esp_modem_vfs_socket_creator.cpp:55,64

Should use sizeof(struct sockaddr_in) or ai_addrlen.

VFS-009: IPv6 Not Supported Despite AF_UNSPEC Hint

Severity: Low | esp_modem_vfs_socket_creator.cpp:35,51-61

Only IPv4 handled; IPv6-first DNS results cause connection failure.

VFS-010: 1-Second Select Timeout Timing Differences

Severity: Informational | esp_modem_term_fs.cpp:102-130

Different timing characteristics versus UART could expose timing-sensitive race conditions.

VFS-011: Potential Race on on_read Callback

Severity: Low | esp_modem_term_fs.cpp:55-58, 113-115

(Same as UART-004.) Signal-based coordination is incomplete.

---

Prioritized Remediation Roadmap

Tier 1 — Critical / High (fix immediately)

#	Finding	Status
1	AT-001/003/004/005/006: Response parsing bugs (UB, wrong values, inverted ranges)	FIXED
2	URC-002: Stale last_urc_processed	FIXED
3	URC-003: Unbounded consume_size	FIXED
4	PPP-004: Netif destructor UAF	FIXED
5	CMUX-001: No CRC validation	Medium — enable FCS unconditionally
6	UART-001: Unbounded inflatable buffer	Low — add configurable max size
7	UART-002: Negative read() → SIZE_MAX	Low — check return value
8	PPP-002: Non-PPP data forwarding	Low — make if/else if, validate 0x7E position
9	URC-001: Deadlock hazard	Medium — runtime guard + documentation
10	URC-004/005: Handler interaction bugs	Medium — mutual exclusion
11	VFS-002: No TLS on socket terminal	High — requires TLS integration or documentation

Tier 2 — Medium (fix in next release)

• UART-003/004: Callback data races (add mutex)
• UART-006: DTE destructor UAF (explicit cleanup)
• PPP-001/003/005: Netif validation and race conditions
• AT-011: Signed shift overflow in band configuration
• CMUX-003/004/007/008: Parser hardening
• VFS-001/003/004/005: Socket reliability
• URC-006/007/008/009: Handler improvements

Tier 3 — Low / Informational (backlog)

• All Low/Informational findings — defensive hardening, type corrections, logging fixes

---

Cross-Cutting Architectural Observations

1. Trust boundary is implicit: There is no explicit validation layer between the modem serial link and the parsing/dispatch code. All untrusted input flows directly from terminal read() into parsers. Consider a dedicated input sanitization layer.

2. Callback lifecycle management: Multiple findings (UART-003/004/006, PPP-004) stem from the same pattern: std::function callbacks capture raw this pointers without lifecycle coordination with the terminal task. A centralized callback management pattern (e.g., weak_ptr captures, or explicit stop-before-destroy) would eliminate this class of bugs.

3. CMUX integrity: The decision to make CRC optional fundamentally weakens the protocol's integrity guarantees. Given that CMUX operates over a potentially noisy serial link, CRC should be mandatory.

4. URC subsystem maturity: The enhanced URC handler (CONFIG_ESP_MODEM_URC_HANDLER) has several state-management and interaction bugs (URC-002 through URC-009) suggesting it may benefit from additional test coverage and a clearer contract specification.

---

Note

Medium Risk
Touches core serial parsing and URC/PPP callback lifecycles; while changes are defensive, mistakes could cause regressions in modem response handling or URC consumption behavior.

Overview
Hardens modem handling against malformed/untrusted modem input by tightening AT response parsing and validation in esp_modem_command_library.cpp (safe CR/LF stripping, broader from_chars error checks, bounds checks for battery parsing, and stricter operator name/+CNSMOD parsing).

Fixes enhanced URC state management in esp_modem_dte.cpp by resetting last_urc_processed at command end and validating CONSUME_PARTIAL sizes to avoid underflow/OOB pointer math. Also prevents a PPP Netif destructor use-after-free by clearing the DTE read callback before shutdown in esp_modem_netif.cpp.

^{Written by Cursor Bugbot for commit 0df8c0b. This will update automatically on new commits. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on March 20

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[cursor]

URC state reset races with UART task

High Severity

DTE::command() now writes buffer_state.last_urc_processed = 0 without taking command_cb.line_lock, while the terminal read callback reads/writes buffer_state.* under line_lock. This introduces a cross-thread data race on buffer_state.last_urc_processed (and related fields), which is undefined behavior and can corrupt URC parsing state.

Additional Locations (1)

• components/esp_modem/src/esp_modem_dte.cpp#L381-L498