Pull request #5183: pub_sub: add content-length validation

Merge in SNORT/snort3 from ~VTRON/snort3:fix_malformed_content_length to master

Squashed commit of the following:

commit 3d6413c713a99b980ed9d91b57483548aeee21ab
Author: vtron <vtron@cisco.com>
Date:   Fri Feb 27 05:57:52 2026 -0800

    pub_sub: add content-length validation

diff --git a/src/pub_sub/http_events.cc b/src/pub_sub/http_events.cc
index 90076bfb1b9d579013ba99af2294fb7cee43f3d9..fc21fc93f7061745641bf392b39985312b5f94c5 100644 (file)
--- a/src/pub_sub/http_events.cc
+++ b/src/pub_sub/http_events.cc
@@ -53,10 +53,63 @@ const uint8_t* HttpEvent::get_all_raw_headers(int32_t& length)
     return get_header(HttpEnums::HTTP_BUFFER_RAW_HEADER, 0, length);
 }
 
-const uint8_t* HttpEvent::get_content_length(int32_t& length)
+// Parse a decimal Content-Length value per RFC 7230 (Content-Length = 1*DIGIT).
+// Comma-separated duplicate values (e.g. "42,42") may arrive from the normalizer when multiple Content-Length headers
+// are present. The first value before the comma is used, consistent with norm_decimal_integer behavior.
+ContentLengthStatus HttpEvent::parse_content_length_value(const char* str, int32_t len, int64_t& value)
 {
-    return get_header(HttpEnums::HTTP_BUFFER_HEADER,
-        HttpEnums::HEAD_CONTENT_LENGTH, length);
+    // 18 significant digits fit in int64_t without overflow (max 999999999999999999)
+    constexpr int max_significant_digits = 18;
+    const char* p = str;
+    const char* const end = str + len;
+
+    // Skip leading zeros separately so the hot digit loop below stays branch-minimal
+    while (p < end and *p == '0')
+        ++p;
+
+    if (p == end or *p == ',')
+    {
+        // No digits consumed at all (e.g. ",42") — reject as malformed
+        if (p == str)
+            return ContentLengthStatus::MALFORMED;
+        value = 0;
+        return ContentLengthStatus::OK;
+    }
+
+    // Pre-compute the scan boundary so the overflow guard is a pointer compare already needed for bounds checking, 
+    // rather than a per-iteration counter
+    const char* const scan_end = (end - p > max_significant_digits) ? p + max_significant_digits : end;
+    int64_t total = 0;
+
+    // Hot loop: unsigned cast turns two-sided range check into a single comparison
+    do
+    {
+        unsigned d = static_cast<unsigned>(*p - '0');
+        if (d > 9u)
+            return ContentLengthStatus::MALFORMED;
+        total = total * 10 + d;
+    }
+    while (++p < scan_end and *p != ',');
+
+    // If we stopped at scan_end with more non-comma chars remaining, too many digits
+    if (p < end and *p != ',')
+        return ContentLengthStatus::MALFORMED;
+
+    value = total;
+    return ContentLengthStatus::OK;
+}
+
+ContentLengthStatus HttpEvent::get_content_length(int64_t& value)
+{
+    value = 0;
+    int32_t hdr_len = 0;
+
+    const char* hdr_value_str = (const char*)get_header(HttpEnums::HTTP_BUFFER_HEADER,
+        HttpEnums::HEAD_CONTENT_LENGTH, hdr_len);
+    if (not hdr_value_str)
+        return ContentLengthStatus::NOT_FOUND;
+
+    return parse_content_length_value(hdr_value_str, hdr_len, value);
 }
 
 const uint8_t* HttpEvent::get_content_type(int32_t& length)

diff --git a/src/pub_sub/http_events.h b/src/pub_sub/http_events.h
index 60942905cfe42ee859c60b39e2205a26b53f8e7b..22bfbf428e8abe21e3aacd12ed472bca81d41418 100644 (file)
--- a/src/pub_sub/http_events.h
+++ b/src/pub_sub/http_events.h
@@ -31,6 +31,8 @@ class HttpMsgHeader;
 namespace snort
 {
 
+enum class ContentLengthStatus { OK, NOT_FOUND, MALFORMED };
+
 class SO_PUBLIC HttpEvent : public snort::DataEvent
 {
 public:
@@ -38,7 +40,7 @@ public:
         http_msg_header(http_msg_header_), is_httpx(httpx), httpx_stream_id(stream_id) { }
 
     const uint8_t* get_all_raw_headers(int32_t &length); // Returns all HTTP headers plus cookies.
-    const uint8_t* get_content_length(int32_t &length);
+    ContentLengthStatus get_content_length(int64_t& value);
     const uint8_t* get_content_type(int32_t &length);
     const uint8_t* get_cookie(int32_t &length);
     const uint8_t* get_authority(int32_t &length);
@@ -66,6 +68,7 @@ private:
     int64_t httpx_stream_id = -1;
 
     const uint8_t* get_header(unsigned, uint64_t, int32_t&);
+    static ContentLengthStatus parse_content_length_value(const char*, int32_t, int64_t&);
 
 };
 }

