Cap curl HTTP response body size to prevent memory-amplification DoS

[bmehta001]

Summary

The libcurl HTTP transport buffered the entire collector response with no
upper bound. Both response-buffering callbacks grew until the whole body was
received:

• WriteMemoryCallback — realloc(mem->memory, mem->size + realsize + 1) then append (raw-response path)
• WriteVectorCallback — data->insert(data->end(), begin, end) (normal path)

A hostile or MITM'd collector (reachable e.g. via a redirected/attacker-controlled
SetCollectorUri) could return an arbitrarily large body and drive unbounded
memory growth in the embedding process — a memory-amplification denial of
service. Legitimate OneCollector responses are tiny (status line, kill-switch
tokens, Retry-After, small config), so a generous fixed cap never rejects a
real response.

Change

• Add a 16 MB cap kMaxResponseBytes and enforce it overflow-safely
  (realsize > kMaxResponseBytes - buffered) in both curl write callbacks.
• Exceeding the cap returns a short byte count, which makes libcurl abort the
  transfer with CURLE_WRITE_ERROR. The SDK already maps that to
  HttpResult_NetworkFailure, so the upload is treated as a transient network
  failure and retried — no crash, no partial-response corruption.

Tests

New HttpClientCurlResponseCapTests (activates on the curl path: Linux,
non-Apple, non-Android) drive a real in-process HttpServer:

• AbortsOversizedResponseBody — a 17 MB body is refused: result is
  NetworkFailure and the buffered body never exceeds the cap.
• AcceptsLargeResponseUnderCap — a 4 MB body is still received in full
  (HttpResult_OK, status 200, exact size), proving the cap does not regress
  normal responses.

Validation (WSL, GCC 13, -std=c++11, Debug):

• UnitTests build clean.
• HttpClientCurl*Tests.* — 13/13 pass (incl. the 2 new).
• Http* / *Curl* suites — 38/38 pass (no regression).

Scope / follow-ups

• This caps the libcurl transport, which is the primary threat-model
  transport on Linux/Android/POSIX. The unbounded-buffering pattern also exists
  per-transport in WinInet (m_bodyBuffer), WinRt, and Apple NSURLSession;
  capping those is a reasonable follow-up for full coverage.
• The cap is a conservative fixed constant. Making it configurable via
  ILogConfiguration (e.g. a CFG_INT_HTTP_MAX_RESPONSE_SIZE key) could be
  added later if any consumer legitimately needs a larger ceiling.

[Copilot]

Pull request overview

This PR hardens the libcurl HTTP transport against memory-amplification DoS by enforcing a fixed upper bound on buffered HTTP response bodies, and adds unit tests that validate both aborting oversized responses and accepting large responses under the cap.

Changes:

• Introduces a 16 MB response-body buffering cap (kMaxResponseBytes) in libcurl write callbacks.
• Aborts transfers that exceed the cap so oversized responses are treated as network failures.
• Adds new in-process HttpServer-backed unit tests validating both oversized and under-cap responses on the curl path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
tests/unittests/HttpClientCurlTests.cpp	Adds new curl-only tests using an in-process HTTP server to validate response-body cap behavior.
lib/http/HttpClient_Curl.hpp	Adds a 16 MB buffered-response cap and enforces it in both curl write callbacks.

Comments suppressed due to low confidence (1)

lib/http/HttpClient_Curl.hpp:566

• WriteVectorCallback (1) calculates realsize = size * nmemb without checking for multiplication overflow and (2) returns size * nmemb at the end instead of returning the already-computed realsize. Both can lead to incorrect return values (and potentially bypass/disable the intended cap logic if overflow occurs), and also recompute the potentially-overflowing expression.

    static size_t WriteVectorCallback(void *ptr, size_t size, size_t nmemb, std::vector<uint8_t>* data)
    {
        if (data != nullptr) {
            size_t realsize = size * nmemb;
            // SECURITY: bound the buffered response (see kMaxResponseBytes). Compare
            // overflow-safely (data->size() is always <= kMaxResponseBytes here).
            // Returning a short count aborts the transfer with CURLE_WRITE_ERROR.
            if (realsize > kMaxResponseBytes - data->size()) {
                TRACE("Response exceeds max buffered size (%zu bytes); aborting transfer\n", kMaxResponseBytes);
                return 0;
            }
            const auto* begin = static_cast<const uint8_t*>(ptr);
            const auto* end   = begin + realsize;
            data->insert( data->end(), begin, end);
        }
        return size * nmemb;
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.