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

lib/http/HttpClient_Curl.hpp

@@ -484,6 +484,14 @@ class CurlHttpOperation {
         return poll(&pfd, 1, static_cast<int>(timeout));
     }
 
+    // SECURITY: upper bound on the collector response the client will buffer. The
+    // OneCollector protocol responses (status, kill-switch tokens, retry-after, small
+    // config) are tiny, so this generous cap never rejects a legitimate response but
+    // stops a hostile or MITM'd collector from driving unbounded memory growth by
+    // returning an oversized body (a memory-amplification DoS of the embedding process).
+    // Exceeding it aborts the transfer, so the upload is treated as failed and retried.
+    static constexpr size_t kMaxResponseBytes = 16 * 1024 * 1024; // 16 MB
+
     // Raw response buffer
     struct MemoryStruct {
       char *memory;
@@ -501,9 +509,21 @@ class CurlHttpOperation {
      */
     static size_t WriteMemoryCallback(void *contents, size_t size, size_t nmemb, void *userp)
     {
+        // Guard the size * nmemb product against size_t overflow before using it.
+        if (nmemb != 0 && size > static_cast<size_t>(-1) / nmemb) {
+            return 0;
+        }
         size_t realsize = size * nmemb;
         struct MemoryStruct *mem = (struct MemoryStruct *)userp;
 
+        // SECURITY: bound the buffered response (see kMaxResponseBytes). Compare
+        // overflow-safely (mem->size is always <= kMaxResponseBytes here). Returning a
+        // short count aborts the transfer with CURLE_WRITE_ERROR.
+        if (realsize > kMaxResponseBytes - mem->size) {
+            TRACE("Response exceeds max buffered size (%zu bytes); aborting transfer\n", kMaxResponseBytes);
+            return 0;
+        }
+
         auto* memory = static_cast<char*>(realloc(mem->memory, mem->size + realsize + 1));
         if(memory == nullptr) {
           /* out of memory! */
@@ -533,9 +553,21 @@ class CurlHttpOperation {
      */
     static size_t WriteVectorCallback(void *ptr, size_t size, size_t nmemb, std::vector<uint8_t>* data)
     {
+        // Guard the size * nmemb product against size_t overflow before using it.
+        if (nmemb != 0 && size > static_cast<size_t>(-1) / nmemb) {
+            return 0;
+        }
         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 + size * nmemb;
+            const auto* end   = begin + realsize;
             data->insert( data->end(), begin, end);
         }
         return size * nmemb;

tests/unittests/HttpClientCurlTests.cpp

@@ -9,6 +9,7 @@
     && !defined(__APPLE__) && !defined(ANDROID)
 
 #include "common/Common.hpp"
+#include "common/HttpServer.hpp"
 #include "http/HttpClient_Curl.hpp"
 #include "config/RuntimeConfig_Default.hpp"
 
@@ -126,4 +127,118 @@ TEST_F(HttpClientCurlTests, SetSslVerification_ConcurrentCallsNoRace)
     SUCCEED();
 }
 
+// --- Response-size cap (memory-amplification DoS hardening) ---
+
+class HttpClientCurlResponseCapTests : public ::testing::Test,
+                                       public HttpServer::Callback,
+                                       public IHttpResponseCallback
+{
+protected:
+    HttpServer      m_server;
+    HttpClient_Curl m_client;
+    // The client never takes ownership of the request (it only stores a raw pointer
+    // and erases it); the fixture owns it and frees it in TearDown -- on the main
+    // thread, after the transfer has completed. Freeing it inside OnHttpResponse
+    // would destroy the CurlHttpOperation from within its own async task, whose
+    // destructor waits on that task (a self-join deadlock).
+    std::unique_ptr<IHttpRequest> m_request;
+    std::string     m_hostname;
+    size_t          m_responseBodySize {0};
+
+    std::mutex      m_lock;
+    bool            m_received {false};
+    HttpResult      m_result {};
+    unsigned int    m_statusCode {0};
+    size_t          m_bodySize {0};
+
+    void SetUp() override
+    {
+        int port = m_server.addListeningPort(0);
+        std::ostringstream os;
+        os << "127.0.0.1:" << port;
+        m_hostname = os.str();
+        m_server.setServerName(m_hostname);
+        m_server.addHandler("/huge/", *this);
+        m_server.start();
+    }
+
+    void TearDown() override
+    {
+        m_server.stop();
+        m_request.reset();
+    }
+
+    // HttpServer::Callback -- returns a body of m_responseBodySize bytes.
+    int onHttpRequest(HttpServer::Request const& /*request*/, HttpServer::Response& response) override
+    {
+        size_t bodySize;
+        {
+            std::lock_guard<std::mutex> lock(m_lock);
+            bodySize = m_responseBodySize;
+        }
+        response.headers["Content-Type"] = "application/octet-stream";
+        response.content = std::string(bodySize, 'A');
+        return 200;
+    }
+
+    // IHttpResponseCallback -- the SDK hands over ownership of the response.
+    void OnHttpResponse(IHttpResponse* response) override
+    {
+        std::unique_ptr<IHttpResponse> owned(response);
+        std::lock_guard<std::mutex> lock(m_lock);
+        m_result = owned->GetResult();
+        m_statusCode = owned->GetStatusCode();
+        m_bodySize = owned->GetBody().size();
+        m_received = true;
+    }
+
+    bool responseReceived()
+    {
+        std::lock_guard<std::mutex> lock(m_lock);
+        return m_received;
+    }
+
+    void sendAndWait(size_t bodySize)
+    {
+        {
+            std::lock_guard<std::mutex> lock(m_lock);
+            m_received = false;
+            m_result = HttpResult{};
+            m_statusCode = 0;
+            m_bodySize = 0;
+            m_responseBodySize = bodySize; // read under the same lock by onHttpRequest
+        }
+        m_request.reset(m_client.CreateRequest());
+        m_request->SetUrl("http://" + m_hostname + "/huge/");
+        m_client.SendRequestAsync(m_request.get(), this);
+        for (int i = 0; i < 300 && !responseReceived(); i++)
+            PAL::sleep(100);
+    }
+};
+
+TEST_F(HttpClientCurlResponseCapTests, AbortsOversizedResponseBody)
+{
+    // A response body larger than the client's response-size cap (kMaxResponseBytes,
+    // 16 MB) must be refused, not buffered in full, so a hostile/MITM'd collector
+    // cannot exhaust process memory.
+    sendAndWait(17u * 1024u * 1024u);
+    ASSERT_TRUE(responseReceived());
+    // curl aborts the transfer (CURLE_WRITE_ERROR) once the cap is hit -> NetworkFailure.
+    EXPECT_EQ(m_result, HttpResult_NetworkFailure);
+    // The oversized body is never fully buffered.
+    EXPECT_LE(m_bodySize, static_cast<size_t>(16u * 1024u * 1024u));
+}
+
+TEST_F(HttpClientCurlResponseCapTests, AcceptsLargeResponseUnderCap)
+{
+    // A large-but-legitimate response (well under the cap) must still be received
+    // in full: the cap must not regress normal responses.
+    const size_t bodySize = 4u * 1024u * 1024u;
+    sendAndWait(bodySize);
+    ASSERT_TRUE(responseReceived());
+    EXPECT_EQ(m_result, HttpResult_OK);
+    EXPECT_EQ(m_statusCode, 200u);
+    EXPECT_EQ(m_bodySize, bodySize);
+}
+
 #endif // MATSDK_PAL_CPP11 && !_MSC_VER && HAVE_MAT_DEFAULT_HTTP_CLIENT