diff --git a/src/pub_sub/test/pub_sub_http_event_test.cc b/src/pub_sub/test/pub_sub_http_event_test.cc
index 520ab1b385bf163be11c3822f1a727cdb6201310..8c11f24047383940ac42edd618f571ccb6601739 100644 (file)
--- a/src/pub_sub/test/pub_sub_http_event_test.cc
+++ b/src/pub_sub/test/pub_sub_http_event_test.cc
@@ -34,6 +34,9 @@
 #include <CppUTest/TestHarness.h>
 #include <CppUTestExt/MockSupport.h>
 
+#include <string>
+#include <vector>
+
 using namespace snort;
 using namespace HttpCommon;
 
@@ -191,16 +194,88 @@ TEST(pub_sub_http_event_test, get_content_length)
 {
     const char* content_length = "20000";
     const int32_t content_length_len = strlen(content_length);
-    const uint8_t* retrieved_content_length;
-    int32_t retrieved_length;
+    int64_t value = 0;
 
     mock().expectOneCall("get_classic_buffer").withParameter("buffer_type", HttpEnums::HTTP_BUFFER_HEADER);
     Field input(content_length_len, (const uint8_t*) content_length);
     mock().setDataObject("output", "Field", &input);
     HttpEvent event(nullptr, false, 0);
-    retrieved_content_length = event.get_content_length(retrieved_length);
-    CHECK(content_length_len == retrieved_length);
-    CHECK(memcmp(retrieved_content_length, content_length, content_length_len) == 0);
+    ContentLengthStatus status = event.get_content_length(value);
+    CHECK_EQUAL(20000, value);
+    CHECK(status == ContentLengthStatus::OK);
+}
+
+TEST(pub_sub_http_event_test, get_content_length_not_found)
+{
+    int64_t value = 0;
+
+    mock().expectOneCall("get_classic_buffer").withParameter("buffer_type", HttpEnums::HTTP_BUFFER_HEADER);
+    Field input(0, nullptr);
+    mock().setDataObject("output", "Field", &input);
+    HttpEvent event(nullptr, false, 0);
+    ContentLengthStatus status = event.get_content_length(value);
+    CHECK(status == ContentLengthStatus::NOT_FOUND);
+}
+
+void test_bad_content_length_value(const char* content_length_str)
+{
+    const int32_t content_length_len = strlen(content_length_str);
+    int64_t value = 0;
+
+    mock().expectOneCall("get_classic_buffer").withParameter("buffer_type", HttpEnums::HTTP_BUFFER_HEADER);
+    Field input(content_length_len, (const uint8_t*) content_length_str);
+    mock().setDataObject("output", "Field", &input);
+    HttpEvent event(nullptr, false, 0);
+    ContentLengthStatus status = event.get_content_length(value);
+    std::string error_msg = "Content length value should be found to be invalid: " + std::string(content_length_str);
+    CHECK_TEXT(status == ContentLengthStatus::MALFORMED, error_msg.c_str());
+}
+
+void test_valid_content_length(const char* content_length_str, int64_t expected)
+{
+    const int32_t content_length_len = strlen(content_length_str);
+    int64_t value = 0;
+
+    mock().expectOneCall("get_classic_buffer").withParameter("buffer_type", HttpEnums::HTTP_BUFFER_HEADER);
+    Field input(content_length_len, (const uint8_t*) content_length_str);
+    mock().setDataObject("output", "Field", &input);
+    HttpEvent event(nullptr, false, 0);
+    ContentLengthStatus status = event.get_content_length(value);
+    std::string error_msg = "Content length should be valid: " + std::string(content_length_str);
+    CHECK_TEXT(status == ContentLengthStatus::OK, error_msg.c_str());
+    CHECK_EQUAL(expected, value);
+}
+
+TEST(pub_sub_http_event_test, get_content_length_valid_values)
+{
+    test_valid_content_length("0", 0);
+    test_valid_content_length("007", 7);
+    test_valid_content_length("42,42", 42);
+    test_valid_content_length("42,99", 42);
+    test_valid_content_length("0,0", 0);
+    test_valid_content_length("999999999999999999", 999999999999999999LL);
+}
+
+TEST(pub_sub_http_event_test, get_content_length_invalid_values)
+{
+    std::vector<const char*> invalid_content_len_values =
+    {
+        "12345678901234567890123456789012345678901234567890",
+        "9223372036854775807",
+        "9223372036854775808",
+        "00aA",
+        "AC32",
+        "-500",
+        "+123",
+        "12.5",
+        ".",
+        ",42",
+        " ",
+        "\t",
+    };
+
+    for (const char* content_length_str: invalid_content_len_values)
+        test_bad_content_length_value(content_length_str);
 }
 
 int main(int argc, char** argv)