Mercurial > hg > orthanc
changeset 6656:007e2424375f machine-spirits
fix CWE-770: Memory exhaustion via very large "Content-Length"
| author | Sebastien Jodogne <s.jodogne@gmail.com> |
|---|---|
| date | Fri, 20 Mar 2026 16:17:30 +0100 |
| parents | 6ea250bc6f68 |
| children | db756d9176cc |
| files | NEWS OrthancFramework/Sources/HttpServer/HttpServer.cpp |
| diffstat | 2 files changed, 104 insertions(+), 44 deletions(-) [+] |
line wrap: on
line diff
--- a/NEWS Fri Mar 20 14:54:35 2026 +0100 +++ b/NEWS Fri Mar 20 16:17:30 2026 +0100 @@ -56,6 +56,8 @@ * Save the jobs registry in DB only if it has changed. https://discourse.orthanc-server.org/t/frequent-idle-messages-between-postgres-and-orthanc/6406 * New CMake option: "USE_SYSTEM_MINIZIP" to use the system-wide version of minizip +* Security fixes courtesy of Machine Spirits UG: + - CWE-770: Memory exhaustion via very large "Content-Length" * Upgraded dependencies for static builds: - boost 1.89.0 - dcmtk 3.7.0
--- a/OrthancFramework/Sources/HttpServer/HttpServer.cpp Fri Mar 20 14:54:35 2026 +0100 +++ b/OrthancFramework/Sources/HttpServer/HttpServer.cpp Fri Mar 20 16:17:30 2026 +0100 @@ -185,7 +185,8 @@ PostDataStatus_Success, PostDataStatus_NoLength, PostDataStatus_Pending, - PostDataStatus_Failure + PostDataStatus_Failure, + PostDataStatus_RequestEntityTooLarge // New in Orthanc 1.12.11 }; } @@ -491,57 +492,19 @@ reader.AddChunk(body); reader.CloseStream(); } - - static PostDataStatus ReadBodyWithContentLength(std::string& body, - struct mg_connection *connection, - const std::string& contentLength) - { - size_t length; - try - { - int64_t tmp = boost::lexical_cast<int64_t>(contentLength); - if (tmp < 0) - { - return PostDataStatus_NoLength; - } - - length = static_cast<size_t>(tmp); - } - catch (boost::bad_lexical_cast&) - { - return PostDataStatus_NoLength; - } - body.resize(length); - - size_t pos = 0; - while (length > 0) - { - int r = mg_read(connection, &body[pos], length); - if (r <= 0) - { - return PostDataStatus_Failure; - } - - assert(static_cast<size_t>(r) <= length); - length -= r; - pos += r; - } - - return PostDataStatus_Success; - } - - - static PostDataStatus ReadBodyWithoutContentLength(std::string& body, - struct mg_connection *connection) + static PostDataStatus ReadBodyUsingFile(std::string& body, + struct mg_connection *connection, + size_t maxSize /* "0" means no limit */) { // Store the individual chunks in a temporary file, then read it // back into the memory buffer "body" FileBuffer buffer; + uint64_t readSoFar = 0; std::string tmp(1024 * 1024, 0); - + for (;;) { int r = mg_read(connection, &tmp[0], tmp.size()); @@ -555,6 +518,15 @@ } else { + readSoFar += r; + + if (readSoFar > std::numeric_limits<size_t>::max() || + (maxSize != 0 && + readSoFar > maxSize)) + { + return PostDataStatus_RequestEntityTooLarge; + } + buffer.Append(tmp.c_str(), r); } } @@ -563,6 +535,88 @@ return PostDataStatus_Success; } + + + static PostDataStatus ReadBodyWithContentLength(std::string& body, + struct mg_connection *connection, + const std::string& contentLength) + { + static const size_t MAXIMUM_BODY_SIZE_IN_MEMORY = 10 * 1024 * 1024; // 10MB + + size_t length; + try + { + int64_t tmp = boost::lexical_cast<int64_t>(contentLength); + if (tmp < 0) + { + return PostDataStatus_NoLength; + } + + length = static_cast<size_t>(tmp); + if (static_cast<int64_t>(length) != tmp) + { + return PostDataStatus_Failure; + } + } + catch (boost::bad_lexical_cast&) + { + return PostDataStatus_NoLength; + } + + if (length < MAXIMUM_BODY_SIZE_IN_MEMORY) + { + /** + * Small POST bodies should land into RAM to avoid creating + * temporary files, which would result in bad performance. + **/ + body.resize(length); + + size_t pos = 0; + while (length > 0) + { + int r = mg_read(connection, &body[pos], length); + if (r <= 0) + { + return PostDataStatus_Failure; + } + + assert(static_cast<size_t>(r) <= length); + length -= r; + pos += r; + } + + return PostDataStatus_Success; + } + else + { + /** + * Deal with CWE-770 (Machine Spirits UG). If the client wants + * to send a large body, use a temporary file to prevent memory + * exhaustion by a malicious client that would set a large + * "Content-Length" without sending any actual data. + **/ + + PostDataStatus status = ReadBodyUsingFile(body, connection, length); + + if (status == PostDataStatus_Success) + { + return (body.size() == length ? + PostDataStatus_Success : + PostDataStatus_Failure); + } + else + { + return status; + } + } + } + + + static PostDataStatus ReadBodyWithoutContentLength(std::string& body, + struct mg_connection *connection) + { + return ReadBodyUsingFile(body, connection, 0 /* TODO - no bound */); + } static PostDataStatus ReadBodyToString(std::string& body, @@ -1492,6 +1546,10 @@ output.SendStatus(HttpStatus_411_LengthRequired); return; + case PostDataStatus_RequestEntityTooLarge: // New in Orthanc 1.12.11 + output.SendStatus(HttpStatus_413_RequestEntityTooLarge); + return; + case PostDataStatus_Failure: output.SendStatus(HttpStatus_400_BadRequest); return;
