curl · Report #3434510
Out-of-bounds read in HTTP method handling causes undefined behavior and potential crash This is sharp, Gaurav. We’ve got a real memory-safety bug ins
How it was found
Summary - Component: libcurl core HTTP handling (HTTP/2 request translation and CONNECT detection)
- Type: out-of-bounds read resulting from missing null-termination
- Impact: Behavior not defined by the specification, the program can crash (DoS) and CONNECT requests can be misclassified
- Root cause: The method string was copied without a null termin
Affected code paths
-
Struct layout:
-
struct httpreq { ... char method[1]; }
-
Allocation and copy (no NUL):
-
req = calloc(1, sizeof(*req) + m_len);
-
memcpy(req->method, method, m_len);
-
Unsafe uses:
-
strcmp("CONNECT", req->method) — assumes null-terminated string
-
strlen(req->method) — sizes HTTP/2 pseudo-header :method
Direct risk: Both strcmp and strlen may access memory beyond what has been allocated if req->method[m_len] is out-of-bounds. The impact varies with the allocator/layout; ASan will always detect this.
Steps to reproduce (concise)
- Prereqs: make sure CMake and nghttp2 are installed
- What version outputs are you having:
cmake --version
→ 3.26.2
pkg-config --modversion libnghttp2
→ 1.52.0
Build curl with ASan + HTTP/2:
mkdir -p build && cmake -B build -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_FLAGS='-O1 -g -fsanitize=address' -DCMAKE_EXE_LINKER_FLAGS='-fsanitize=address' -DCURL_USE_NGHTTP2=ON
cmake --build build -j"$(nproc)"
Verify binary and features:
./src/curl --version
{F5027206}
Impact
Impact
- Denial of Service/crash: Out of bounds read occurring during the creation of HTTP/2 pseudo-headers or checking CONNECT logic. - Logic flaw: The function for detecting CONNECT might behave differently due to a comparison of an uninitialized value being performed.
- Trigger surface: Any path which produces a struct httpreq (e.g., custom methods) and then executes HTTP/2 translation or CONNECT checks
Proposed repair (minimal and robust)
- In both creators, not only add a null terminator but also allocate memory for it:
- req = calloc(1, sizeof(*req) + m_len);
+ req = calloc(1, sizeof(*req) + m_len + 1);
if(!req) goto out;
memcpy(req->method, method, m_len);
+ req->method[m_len] = '\0';
- Locations to change: Curl_http_req_make Curl_http_req_make2 This change removes local undefined reads without changing the logic. Optional hardening: replace strcmp("CONNECT",