diff --git a/src/pcm-sensor-server.cpp b/src/pcm-sensor-server.cpp index f34c9025..a17289f4 100644 --- a/src/pcm-sensor-server.cpp +++ b/src/pcm-sensor-server.cpp @@ -2338,6 +2338,13 @@ std::unordered_map> mimeTypeMap = { { ApplicationJSON, "application/json" } }; +// Upper bound on the request body size the server is willing to accept and +// buffer in memory. Content-Length values above this are rejected before any +// allocation happens. This caps the attacker-controlled allocation length +// (CWE-190/CWE-400) so an unauthenticated client cannot drive unbounded +// memory growth via large or negative Content-Length headers. +static constexpr long long kMaxRequestBodyBytes = 16 * 1024 * 1024; // 16 MiB + class HTTPHeader { public: HTTPHeader() { @@ -2445,8 +2452,16 @@ class HTTPHeader { } size_t headerValueAsNumber() const { - size_t number = std::stoll( value_ ); - return number; + // Parse as signed so we can detect negative values (which would + // otherwise wrap to a huge size_t) and reject anything outside the + // accepted request-body bounds before it is used as an allocation + // length. See kMaxRequestBodyBytes (CWE-190/CWE-400). + long long parsed = std::stoll( value_ ); + if ( parsed < 0 ) + throw std::runtime_error( "Negative header value not allowed" ); + if ( parsed > kMaxRequestBodyBytes ) + throw std::runtime_error( "Header value exceeds maximum allowed size" ); + return static_cast( parsed ); } double headerValueAsDouble() const { @@ -2616,8 +2631,17 @@ class HTTPMessage { static constexpr unsigned long long MAX_CHUNK_BYTES = 64ULL * 1024ULL * 1024ULL; std::string readData( socketstream& in, size_t length ) { + // Defense-in-depth: never allocate a body buffer larger than the + // configured maximum, even if a caller bypasses headerValueAsNumber() + // validation (CWE-190/CWE-400). + if ( length > static_cast( kMaxRequestBodyBytes ) ) + throw std::runtime_error( "Request body exceeds maximum allowed size" ); std::string data( length, '\0' ); - in.read( &data[0], length ); + // Avoid indexing data[0] when length is zero: for an empty string this + // is undefined behavior and can be triggered by a Content-Length: 0 + // request (or optional-body methods). + if ( length > 0 ) + in.read( &data[0], length ); return data; } @@ -2652,6 +2676,10 @@ class HTTPMessage { size_t length = static_cast( parsedLength ); if ( length == 0 ) break; + // Bound the total accumulated body so that many chunks cannot be + // used to grow the buffer without limit (CWE-400). + if ( data.size() + length > static_cast( kMaxRequestBodyBytes ) ) + throw std::runtime_error( "Request body exceeds maximum allowed size" ); DBG( 3, "length: '", length, "'" ); // Initialize chunk to all zeros std::string chunk( length, '\0' ); diff --git a/tests/utests/CMakeLists.txt b/tests/utests/CMakeLists.txt index aa394920..5182f5d3 100644 --- a/tests/utests/CMakeLists.txt +++ b/tests/utests/CMakeLists.txt @@ -16,6 +16,7 @@ file(GLOB READ_NUMBER_TEST_FILES read-number-utest.cpp) file(GLOB EVENT_RESOLVER_TEST_FILES event-resolver-utest.cpp ${CMAKE_SOURCE_DIR}/src/event-resolver.cpp) file(GLOB PCM_IO_METRICS_TEST_FILES pcm-io-metrics-utest.cpp ${CMAKE_SOURCE_DIR}/src/pcm-io-metrics.cpp) file(GLOB PCM_SENSOR_SERVER_OVERFLOW_TEST_FILES pcm-sensor-server-overflow-utest.cpp) +file(GLOB PCM_SENSOR_SERVER_CONTENT_LENGTH_TEST_FILES pcm-sensor-server-content-length-utest.cpp) file(GLOB PCM_SENSOR_SERVER_HEADER_LIMITS_TEST_FILES pcm-sensor-server-header-limits-utest.cpp) set(LIBS Threads::Threads PCM_STATIC) @@ -26,6 +27,7 @@ add_executable(read-number-utest ${READ_NUMBER_TEST_FILES}) add_executable(event-resolver-utest ${EVENT_RESOLVER_TEST_FILES}) add_executable(pcm-io-metrics-utest ${PCM_IO_METRICS_TEST_FILES}) add_executable(pcm-sensor-server-overflow-utest ${PCM_SENSOR_SERVER_OVERFLOW_TEST_FILES}) +add_executable(pcm-sensor-server-content-length-utest ${PCM_SENSOR_SERVER_CONTENT_LENGTH_TEST_FILES}) add_executable(pcm-sensor-server-header-limits-utest ${PCM_SENSOR_SERVER_HEADER_LIMITS_TEST_FILES}) configure_file( @@ -88,6 +90,13 @@ target_link_libraries( ${LIBS} ) +target_link_libraries( + pcm-sensor-server-content-length-utest + GTest::gtest_main + GTest::gmock_main + ${LIBS} +) + target_link_libraries( pcm-sensor-server-header-limits-utest GTest::gtest_main @@ -102,4 +111,5 @@ gtest_discover_tests(read-number-utest) gtest_discover_tests(event-resolver-utest) gtest_discover_tests(pcm-io-metrics-utest) gtest_discover_tests(pcm-sensor-server-overflow-utest) +gtest_discover_tests(pcm-sensor-server-content-length-utest) gtest_discover_tests(pcm-sensor-server-header-limits-utest) diff --git a/tests/utests/pcm-sensor-server-content-length-utest.cpp b/tests/utests/pcm-sensor-server-content-length-utest.cpp new file mode 100644 index 00000000..eab6bae1 --- /dev/null +++ b/tests/utests/pcm-sensor-server-content-length-utest.cpp @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2009-2025, Intel Corporation + +// Regression test for the Content-Length integer-conversion / unbounded +// allocation issue in src/pcm-sensor-server.cpp. +// +// HTTPHeader::headerValueAsNumber() used to parse the Content-Length value +// with std::stoll() and return it directly as a size_t. A negative value such +// as "-1" therefore wrapped to SIZE_MAX, and arbitrarily large positive values +// were accepted as attacker-controlled allocation lengths (CWE-190/CWE-400). +// +// The fixed implementation rejects negative values and values above +// kMaxRequestBodyBytes before converting to size_t. This test exercises that +// helper directly and verifies: +// 1. "-1" is rejected instead of wrapping to a huge size_t, +// 2. an oversized positive Content-Length is rejected, +// 3. a valid, in-bounds value is accepted and returned unchanged. + +#include +#include + +// Pull the real HTTPHeader implementation out of pcm-sensor-server.cpp without +// bringing in its main(). The same mechanism is already used by +// tests/pcm-sensor-server-fuzz.cpp and the overflow unit test. +#define UNIT_TEST 1 +#include "../../src/pcm-sensor-server.cpp" +#undef UNIT_TEST + +#include + +TEST(PcmSensorServerContentLengthTest, NegativeContentLengthIsRejected) +{ + HTTPHeader const h( "Content-Length", "-1" ); + // Must throw rather than wrapping -1 to SIZE_MAX. + EXPECT_THROW(h.headerValueAsNumber(), std::runtime_error); +} + +TEST(PcmSensorServerContentLengthTest, OversizedContentLengthIsRejected) +{ + long long const tooLarge = kMaxRequestBodyBytes + 1; + HTTPHeader const h( "Content-Length", std::to_string( tooLarge ) ); + EXPECT_THROW(h.headerValueAsNumber(), std::runtime_error); +} + +TEST(PcmSensorServerContentLengthTest, ValidContentLengthIsAccepted) +{ + HTTPHeader const h( "Content-Length", "1024" ); + EXPECT_EQ(static_cast( 1024 ), h.headerValueAsNumber()); +} + +TEST(PcmSensorServerContentLengthTest, MaximumContentLengthIsAccepted) +{ + HTTPHeader const h( "Content-Length", std::to_string( kMaxRequestBodyBytes ) ); + EXPECT_EQ(static_cast( kMaxRequestBodyBytes ), h.headerValueAsNumber()); +}