diff --git a/lib/http/HttpClient_Apple.mm b/lib/http/HttpClient_Apple.mm index 449b4c9af..b7d6646a4 100644 --- a/lib/http/HttpClient_Apple.mm +++ b/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 +- (void)registerTask:(NSURLSessionTask*)task + handler:(void (^)(NSData* data, NSURLResponse* response, NSError* error))handler; +@end + +@implementation MATStreamingSessionDelegate { + NSMutableDictionary* _buffers; + NSMutableDictionary* _handlers; + NSMutableSet* _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(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([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(data.length); + if (length > 0) + { + auto body = static_cast([data bytes]); + simpleResponse->m_body.reserve(length); + std::copy(body, body + length, std::back_inserter(simpleResponse->m_body)); + } } m_callback->OnHttpResponse(simpleResponse); } diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index eaefb2318..b1d3b4013 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/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); + } } } diff --git a/lib/http/HttpClient_WinRt.cpp b/lib/http/HttpClient_WinRt.cpp index e46e4c49c..1efc1bb22 100644 --- a/lib/http/HttpClient_WinRt.cpp +++ b/lib/http/HttpClient_WinRt.cpp @@ -144,7 +144,7 @@ namespace MAT_NS_BEGIN { void SendHttpAsyncRequest(HttpRequestMessage ^req) { - IAsyncOperationWithProgress^ operation = m_parent.getHttpClient()->SendRequestAsync(req, HttpCompletionOption::ResponseContentRead); + IAsyncOperationWithProgress^ 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^ contentHeadersView = m_httpResponseMessage->Content->Headers->GetView(); + auto contentHeadersiterator = contentHeadersView->First(); + unsigned int contentHeadersIndex = 0; + while (contentHeadersIndex < contentHeadersView->Size) { - IMapView^ 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(reinterpret_cast(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(reinterpret_cast(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 { diff --git a/lib/include/public/IHttpClient.hpp b/lib/include/public/IHttpClient.hpp index 89e5e6cf0..7a8678ceb 100644 --- a/lib/include/public/IHttpClient.hpp +++ b/lib/include/public/IHttpClient.hpp @@ -14,11 +14,24 @@ #include #include #include +#include ///@cond INTERNAL_DOCS namespace MAT_NS_BEGIN { class ILogConfiguration; + + /// + /// 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. + /// + static constexpr std::size_t MAX_HTTP_RESPONSE_SIZE = 16u * 1024u * 1024u; // 16 MB + /// /// The HttpHeaders class contains a set of HTTP headers. ///