From: Jose Cano -X (jcanogom - SOFTSERVE INC at Cisco) <jcanogom@cisco.com>
Date: Tue, 1 Apr 2025 13:25:31 +0000 (+0000)
Subject: Pull request #4659: http2_inspect: builtin rule for large settings max frame size
X-Git-Tag: 3.7.3.0~13
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=95863487f1669e6141f76adae846fac0c926de82;p=thirdparty%2Fsnort3.git

Pull request #4659: http2_inspect: builtin rule for large settings max frame size

Merge in SNORT/snort3 from ~JCANOGOM/snort3:http2_rule_large_settings_max_frame_size to master

Squashed commit of the following:

commit c0a3a471ecdc029bee8984bed2e38edea6e00531
Author: Jose Cano <jcanogom@cisco.com>
Date:   Tue Mar 11 11:52:25 2025 -0400

    http2_inspect: added settings_max_frame_size parameter and built-in rule 121:44 to check for max frame size
---

diff --git a/doc/reference/builtin_stubs.txt b/doc/reference/builtin_stubs.txt
index c6de43164..eb8f21cd3 100644
--- a/doc/reference/builtin_stubs.txt
+++ b/doc/reference/builtin_stubs.txt
@@ -1481,6 +1481,11 @@ More than 6 unacknowledged settings frames.
 
 Unexpected settings ACK.
 
+121:44
+
+SETTINGS_MAX_FRAME_SIZE value sent in HTTP/2 settings frame is greater than maximum value,
+as configured by settings_max_frame_size.
+
 122:1
 
 Basic one host to one host TCP portscan where multiple TCP ports are scanned on
diff --git a/doc/user/http2_inspect.txt b/doc/user/http2_inspect.txt
index f8685a676..54218eed8 100644
--- a/doc/user/http2_inspect.txt
+++ b/doc/user/http2_inspect.txt
@@ -28,6 +28,10 @@ This limits the maximum number of HTTP/2 streams Snort will process concurrently
 flow. The default and minimum configurable value is 100. It can be configured up to a maximum of
 1000.
 
+===== settings_max_frame_size
+This sets the maximum allowed value for settings frame SETTINGS_MAX_FRAME_SIZE.
+The default and max value is 16777215. The minimum configurable value is 16384.
+
 ==== Detection rules
 
 Since HTTP/2 traffic is processed through the HTTP inspector, all of the rule options discussed
diff --git a/src/service_inspectors/http2_inspect/http2_data_frame.h b/src/service_inspectors/http2_inspect/http2_data_frame.h
index f33bb71c3..5cd9df525 100644
--- a/src/service_inspectors/http2_inspect/http2_data_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_data_frame.h
@@ -38,7 +38,7 @@ public:
     virtual const uint8_t* get_frame_data(uint32_t& length) const override;
 
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
-        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, const Http2ParaList* params, Http2Stream* stream);
 
 #ifdef REG_TEST
     void print_frame(FILE* output) override;
diff --git a/src/service_inspectors/http2_inspect/http2_enum.h b/src/service_inspectors/http2_inspect/http2_enum.h
index a18239281..c42bfafef 100644
--- a/src/service_inspectors/http2_inspect/http2_enum.h
+++ b/src/service_inspectors/http2_inspect/http2_enum.h
@@ -103,6 +103,7 @@ enum EventSid
     EVENT_INVALID_GOAWAY_FRAME = 41,
     EVENT_SETTINGS_QUEUE_OVERFLOW = 42,
     EVENT_SETTINGS_QUEUE_UNDERFLOW = 43,
+    EVENT_ABOVE_SETTINGS_MAX_FRAME_SIZE = 44,
     EVENT__MAX_VALUE
 };
 
@@ -167,6 +168,7 @@ enum Infraction
     INF_BAD_GOAWAY_FRAME_R_BIT = 54,
     INF_SETTINGS_QUEUE_OVERFLOW = 55,
     INF_SETTINGS_QUEUE_UNDERFLOW = 56,
+    INF_ABOVE_SETTINGS_MAX_FRAME_SIZE = 57,
     INF__MAX_VALUE
 };
 
