Cap HTTP response body size across WinInet, WinRt, and Apple transports

lib/http/HttpClient_Apple.mm

@@ -15,6 +15,111 @@
 #include "utils/StringUtils.hpp"
 #include "utils/Utils.hpp"
 
+// Streams the response body in bounded chunks and enforces MAX_HTTP_RESPONSE_SIZE.
+// The completionHandler-based NSURLSession APIs fully materialize the response body
+// as an NSData before handing it over, so an attacker-controlled collector could force
+// a large allocation. This delegate instead accumulates data incrementally in
+// didReceiveData: and cancels the transfer as soon as the cap would be exceeded, so no
+// more than the cap is ever buffered. Delegate callbacks may arrive on the session's
+// delegate queue while a request thread registers a task, so shared state is guarded.
+@interface MATStreamingSessionDelegate : NSObject <NSURLSessionDataDelegate>
+- (void)registerTask:(NSURLSessionTask*)task
+             handler:(void (^)(NSData* data, NSURLResponse* response, NSError* error))handler;
+@end
+
+@implementation MATStreamingSessionDelegate {
+    NSMutableDictionary<NSNumber*, NSMutableData*>* _buffers;
+    NSMutableDictionary<NSNumber*, id>*             _handlers;
+    NSMutableSet<NSNumber*>*                        _overCap;
+}
+
+- (instancetype)init
+{
+    self = [super init];
+    if (self)
+    {
+        _buffers = [NSMutableDictionary new];
+        _handlers = [NSMutableDictionary new];
+        _overCap = [NSMutableSet new];
+    }
+    return self;
+}
+
+- (void)registerTask:(NSURLSessionTask*)task
+             handler:(void (^)(NSData*, NSURLResponse*, NSError*))handler
+{
+    NSNumber* key = @(task.taskIdentifier);
+    @synchronized(self)
+    {
+        _buffers[key] = [NSMutableData new];
+        _handlers[key] = [handler copy];
+    }
+}
+
+- (void)URLSession:(NSURLSession*)session
+          dataTask:(NSURLSessionDataTask*)dataTask
+    didReceiveData:(NSData*)data
+{
+    NSNumber* key = @(dataTask.taskIdentifier);
+    @synchronized(self)
+    {
+        if ([_overCap containsObject:key])
+        {
+            return;
+        }
+        NSMutableData* buffer = _buffers[key];
+        if (buffer == nil)
+        {
+            return;
+        }
+        if (buffer.length + data.length > MAT::MAX_HTTP_RESPONSE_SIZE)
+        {
+            // Refuse the over-large response: stop buffering and cancel the transfer.
+            [_overCap addObject:key];
+            [dataTask cancel];
+            return;
+        }
+        [buffer appendData:data];
+    }
+}
+
+- (void)URLSession:(NSURLSession*)session
+              task:(NSURLSessionTask*)task
+didCompleteWithError:(NSError*)error
+{
+    NSNumber* key = @(task.taskIdentifier);
+    void (^handler)(NSData*, NSURLResponse*, NSError*) = nil;
+    NSData* body = nil;
+    BOOL overCap = NO;
+    @synchronized(self)
+    {
+        handler = (void (^)(NSData*, NSURLResponse*, NSError*))_handlers[key];
+        body = _buffers[key];
+        overCap = [_overCap containsObject:key];
+        [_handlers removeObjectForKey:key];
+        [_buffers removeObjectForKey:key];
+        [_overCap removeObject:key];
+    }
+    if (handler == nil)
+    {
+        return;
+    }
+    if (overCap)
+    {
+        // Surface a non-cancellation error so the request maps to NetworkFailure
+        // (retried), not Aborted (which is reserved for caller-initiated cancels).
+        NSError* capError = [NSError errorWithDomain:@"MATResponseCap"
+                                                code:-1
+                                            userInfo:@{ NSLocalizedDescriptionKey : @"HTTP response exceeds max buffered size" }];
+        handler(nil, task.response, capError);
+    }
+    else
+    {
+        handler(body, task.response, error);
+    }
+}
+@end
+
 namespace MAT_NS_BEGIN {
 
 static std::string NextReqId()
@@ -31,6 +136,7 @@
 
 static dispatch_once_t once;
 static NSURLSession* session;
+static MATStreamingSessionDelegate* sessionDelegate;
 
 class HttpRequestApple : public SimpleHttpRequest
 {
@@ -42,7 +148,10 @@
         m_parent->Add(static_cast<IHttpRequest*>(this));
         dispatch_once(&once, ^{
             NSURLSessionConfiguration* sessionConfig = [NSURLSessionConfiguration defaultSessionConfiguration];
-            session = [NSURLSession sessionWithConfiguration:sessionConfig];
+            sessionDelegate = [MATStreamingSessionDelegate new];
+            session = [NSURLSession sessionWithConfiguration:sessionConfig
+                                                    delegate:sessionDelegate
+                                               delegateQueue:nil];
         });
     }
 
@@ -75,15 +184,18 @@ void SendAsync(IHttpResponseCallback* callback)
             if(equalsIgnoreCase(m_method, "get"))
             {
                 [m_urlRequest setHTTPMethod:@"GET"];
-                m_dataTask = [session dataTaskWithRequest:m_urlRequest completionHandler:m_completionMethod];
+                m_dataTask = [session dataTaskWithRequest:m_urlRequest];
             }
             else
             {
                 [m_urlRequest setHTTPMethod:@"POST"];
                 NSData* postData = [NSData dataWithBytes:m_body.data() length:m_body.size()];
-                m_dataTask = [session uploadTaskWithRequest:m_urlRequest fromData:postData completionHandler:m_completionMethod];
+                m_dataTask = [session uploadTaskWithRequest:m_urlRequest fromData:postData];
             }
 
+            // Register before resume so the streaming delegate has the buffer and
+            // completion handler in place before any response data arrives.
+            [sessionDelegate registerTask:m_dataTask handler:m_completionMethod];
             [m_dataTask resume];
         }
     }
