Null dereference in _parseReqHeader() when the Accept header copy fails to allocate (crash instead of rejecting the request) #500

Summary

When the heap is nearly exhausted, AsyncWebServerRequest::_parseReqHeader() copies the Accept header value into a String and passes c_str() straight to std::strstr(). If that copy cannot allocate, Arduino-ESP32's String becomes invalid and c_str() returns nullptr, so strstr(NULL, ...) faults in the async_tcp task and the device panics. The request should be rejected (or the header skipped) instead.

Where

src/WebRequest.cpp, _parseReqHeader(), the T_ACCEPT branch (lines 670-673 in 3.12.1):

} else if (name.equalsIgnoreCase(T_ACCEPT)) {
  String lowcase(value);
  lowcase.toLowerCase();
  const char *substr = std::strstr(lowcase.c_str(), T_text_event_stream);   // line 673

Why it crashes

In Arduino-ESP32 3.3.12, String(const String&) goes through String::copy(), which on a failed reserve() calls invalidate() -> init(), leaving buffer() == nullptr. String::c_str() is simply return buffer();, so it returns nullptr after an allocation failure. The copy only needs the heap when the value does not fit the inline (SSO) buffer; a browser Accept header such as text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8 does not fit, while a short one (*/*) does.

Observed crash

A request from a desktop browser while the free heap was ~1 KB. exc_cause 28 (LoadProhibited), exc_vaddr 0x00000000, task async_tcp. Decoded backtrace:

strstr                                   (newlib strstr.c:79)
AsyncWebServerRequest::_parseReqHeader() (WebRequest.cpp:673)
AsyncWebServerRequest::_parseLine()      (WebRequest.cpp:1027)
AsyncWebServerRequest::_onData()         (WebRequest.cpp:167)
 <lambda in AsyncWebServerRequest ctor>
AsyncClient::_recv()                     (AsyncTCP.cpp:1093)
AsyncTCP_detail::handle_async_event()    (AsyncTCP.cpp:304)
_async_service_task()                    (AsyncTCP.cpp:340)

The heap was exhausted by many concurrent connections plus other allocations on the device; that part is application-side and not what this report is about. What this report is about is that the library turns a recoverable allocation failure into a crash at this line.

Reproduction (host-level idea)

Make the allocator used by String fail (or run on a device with the heap filled until only a few hundred bytes remain), then send any request with an Accept header longer than the SSO buffer:

GET / HTTP/1.1
Host: x
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8

Related

The T_Transfer_Encoding branch just below does the same String lowcase(value) copy. After a failed copy lowcase.length() is 0, so it does not crash, but it silently skips parsing Transfer-Encoding, i.e. a chunked request would be handled as if it were not chunked. The same failure should reject the request there too.

Suggested fix

Treat a failed String allocation while parsing headers as a request error (close the connection, or answer 500/431) instead of continuing with an invalid string. A minimal local guard for this line would be:

const char *lc = lowcase.c_str();
const char *substr = lc ? std::strstr(lc, T_text_event_stream) : NULL;

but the fuller fix is to detect the failure once (e.g. if (value.length() && !lowcase.length()) after the copy, or avoid the copy with a case-insensitive search such as strcasestr(value.c_str(), T_text_event_stream) and check value.c_str() for null) and abort the request cleanly.

Local workaround and result

We carry this guard in a local copy of 3.12.1, after each String lowcase(value); in the Accept and Transfer-Encoding branches, using the library's own failure pattern:

if (value.length() && !lowcase.length()) {
  _parseState = PARSE_REQ_FAIL;
  abort();
  return true;
}

On the same ESP32-D0WD under the same load (many concurrent connections, heap down to ~1 KB), the unpatched build crashed here after 8.5 h; the patched builds ran about 22 h in total without a crash. The heap was exhausted repeatedly during that time (allocation failures in the WiFi driver and tcp_write were handled normally). We could not force the out-of-memory case on demand, so the guard itself has not been exercised under a deterministic failure.

Happy to test a patch on hardware.