Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions src/pcm-sensor-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2338,6 +2338,13 @@ std::unordered_map<enum MimeType, std::string, std::hash<int>> 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() {
Expand Down Expand Up @@ -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<size_t>( parsed );
}

double headerValueAsDouble() const {
Expand Down Expand Up @@ -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<size_t>( 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;
}

Expand Down Expand Up @@ -2652,6 +2676,10 @@ class HTTPMessage {
size_t length = static_cast<size_t>( 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<size_t>( 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' );
Expand Down
10 changes: 10 additions & 0 deletions tests/utests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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)
55 changes: 55 additions & 0 deletions tests/utests/pcm-sensor-server-content-length-utest.cpp
Original file line number Diff line number Diff line change
@@ -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 <stdexcept>
#include <string>

// 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 <gtest/gtest.h>

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<size_t>( 1024 ), h.headerValueAsNumber());
}

TEST(PcmSensorServerContentLengthTest, MaximumContentLengthIsAccepted)
{
HTTPHeader const h( "Content-Length", std::to_string( kMaxRequestBodyBytes ) );
EXPECT_EQ(static_cast<size_t>( kMaxRequestBodyBytes ), h.headerValueAsNumber());
}
Loading