@@ -120,10 +232,18 @@ void HandleResponse(NSData* data, NSURLResponse* response, NSError* error)
             }
             else
             {
+                // The streaming delegate has already enforced MAX_HTTP_RESPONSE_SIZE
+                // (an over-cap response arrives here as a cap error, handled above), so
+                // data is bounded. Guard against a nil/empty body to avoid pointer
+                // arithmetic on a null [data bytes].
                 simpleResponse->m_result = HttpResult_OK;
-                auto body = static_cast<const uint8_t*>([data bytes]);
-                simpleResponse->m_body.reserve(data.length);
-                std::copy(body, body + data.length, std::back_inserter(simpleResponse->m_body));
+                const size_t length = static_cast<size_t>(data.length);
+                if (length > 0)
+                {
+                    auto body = static_cast<const uint8_t*>([data bytes]);
+                    simpleResponse->m_body.reserve(length);
+                    std::copy(body, body + length, std::back_inserter(simpleResponse->m_body));
+                }
             }
             m_callback->OnHttpResponse(simpleResponse);
         }

lib/http/HttpClient_WinInet.cpp

@@ -324,27 +324,41 @@ class WinInetRequestWrapper
             // It might potentially be another async operation which will
             // trigger INTERNET_STATUS_REQUEST_COMPLETE again.
 