diff --git a/src/service_inspectors/http2_inspect/http2_frame.cc b/src/service_inspectors/http2_inspect/http2_frame.cc
index bdffbda5d..1b69f2bb1 100644
--- a/src/service_inspectors/http2_inspect/http2_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_frame.cc
@@ -57,7 +57,7 @@ Http2Frame::Http2Frame(const uint8_t* header_buffer, const uint32_t header_len,
 
 Http2Frame* Http2Frame::new_frame(const uint8_t* header, const uint32_t header_len,
     const uint8_t* data, const uint32_t data_len, Http2FlowData* session_data, SourceId source_id,
-    Http2Stream* stream)
+    const Http2ParaList* params, Http2Stream* stream)
 {
     Http2Frame* frame = nullptr;
 
@@ -77,7 +77,7 @@ Http2Frame* Http2Frame::new_frame(const uint8_t* header, const uint32_t header_l
             break;
         case FT_SETTINGS:
             frame = new Http2SettingsFrame(header, header_len, data, data_len, session_data,
-                source_id, stream);
+                source_id, stream, params);
             break;
         case FT_DATA:
             frame = new Http2DataFrame(header, header_len, data, data_len, session_data, source_id,
diff --git a/src/service_inspectors/http2_inspect/http2_frame.h b/src/service_inspectors/http2_inspect/http2_frame.h
index 0013324f4..2d2ec3cef 100644
--- a/src/service_inspectors/http2_inspect/http2_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_frame.h
@@ -25,6 +25,7 @@
 #include "service_inspectors/http_inspect/http_field.h"
 
 #include "http2_enum.h"
+#include "http2_module.h"
 
 /* This class is called Http2Frame, but an object of this class may not represent exactly one
  * HTTP/2 frame as received on the wire. For HEADERS frames, the Http2Frame object contains the
@@ -43,7 +44,7 @@ public:
     virtual ~Http2Frame() = default;
     static Http2Frame* new_frame(const uint8_t* header_buffer, const uint32_t header_len,
         const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* session_data,
-        HttpCommon::SourceId source_id, Http2Stream* stream);
+        HttpCommon::SourceId source_id, const Http2ParaList* params, Http2Stream* stream);
     virtual bool valid_sequence(Http2Enums::StreamState) { return true; }
     virtual void analyze_http1(snort::Packet*) { }
     virtual void clear(snort::Packet*) { }
diff --git a/src/service_inspectors/http2_inspect/http2_goaway_frame.h b/src/service_inspectors/http2_inspect/http2_goaway_frame.h
index 8127162bc..e9a428fb5 100644
--- a/src/service_inspectors/http2_inspect/http2_goaway_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_goaway_frame.h
@@ -28,7 +28,7 @@ class Http2GoAwayFrame : public Http2Frame
 {
 public:
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
-        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, const Http2ParaList* params, Http2Stream* stream);
     bool is_detection_required() const override { return false; }
 
 private:
diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_header.h b/src/service_inspectors/http2_inspect/http2_headers_frame_header.h
index 56d278cf9..6a7e3ab3e 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_header.h
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_header.h
@@ -29,7 +29,7 @@ class Http2HeadersFrameHeader : public Http2HeadersFrameWithStartline
 {
 public:
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
-        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, const Http2ParaList* params, Http2Stream* stream);
 
     bool valid_sequence(Http2Enums::StreamState state) override;
     void analyze_http1(snort::Packet*) override;
diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h
index 295b64579..a5afc3694 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h
@@ -27,7 +27,7 @@ class Http2HeadersFrameTrailer : public Http2HeadersFrame
 {
 public:
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
-        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, const Http2ParaList* params, Http2Stream* stream);
 
     bool valid_sequence(Http2Enums::StreamState state) override;
     void analyze_http1(snort::Packet*) override;
diff --git a/src/service_inspectors/http2_inspect/http2_inspect.cc b/src/service_inspectors/http2_inspect/http2_inspect.cc
index ff01c2aa3..d11989506 100644
--- a/src/service_inspectors/http2_inspect/http2_inspect.cc
+++ b/src/service_inspectors/http2_inspect/http2_inspect.cc
@@ -148,7 +148,7 @@ void Http2Inspect::eval(Packet* p)
     uint8_t* const frame_header_copy = new uint8_t[FRAME_HEADER_LENGTH];
     memcpy(frame_header_copy, session_data->lead_frame_header[source_id], FRAME_HEADER_LENGTH);
     stream->eval_frame(frame_header_copy, FRAME_HEADER_LENGTH,
-        session_data->frame_data[source_id], session_data->frame_data_size[source_id], source_id, p);
+        session_data->frame_data[source_id], session_data->frame_data_size[source_id], source_id, p, params);
 
     if (!stream->get_current_frame()->is_detection_required())
         DetectionEngine::disable_all(p);
@@ -208,6 +208,7 @@ void Http2Inspect::show(const SnortConfig*) const
 {
     assert(params);
     ConfigLogger::log_value("concurrent_streams_limit", params->concurrent_streams_limit);
+    ConfigLogger::log_value("settings_max_frame_size", params->settings_max_frame_size);
 }
 
 #ifdef REG_TEST
diff --git a/src/service_inspectors/http2_inspect/http2_module.cc b/src/service_inspectors/http2_inspect/http2_module.cc
index e04dc8248..f6d70fba8 100644
--- a/src/service_inspectors/http2_inspect/http2_module.cc
+++ b/src/service_inspectors/http2_inspect/http2_module.cc
@@ -30,6 +30,8 @@ const Parameter Http2Module::http2_params[] =
 {
     { "concurrent_streams_limit", Parameter::PT_INT, "100:1000", "100",
       "Maximum number of concurrent streams allowed in a single HTTP/2 flow" },
+    { "settings_max_frame_size", Parameter::PT_INT, "16384:16777215", "16777215",
+      "Maximum allowed value for settings frame SETTINGS_MAX_FRAME_SIZE" },
 #ifdef REG_TEST
     { "test_input", Parameter::PT_BOOL, nullptr, "false",
       "read HTTP/2 messages from text file" },
@@ -61,6 +63,10 @@ bool Http2Module::set(const char*, Value& val, SnortConfig*)
     {
         params->concurrent_streams_limit = val.get_uint32();
     }
+    else if (val.is("settings_max_frame_size"))
+    {
+        params->settings_max_frame_size = val.get_uint32();
+    }
 #ifdef REG_TEST
     else if (val.is("test_input"))
     {
diff --git a/src/service_inspectors/http2_inspect/http2_module.h b/src/service_inspectors/http2_inspect/http2_module.h
index 3fc7ac48c..7b703d648 100644
--- a/src/service_inspectors/http2_inspect/http2_module.h
+++ b/src/service_inspectors/http2_inspect/http2_module.h
@@ -35,6 +35,7 @@ struct Http2ParaList
 {
 public:
     uint32_t concurrent_streams_limit;
+    uint32_t settings_max_frame_size;
 #ifdef REG_TEST
 
     bool test_input;
diff --git a/src/service_inspectors/http2_inspect/http2_ping_frame.h b/src/service_inspectors/http2_inspect/http2_ping_frame.h
index 5f44e9dc0..4ae980cdd 100644
--- a/src/service_inspectors/http2_inspect/http2_ping_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_ping_frame.h
@@ -29,7 +29,7 @@ class Http2PingFrame : public Http2Frame
 {
 public:
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
-        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, const Http2ParaList* params, Http2Stream* stream);
     bool is_detection_required() const override { return false; }
 
 private:
diff --git a/src/service_inspectors/http2_inspect/http2_priority_frame.h b/src/service_inspectors/http2_inspect/http2_priority_frame.h
index a8a168b4d..16f8ba3cd 100644
--- a/src/service_inspectors/http2_inspect/http2_priority_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_priority_frame.h
@@ -28,7 +28,7 @@ class Http2PriorityFrame : public Http2Frame
 {
 public:
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
-        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, const Http2ParaList* params, Http2Stream* stream);
     bool is_detection_required() const override { return false; }
 
 private:
diff --git a/src/service_inspectors/http2_inspect/http2_push_promise_frame.h b/src/service_inspectors/http2_inspect/http2_push_promise_frame.h
index 339aed27c..a71f15f22 100644
--- a/src/service_inspectors/http2_inspect/http2_push_promise_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_push_promise_frame.h
@@ -47,7 +47,7 @@ public:
         Http2Infractions* const infractions, const uint8_t* data_buffer, uint32_t data_len);
 
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
-        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, const Http2ParaList* params, Http2Stream* stream);
 
 #ifdef REG_TEST
     void print_frame(FILE* output) override;
diff --git a/src/service_inspectors/http2_inspect/http2_rst_stream_frame.h b/src/service_inspectors/http2_inspect/http2_rst_stream_frame.h
index 069d0be3f..7d9045bdf 100644
--- a/src/service_inspectors/http2_inspect/http2_rst_stream_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_rst_stream_frame.h
@@ -28,7 +28,7 @@ class Http2RstStreamFrame : public Http2Frame
 {
 public:
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
-        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, const Http2ParaList* params, Http2Stream* stream);
     bool is_detection_required() const override { return false; }
     bool valid_sequence(Http2Enums::StreamState state) override;
     void update_stream_state() override;
diff --git a/src/service_inspectors/http2_inspect/http2_settings_frame.cc b/src/service_inspectors/http2_inspect/http2_settings_frame.cc
index cc849d2e8..6b0a1bcc2 100644
--- a/src/service_inspectors/http2_inspect/http2_settings_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_settings_frame.cc
@@ -48,8 +48,8 @@ static uint32_t get_parameter_value(const uint8_t* data_buffer)
 
 Http2SettingsFrame::Http2SettingsFrame(const uint8_t* header_buffer, const uint32_t header_len,
     const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* ssn_data,
-    HttpCommon::SourceId src_id, Http2Stream* stream_) : Http2Frame(header_buffer, header_len,
-    data_buffer, data_len, ssn_data, src_id, stream_)
+    HttpCommon::SourceId src_id, Http2Stream* stream_, const Http2ParaList* params_) : Http2Frame(header_buffer, header_len,
+    data_buffer, data_len, ssn_data, src_id, stream_), params(params_)
 {
     if (!sanity_check())
     {
@@ -101,6 +101,11 @@ void Http2SettingsFrame::queue_settings()
             session_data->events[source_id]->create_event(EVENT_BAD_SETTINGS_VALUE);
             *session_data->infractions[source_id] += INF_BAD_SETTINGS_PUSH_VALUE;
         }
+        else if (parameter_id == SFID_MAX_FRAME_SIZE and parameter_value > params->settings_max_frame_size)
+        {
+            session_data->events[source_id]->create_event(EVENT_ABOVE_SETTINGS_MAX_FRAME_SIZE);
+            *session_data->infractions[source_id] += INF_ABOVE_SETTINGS_MAX_FRAME_SIZE;
+        }
         else
             settings.set_param(parameter_id, parameter_value);
     }
diff --git a/src/service_inspectors/http2_inspect/http2_settings_frame.h b/src/service_inspectors/http2_inspect/http2_settings_frame.h
index dd3c733e2..fbb8e5df5 100644
--- a/src/service_inspectors/http2_inspect/http2_settings_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_settings_frame.h
@@ -21,6 +21,7 @@
 #define HTTP2_SETTINGS_FRAME_H
 
 #include "http2_frame.h"
+#include "http2_module.h"
 
 class Field;
 class Http2ConnectionSettings;
@@ -30,7 +31,7 @@ class Http2SettingsFrame : public Http2Frame
 {
 public:
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
-        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, const Http2ParaList* params, Http2Stream* stream);
 
 #ifdef REG_TEST
     void print_frame(FILE* output) override;
@@ -39,7 +40,7 @@ public:
 private:
     Http2SettingsFrame(const uint8_t* header_buffer, const uint32_t header_len,
         const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* ssn_data,
-        HttpCommon::SourceId src_id, Http2Stream* stream);
+        HttpCommon::SourceId src_id, Http2Stream* stream, const Http2ParaList* params_);
 
     void queue_settings();
     bool sanity_check();
@@ -50,6 +51,7 @@ private:
     { return Http2Enums::FLAG_ACK; }
 
     bool bad_frame = false;
+    const Http2ParaList* params;
 };
 
 class Http2ConnectionSettings
diff --git a/src/service_inspectors/http2_inspect/http2_stream.cc b/src/service_inspectors/http2_inspect/http2_stream.cc
index bd2c1638b..d37f7ef41 100644
--- a/src/service_inspectors/http2_inspect/http2_stream.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream.cc
@@ -47,11 +47,12 @@ Http2Stream::~Http2Stream()
 }
 
 void Http2Stream::eval_frame(const uint8_t* header_buffer, uint32_t header_len,
-    const uint8_t* data_buffer, uint32_t data_len, SourceId source_id, Packet* p)
+    const uint8_t* data_buffer, uint32_t data_len, SourceId source_id, Packet* p,
+    const Http2ParaList* params)
 {
     assert(current_frame == nullptr);
     current_frame = Http2Frame::new_frame(header_buffer, header_len, data_buffer,
-        data_len, session_data, source_id, this);
+        data_len, session_data, source_id, params, this);
     if (!session_data->abort_flow[source_id] && (get_state(source_id) != STREAM_ERROR))
     {
         if (current_frame->valid_sequence(state[source_id]))
diff --git a/src/service_inspectors/http2_inspect/http2_stream.h b/src/service_inspectors/http2_inspect/http2_stream.h
index 29d952152..0533f2ed4 100644
--- a/src/service_inspectors/http2_inspect/http2_stream.h
+++ b/src/service_inspectors/http2_inspect/http2_stream.h
@@ -30,6 +30,7 @@ class Http2DataCutter;
 class Http2FlowData;
 class HttpFlowData;
 class HttpMsgSection;
+struct Http2ParaList;
 
 class Http2Stream
 {
@@ -38,7 +39,7 @@ public:
     ~Http2Stream();
     uint32_t get_stream_id() const { return stream_id; }
     void eval_frame(const uint8_t* header_buffer, uint32_t header_len, const uint8_t* data_buffer,
-        uint32_t data_len, HttpCommon::SourceId source_id, snort::Packet* p);
+        uint32_t data_len, HttpCommon::SourceId source_id, snort::Packet* p, const Http2ParaList* params);
     void check_and_cleanup_completed();
     void clear_frame(snort::Packet* p);
     const Field& get_buf(unsigned id);
diff --git a/src/service_inspectors/http2_inspect/http2_tables.cc b/src/service_inspectors/http2_inspect/http2_tables.cc
index cb787cc07..6b0861353 100644
--- a/src/service_inspectors/http2_inspect/http2_tables.cc
+++ b/src/service_inspectors/http2_inspect/http2_tables.cc
@@ -77,6 +77,7 @@ const RuleMap Http2Module::http2_events[] =
     { EVENT_INVALID_GOAWAY_FRAME, "invalid HTTP/2 GOAWAY frame" },
     { EVENT_SETTINGS_QUEUE_OVERFLOW, "too many unacknowledged settings" },
     { EVENT_SETTINGS_QUEUE_UNDERFLOW, "setting acknowledgment without actual settings" },
+    { EVENT_ABOVE_SETTINGS_MAX_FRAME_SIZE, "settings frame size greater than settings_max_frame_size"},
     { 0, nullptr }
 };
 
diff --git a/src/service_inspectors/http2_inspect/http2_window_update_frame.h b/src/service_inspectors/http2_inspect/http2_window_update_frame.h
index 30f759605..c5971db44 100644
--- a/src/service_inspectors/http2_inspect/http2_window_update_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_window_update_frame.h
@@ -28,7 +28,7 @@ class Http2WindowUpdateFrame : public Http2Frame
 {
 public:
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const uint32_t, const uint8_t*,
-        const uint32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
+        const uint32_t, Http2FlowData*, HttpCommon::SourceId, const Http2ParaList* params, Http2Stream* stream);
     bool is_detection_required() const override { return false; }
     bool valid_sequence(Http2Enums::StreamState state) override;