diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index b1bb5344c..533c522e3 100644 --- a/lib/http/HttpClient_Curl.hpp +++ b/lib/http/HttpClient_Curl.hpp @@ -484,6 +484,14 @@ class CurlHttpOperation { return poll(&pfd, 1, static_cast(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(-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(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* data) { + // Guard the size * nmemb product against size_t overflow before using it. + if (nmemb != 0 && size > static_cast(-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(ptr); - const auto* end = begin + size * nmemb; + const auto* end = begin + realsize; data->insert( data->end(), begin, end); } return size * nmemb; diff --git a/tests/unittests/HttpClientCurlTests.cpp b/tests/unittests/HttpClientCurlTests.cpp index c9894b90d..d494ba2fc 100644 --- a/tests/unittests/HttpClientCurlTests.cpp +++ b/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 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 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 owned(response); + std::lock_guard lock(m_lock); + m_result = owned->GetResult(); + m_statusCode = owned->GetStatusCode(); + m_bodySize = owned->GetBody().size(); + m_received = true; + } + + bool responseReceived() + { + std::lock_guard lock(m_lock); + return m_received; + } + + void sendAndWait(size_t bodySize) + { + { + std::lock_guard 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(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