-            m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed);
-            while (!m_readingData || m_bufferUsed != 0) {
-                BOOL bResult = ::InternetReadFile(m_hWinInetRequest, m_buffer, sizeof(m_buffer), &m_bufferUsed);
-                m_readingData = true;
-                if (!bResult) {
-                    dwError = GetLastError();
-                    if (dwError == ERROR_IO_PENDING) {
-                        // Do not touch anything from this thread anymore.
-                        // The buffer passed to InternetReadFile() and the
-                        // read count will be filled asynchronously, so they
-                        // must stay valid and writable until the next
-                        // INTERNET_STATUS_REQUEST_COMPLETE callback comes
-                        // (that's why those are member variables).
-                        LOG_TRACE("InternetReadFile() failed: ERROR_IO_PENDING. Waiting for INTERNET_STATUS_REQUEST_COMPLETE to be called again");
-                        return;
+            // SECURITY: refuse an over-large response instead of buffering it (see
+            // MAX_HTTP_RESPONSE_SIZE) so a hostile/MITM'd collector cannot exhaust
+            // process memory. Checked before every append so the buffer never exceeds
+            // the cap; reported as an invalid server response -> NetworkFailure (retried).
+            if (m_bodyBuffer.size() + m_bufferUsed > MAX_HTTP_RESPONSE_SIZE) {
+                LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE);
+                dwError = ERROR_HTTP_INVALID_SERVER_RESPONSE;
+            } else {
+                m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed);
+                while (!m_readingData || m_bufferUsed != 0) {
+                    BOOL bResult = ::InternetReadFile(m_hWinInetRequest, m_buffer, sizeof(m_buffer), &m_bufferUsed);
+                    m_readingData = true;
+                    if (!bResult) {
+                        dwError = GetLastError();
+                        if (dwError == ERROR_IO_PENDING) {
+                            // Do not touch anything from this thread anymore.
+                            // The buffer passed to InternetReadFile() and the
+                            // read count will be filled asynchronously, so they
+                            // must stay valid and writable until the next
+                            // INTERNET_STATUS_REQUEST_COMPLETE callback comes
+                            // (that's why those are member variables).
+                            LOG_TRACE("InternetReadFile() failed: ERROR_IO_PENDING. Waiting for INTERNET_STATUS_REQUEST_COMPLETE to be called again");
+                            return;
+                        }
+                        LOG_WARN("InternetReadFile() failed: %d", dwError);
+                        break;
                     }
-                    LOG_WARN("InternetReadFile() failed: %d", dwError);
-                    break;
-                }
 
-                m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed);
+                    if (m_bodyBuffer.size() + m_bufferUsed > MAX_HTTP_RESPONSE_SIZE) {
+                        LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE);
+                        dwError = ERROR_HTTP_INVALID_SERVER_RESPONSE;
+                        break;
+                    }
+                    m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed);
+                }
             }
         }
 

lib/http/HttpClient_WinRt.cpp

@@ -144,7 +144,7 @@ namespace MAT_NS_BEGIN {
 
         void SendHttpAsyncRequest(HttpRequestMessage ^req)
         {
-            IAsyncOperationWithProgress<HttpResponseMessage^, HttpProgress>^ operation = m_parent.getHttpClient()->SendRequestAsync(req, HttpCompletionOption::ResponseContentRead);
+            IAsyncOperationWithProgress<HttpResponseMessage^, HttpProgress>^ operation = m_parent.getHttpClient()->SendRequestAsync(req, HttpCompletionOption::ResponseHeadersRead);
             m_cancellationTokenSource = cancellation_token_source();
 
             create_task(operation, m_cancellationTokenSource.get_token()).
@@ -202,37 +202,98 @@ namespace MAT_NS_BEGIN {
                     index++;
                 }
 
-                auto operation = m_httpResponseMessage->Content->ReadAsBufferAsync();
-                auto task = create_task(operation);
-                if (task.wait() == task_status::completed)
+                // Read content headers before streaming the body.
+                IMapView<String^, String^>^ contentHeadersView = m_httpResponseMessage->Content->Headers->GetView();
+                auto contentHeadersiterator = contentHeadersView->First();
+                unsigned int  contentHeadersIndex = 0;
+                while (contentHeadersIndex < contentHeadersView->Size)
                 {
-                    IMapView<String^, String^>^ contentHeadersView = m_httpResponseMessage->Content->Headers->GetView();
+                    String^ Key = contentHeadersiterator->Current->Key;
+                    String^ Value = contentHeadersiterator->Current->Value;
 
-                    auto contentHeadersiterator = contentHeadersView->First();
-                    unsigned int  contentHeadersIndex = 0;
-                    while (contentHeadersIndex < contentHeadersView->Size)
-                    {
-                        String^ Key = contentHeadersiterator->Current->Key;
-                        String^ Value = contentHeadersiterator->Current->Value;
+                    response->m_headers.add(from_platform_string(Key), from_platform_string(Value));
+                    contentHeadersiterator->MoveNext();
+                    contentHeadersIndex++;
+                }
 
-                        response->m_headers.add(from_platform_string(Key), from_platform_string(Value));
-                        contentHeadersiterator->MoveNext();
-                        contentHeadersIndex++;
+                // SECURITY: stream the body in bounded chunks and enforce
+                // MAX_HTTP_RESPONSE_SIZE. SendRequestAsync uses ResponseHeadersRead, so
+                // the framework does not pre-buffer the whole body; reading it here in
+                // chunks ensures an oversized response is never fully materialized in
+                // memory (a hostile/MITM'd collector cannot exhaust process memory).
+                // task::wait()/get() rethrow if a read faults, so guard the whole stream.
+                try
+                {
+                    IInputStream^ inputStream = nullptr;
+                    {
+                        auto streamOp = m_httpResponseMessage->Content->ReadAsInputStreamAsync();
+                        auto streamTask = create_task(streamOp, m_cancellationTokenSource.get_token());
+                        auto status = streamTask.wait();
+                        if (status == task_status::completed)
+                        {
+                            inputStream = streamTask.get();
+                        }
+                        else
+                        {
+                            // Caller-initiated cancel maps to Aborted; anything else is a failure.
+                            response->m_result = (status == task_status::canceled) ? HttpResult_Aborted : HttpResult_NetworkFailure;
+                        }
                     }
 
-                    auto buffer = task.get();
-                    size_t length = buffer->Length;
-
-                    if (length > 0)
+                    if (inputStream != nullptr)
                     {
-                        response->m_body.reserve(length);
-                        response->m_body.resize(length);
-                        DataReader^ dataReader = DataReader::FromBuffer(buffer);
-                        dataReader->ReadBytes((Platform::ArrayReference<unsigned char>(reinterpret_cast<unsigned char*>(response->m_body.data()), (DWORD)length)));
-                        dataReader->DetachBuffer();
-                        delete dataReader;
+                        const unsigned int chunkSize = 64 * 1024;
+                        for (;;)
+                        {
+                            Buffer^ chunk = ref new Buffer(chunkSize);
+                            auto readOp = inputStream->ReadAsync(chunk, chunkSize, InputStreamOptions::Partial);
+                            auto readTask = create_task(readOp, m_cancellationTokenSource.get_token());
+                            auto status = readTask.wait();
+                            if (status != task_status::completed)
+                            {
+                                // Drop any partial body; caller cancel -> Aborted, else failure.
+                                response->m_result = (status == task_status::canceled) ? HttpResult_Aborted : HttpResult_NetworkFailure;
+                                response->m_body.clear();
+                                break;
+                            }
+
+                            IBuffer^ readBuffer = readTask.get();
+                            unsigned int readLength = (readBuffer != nullptr) ? readBuffer->Length : 0;
+                            if (readLength == 0)
+                            {
+                                break; // end of stream
+                            }
+
+                            if (response->m_body.size() + readLength > MAX_HTTP_RESPONSE_SIZE)
+                            {
+                                LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE);
+                                response->m_result = HttpResult_NetworkFailure;
+                                response->m_body.clear();
+                                break;
+                            }
+
+                            const size_t oldSize = response->m_body.size();
+                            response->m_body.resize(oldSize + readLength);
+                            DataReader^ dataReader = DataReader::FromBuffer(readBuffer);
+                            dataReader->ReadBytes((Platform::ArrayReference<unsigned char>(reinterpret_cast<unsigned char*>(response->m_body.data() + oldSize), readLength)));
+                            dataReader->DetachBuffer();
+                            delete dataReader;
+                        }
+                        delete inputStream;
                     }
                 }
+                catch (Platform::Exception^ ex)
+                {
+                    // A faulted read rethrows here; drop any partial body and fail the request.
+                    LOG_WARN("Reading HTTP response body failed: 0x%08x", ex->HResult);
+                    response->m_result = HttpResult_NetworkFailure;
+                    response->m_body.clear();
+                }
+                catch (...)
+                {
+                    response->m_result = HttpResult_NetworkFailure;
+                    response->m_body.clear();
+                }
             }
             else
             {

lib/include/public/IHttpClient.hpp

@@ -14,11 +14,24 @@
 #include <map>
 #include <string>
 #include <vector>
+#include <cstddef>
 
 ///@cond INTERNAL_DOCS
 namespace MAT_NS_BEGIN
 {
     class ILogConfiguration;
+
+    /// <summary>
+    /// SECURITY: upper bound (in bytes) on an HTTP response body that a transport
+    /// will buffer. OneCollector protocol responses are small (status, kill-switch
+    /// tokens, retry-after, small config), so this generous cap never rejects a
+    /// legitimate response, but it 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). A transport that would exceed it refuses the
+    /// response and reports the request as a network failure so it is retried.
+    /// </summary>
+    static constexpr std::size_t MAX_HTTP_RESPONSE_SIZE = 16u * 1024u * 1024u; // 16 MB
+
     /// <summary>
     /// The HttpHeaders class contains a set of HTTP headers.
     /// </summary>