From: Adrian Mamolea (admamole) <admamole@cisco.com>
Date: Mon, 20 May 2024 08:07:31 +0000 (+0000)
Subject: Pull request #4286: service_inspectors: fix get_buf handling
X-Git-Tag: 3.2.2.0~15
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ec51156a20e781b108b4a907371588745b330f5c;p=thirdparty%2Fsnort3.git

Pull request #4286: service_inspectors: fix get_buf handling

Merge in SNORT/snort3 from ~ADMAMOLE/snort3:get_buf_2 to master

Squashed commit of the following:

commit e243f646716bfa8f66330a2a059983274cc59f82
Author: Adrian Mamolea <admamole@cisco.com>
Date:   Thu Apr 4 08:11:09 2024 -0400

    service_inspectors: fix get_buf handling
---

diff --git a/src/loggers/alert_fast.cc b/src/loggers/alert_fast.cc
index cb2d4a30c..bccb3c786 100644
--- a/src/loggers/alert_fast.cc
+++ b/src/loggers/alert_fast.cc
@@ -58,6 +58,24 @@ static once_flag init_flag;
 #define S_NAME "alert_fast"
 #define F_NAME S_NAME ".txt"
 
+//-------------------------------------------------------------------------
+// translation stuff
+//-------------------------------------------------------------------------
+
+enum BuffersToOutput
+{
+    BUFFERS_NONE = 0,
+    BUFFERS_RULE,
+    BUFFERS_INSPECTOR,
+    BUFFERS_BOTH,
+};
+
+static void param_to_buffers(unsigned param, bool& buffers_rule, bool& buffers_inspector)
+{
+    buffers_rule = param == BUFFERS_RULE or param == BUFFERS_BOTH;
+    buffers_inspector = param == BUFFERS_INSPECTOR or param == BUFFERS_BOTH;
+}
+
 //-------------------------------------------------------------------------
 // module stuff
 //-------------------------------------------------------------------------
@@ -70,8 +88,8 @@ static const Parameter s_params[] =
     { "packet", Parameter::PT_BOOL, nullptr, "false",
       "output packet dump with alert" },
 
-    { "buffers", Parameter::PT_BOOL, nullptr, "false",
-      "output IPS buffer dump" },
+    { "buffers", Parameter::PT_ENUM, "none | rule | inspector | both", "none",
+      "output IPS buffer dump (evaluated by IPS rule or an inspector)" },
 
     { "buffers_depth", Parameter::PT_INT, "0:maxSZ", "0",
       "number of IPS buffer bytes to dump per buffer (0 is unlimited)" },
@@ -101,7 +119,8 @@ public:
     size_t buffers_depth = 0;
     bool file = false;
     bool packet = false;
-    bool buffers = false;
+    bool buffers_rule = false;
+    bool buffers_inspector = false;
 };
 
 bool FastModule::set(const char*, Value& v, SnortConfig*)
@@ -113,7 +132,7 @@ bool FastModule::set(const char*, Value& v, SnortConfig*)
         packet = v.get_bool();
 
     else if ( v.is("buffers") )
-        buffers = v.get_bool();
+        param_to_buffers(v.get_uint8(), buffers_rule, buffers_inspector);
 
     else if ( v.is("limit") )
         limit = v.get_size() * 1024 * 1024;
@@ -129,7 +148,8 @@ bool FastModule::begin(const char*, int, SnortConfig*)
     file = false;
     limit = 0;
     packet = false;
-    buffers = false;
+    buffers_rule = false;
+    buffers_inspector = false;
     buffers_depth = 0;
     return true;
 }
@@ -212,6 +232,31 @@ static void log_ips_buffers(Packet* p, const char** buffs_to_dump, unsigned long
     }
 }
 
+static void log_inspector_buffers(Packet* p, unsigned long depth)
+{
+    if ( !p->flow or !p->flow->gadget )
+        return;
+
+    Inspector* gadget = p->flow->gadget;
+    const char* gadget_name = gadget->get_name();
+    const char** buffers = gadget->get_api()->buffers;
+
+    if ( !buffers )
+        return;
+
+    for ( ; *buffers; buffers++ )
+    {
+        InspectionBuffer buf;
+
+        // FIXIT-E avoid forcing evaluation of JIT buffers
+        if ( gadget->get_buf(*buffers, p, buf) )
+        {
+            int log_depth = depth && depth < buf.len ? depth : buf.len;
+            ObfuscateLogNetData(fast_log, buf.data, log_depth, p, *buffers, *buffers, gadget_name);
+        }
+    }
+}
+
 using BufferIds = std::vector<unsigned>;
 
 //-------------------------------------------------------------------------
@@ -239,7 +284,8 @@ private:
     unsigned long limit;
     unsigned long buffers_depth;
     bool packet;
-    bool log_buffers;
+    bool buffers_rule;
+    bool buffers_inspector;
 
     static std::vector<unsigned> req_ids;
     static std::vector<unsigned> rsp_ids;
@@ -249,7 +295,8 @@ std::vector<unsigned> FastLogger::req_ids;
 std::vector<unsigned> FastLogger::rsp_ids;
 
 FastLogger::FastLogger(FastModule* m) : file(m->file ? F_NAME : "stdout"), limit(m->limit),
-    buffers_depth(m->buffers_depth), packet(m->packet), log_buffers(m->buffers)
+    buffers_depth(m->buffers_depth), packet(m->packet), buffers_rule(m->buffers_rule),
+    buffers_inspector(m->buffers_inspector)
 { }
 
 //-----------------------------------------------------------------
@@ -326,6 +373,9 @@ void FastLogger::alert(Packet* p, const char* msg, const Event& event)
     }
     TextLog_NewLine(fast_log);
     TextLog_Flush(fast_log);
+
+    if ( buffers_inspector )
+        log_inspector_buffers(p, buffers_depth);
 }
 
 // log packet (p) if this is not an http request with one or more buffers
@@ -389,7 +439,7 @@ void FastLogger::log_data(Packet* p, const Event& event)
     if ( buf.len and event.get_gid() != 116 )
         LogNetData(fast_log, buf.data, buf.len, p, "alt");
 
-    if ( log_buffers )
+    if ( buffers_rule )
         log_ips_buffers(p, event.get_buffers(), buffers_depth);
 }
 
diff --git a/src/service_inspectors/dnp3/dnp3.cc b/src/service_inspectors/dnp3/dnp3.cc
index 9f6344c53..77d87712c 100644
--- a/src/service_inspectors/dnp3/dnp3.cc
+++ b/src/service_inspectors/dnp3/dnp3.cc
@@ -34,9 +34,38 @@
 
 using namespace snort;
 
+// Indices in the buffer array exposed by InspectApi
+// Must remain synchronized with dnp3_bufs
+enum Dnp3BufId
+{
+    DNP3_DATA_BUFID = 1
+};
+
 THREAD_LOCAL Dnp3Stats dnp3_stats;
 THREAD_LOCAL ProfileStats dnp3_perf_stats;
 
+bool get_buf_dnp3_data(snort::Packet* p, snort::InspectionBuffer& b)
+{
+    if ((p->has_tcp_data() && !p->is_full_pdu()) || !p->flow || !p->dsize)
+        return false;
+
+    Dnp3FlowData* fd = (Dnp3FlowData*)p->flow->get_flow_data(Dnp3FlowData::inspector_id);
+    if (!fd)
+        return false;
+
+    const dnp3_session_data_t& sd = fd->dnp3_session;
+    const dnp3_reassembly_data_t& rdata = (sd.direction == DNP3_CLIENT) ? sd.client_rdata : sd.server_rdata;
+
+    /* Return a buffer only for complete application-layer fragments */
+    if (rdata.state != DNP3_REASSEMBLY_STATE__DONE)
+        return false;
+
+    b.data = rdata.buffer;
+    b.len = rdata.buflen;
+    b.is_accumulated = false;
+    return true;
+}
+
 Dnp3FlowData::Dnp3FlowData() : FlowData(inspector_id)
 {
     dnp3_stats.concurrent_sessions++;
@@ -200,6 +229,9 @@ public:
     StreamSplitter* get_splitter(bool c2s) override
     { return new Dnp3Splitter(c2s); }
 
+    bool get_buf(unsigned id, snort::Packet* p, snort::InspectionBuffer& b) override
+    { return (id == DNP3_DATA_BUFID) ? get_buf_dnp3_data(p, b) : false; }
+
 private:
     dnp3ProtoConf config;
 };
diff --git a/src/service_inspectors/dnp3/dnp3.h b/src/service_inspectors/dnp3/dnp3.h
index 52645d176..52bf4c81f 100644
--- a/src/service_inspectors/dnp3/dnp3.h
+++ b/src/service_inspectors/dnp3/dnp3.h
@@ -184,5 +184,7 @@ public:
 extern THREAD_LOCAL Dnp3Stats dnp3_stats;
 extern THREAD_LOCAL snort::ProfileStats dnp3_perf_stats;
 
+bool get_buf_dnp3_data(snort::Packet* p, snort::InspectionBuffer& b);
+
 #endif
 
diff --git a/src/service_inspectors/dnp3/ips_dnp3_data.cc b/src/service_inspectors/dnp3/ips_dnp3_data.cc
index 439637b61..9e8ec83eb 100644
--- a/src/service_inspectors/dnp3/ips_dnp3_data.cc
+++ b/src/service_inspectors/dnp3/ips_dnp3_data.cc
@@ -78,27 +78,11 @@ IpsOption::EvalStatus Dnp3DataOption::eval(Cursor& c, Packet* p)
     // cppcheck-suppress unreadVariable
     RuleProfile profile(dnp3_data_perf_stats);
 
-    if ((p->has_tcp_data() && !p->is_full_pdu()) || !p->flow || !p->dsize)
+    InspectionBuffer b;
+    if (!get_buf_dnp3_data(p, b))
         return NO_MATCH;
 
-    Dnp3FlowData* fd = (Dnp3FlowData*)p->flow->get_flow_data(Dnp3FlowData::inspector_id);
-
-    if (!fd)
-        return NO_MATCH;
-
-    dnp3_session_data_t* dnp3_session = &fd->dnp3_session;
-    dnp3_reassembly_data_t* rdata;
-
-    if (dnp3_session->direction == DNP3_CLIENT)
-        rdata = &(dnp3_session->client_rdata);
-    else
-        rdata = &(dnp3_session->server_rdata);
-
-    /* Only evaluate rules against complete Application-layer fragments */
-    if (rdata->state != DNP3_REASSEMBLY_STATE__DONE)
-        return NO_MATCH;
-
-    c.set(s_name,(uint8_t*)rdata->buffer, rdata->buflen);
+    c.set(s_name, b.data, b.len);
 
     return MATCH;
 }
diff --git a/src/service_inspectors/http2_inspect/http2_api.cc b/src/service_inspectors/http2_inspect/http2_api.cc
index 095634995..a5c554927 100644
--- a/src/service_inspectors/http2_inspect/http2_api.cc
+++ b/src/service_inspectors/http2_inspect/http2_api.cc
@@ -21,8 +21,9 @@
 #include "config.h"
 #endif
 
-#include "http2_api.h"
+#include "service_inspectors/http_inspect/http_api.h"
 
+#include "http2_api.h"
 #include "http2_inspect.h"
 
 using namespace snort;
@@ -42,6 +43,7 @@ const char* Http2Api::classic_buffer_names[] =
     "http2_frame_header",
     "http2_decoded_header",
 #endif
+    HTTP_CLASSIC_BUFFER_NAMES,
     nullptr
 };
 
diff --git a/src/service_inspectors/http2_inspect/http2_inspect.cc b/src/service_inspectors/http2_inspect/http2_inspect.cc
index 40d7b03ae..c44ae0996 100644
--- a/src/service_inspectors/http2_inspect/http2_inspect.cc
+++ b/src/service_inspectors/http2_inspect/http2_inspect.cc
@@ -79,6 +79,9 @@ bool Http2Inspect::get_buf(unsigned id, Packet* p, InspectionBuffer& b)
     if (session_data->stream_in_hi == Http2Enums::NO_STREAM_ID)
         return false;
 
+    if  (id >= HTTP2_BUFFER__MAX)
+        return session_data->hi->get_buf(id - HTTP2_BUFFER__MAX + 1, p, b);
+
     Http2Stream* const stream = session_data->find_processing_stream();
     assert(stream != nullptr);
     const Field& buffer = stream->get_buf(id);
diff --git a/src/service_inspectors/http_inspect/http_api.cc b/src/service_inspectors/http_inspect/http_api.cc
index 27d9309e1..c7831a0c8 100644
--- a/src/service_inspectors/http_inspect/http_api.cc
+++ b/src/service_inspectors/http_inspect/http_api.cc
@@ -49,27 +49,7 @@ void HttpApi::http_init()
 
 const char* HttpApi::classic_buffer_names[] =
 {
-    "file_data",
-    "http_client_body",
-    "http_cookie",
-    "http_header",
-    "http_method",
-    "http_param",
-    "http_raw_body",
-    "http_raw_cookie",
-    "http_raw_header",
-    "http_raw_request",
-    "http_raw_status",
-    "http_raw_trailer",
-    "http_raw_uri",
-    "http_stat_code",
-    "http_stat_msg",
-    "http_trailer",
-    "http_true_ip",
-    "http_uri",
-    "http_version",
-    "js_data",
-    "vba_data",
+    HTTP_CLASSIC_BUFFER_NAMES,
     nullptr
 };
 
diff --git a/src/service_inspectors/http_inspect/http_api.h b/src/service_inspectors/http_inspect/http_api.h
index 1ad901ca4..dc58d3327 100644
--- a/src/service_inspectors/http_inspect/http_api.h
+++ b/src/service_inspectors/http_inspect/http_api.h
@@ -26,6 +26,29 @@
 #include "http_flow_data.h"
 #include "http_module.h"
 
+#define HTTP_CLASSIC_BUFFER_NAMES \
+    "file_data",                  \
+    "http_client_body",           \
+    "http_cookie",                \
+    "http_header",                \
+    "http_method",                \
+    "http_param",                 \
+    "http_raw_body",              \
+    "http_raw_cookie",            \
+    "http_raw_header",            \
+    "http_raw_request",           \
+    "http_raw_status",            \
+    "http_raw_trailer",           \
+    "http_raw_uri",               \
+    "http_stat_code",             \
+    "http_stat_msg",              \
+    "http_trailer",               \
+    "http_true_ip",               \
+    "http_uri",                   \
+    "http_version",               \
+    "js_data",                    \
+    "vba_data"
+
 class HttpApi
 {
 public:
diff --git a/src/service_inspectors/http_inspect/http_msg_section.cc b/src/service_inspectors/http_inspect/http_msg_section.cc
index b578440da..c93e03db7 100644
--- a/src/service_inspectors/http_inspect/http_msg_section.cc
+++ b/src/service_inspectors/http_inspect/http_msg_section.cc
@@ -283,7 +283,7 @@ const Field& HttpMsgSection::get_classic_buffer(const HttpBufferInfo& buf)
             return Field::FIELD_NULL;
     }
     default:
-        assert(false);
+        assert(buf.type <= HTTP__BUFFER_MAX);
         return Field::FIELD_NULL;
     }
 }
diff --git a/src/service_inspectors/imap/imap.cc b/src/service_inspectors/imap/imap.cc
index 5617f46c3..cd5b4e677 100644
--- a/src/service_inspectors/imap/imap.cc
+++ b/src/service_inspectors/imap/imap.cc
@@ -40,6 +40,13 @@
 
 using namespace snort;
 
+// Indices in the buffer array exposed by InspectApi
+// Must remain synchronized with imap_bufs
+enum ImapBufId
+{
+    IMAP_FILE_DATA_ID = 1, IMAP_VBA_DATA_ID, IMAP_JS_DATA_ID
+};
+
 THREAD_LOCAL ProfileStats imapPerfStats;
 THREAD_LOCAL ImapStats imapstats;
 
@@ -763,6 +770,7 @@ public:
     bool get_buf(InspectionBuffer::Type, Packet*, InspectionBuffer&) override;
     bool get_fp_buf(snort::InspectionBuffer::Type ibt, snort::Packet* p,
         snort::InspectionBuffer& b) override;
+    bool get_buf(unsigned id, snort::Packet* p, snort::InspectionBuffer& b) override;
 
 private:
     IMAP_PROTO_CONF* config;
@@ -860,6 +868,21 @@ bool Imap::get_fp_buf(InspectionBuffer::Type ibt, Packet* p, InspectionBuffer& b
     return get_buf(ibt, p, b);
 }
 
+bool Imap::get_buf(unsigned id, snort::Packet* p, snort::InspectionBuffer& b)
+{
+    switch (id)
+    {
+    case IMAP_FILE_DATA_ID:
+        return false;
+    case IMAP_VBA_DATA_ID:
+        return get_buf(InspectionBuffer::IBT_VBA, p, b);
+    case IMAP_JS_DATA_ID:
+        return get_buf(InspectionBuffer::IBT_JS_DATA, p, b);
+    default:
+        return false;
+    }
+}
+
 //-------------------------------------------------------------------------
 // api stuff
 //-------------------------------------------------------------------------
diff --git a/src/service_inspectors/mms/ips_mms_data.cc b/src/service_inspectors/mms/ips_mms_data.cc
index 9402b3d3d..2109ced33 100644
--- a/src/service_inspectors/mms/ips_mms_data.cc
+++ b/src/service_inspectors/mms/ips_mms_data.cc
@@ -58,34 +58,11 @@ IpsOption::EvalStatus MmsDataOption::eval(Cursor& c, Packet* p)
     // cppcheck-suppress unreadVariable
     RuleProfile profile(mms_data_prof);
 
-    if (!p->flow)
-    {
-        return NO_MATCH;
-    }
-
-    // not including any checks for a full PDU as we're not guaranteed to
-    // have one with the available pipelining options to get to MMS
-
-    MmsFlowData* mmsfd = (MmsFlowData*)p->flow->get_flow_data(MmsFlowData::inspector_id);
-
-    if (!mmsfd)
-    {
-        return NO_MATCH;
-    }
-
-    if (!mmsfd->is_mms_found())
-    {
-        return NO_MATCH;
-    }
-
-    if (mmsfd->get_mms_offset() >= p->dsize)
-    {
+    InspectionBuffer b;
+    if (!get_buf_mms_data(p, b))
         return NO_MATCH;
-    }
 
-    // setting the cursor to the offset previously determined by util_tpkt
-    // to be the start of the MMS message
-    c.set(s_name, p->data + mmsfd->get_mms_offset(), p->dsize - mmsfd->get_mms_offset());
+    c.set(s_name, b.data, b.len);
 
     return MATCH;
 }
diff --git a/src/service_inspectors/mms/mms.cc b/src/service_inspectors/mms/mms.cc
index 9c7506d44..cca73ce6d 100644
--- a/src/service_inspectors/mms/mms.cc
+++ b/src/service_inspectors/mms/mms.cc
@@ -37,8 +37,34 @@
 
 using namespace snort;
 
+// Indices in the buffer array exposed by InspectApi
+// Must remain synchronized with mms_bufs
+enum MmsBufId
+{
+    MMS_DATA_BUFID = 1
+};
+
 THREAD_LOCAL MmsStats mms_stats;
 
+bool get_buf_mms_data(snort::Packet* p, snort::InspectionBuffer& b)
+{
+    if (!p->flow)
+        return false;
+
+    // not including any checks for a full PDU as we're not guaranteed to
+    // have one with the available pipelining options to get to MMS
+    MmsFlowData* mmsfd = (MmsFlowData*)p->flow->get_flow_data(MmsFlowData::inspector_id);
+    if (!mmsfd or !mmsfd->is_mms_found() or mmsfd->get_mms_offset() >= p->dsize)
+        return false;
+
+    // setting the cursor to the offset previously determined by util_tpkt
+    // to be the start of the MMS message
+    b.data = p->data + mmsfd->get_mms_offset();
+    b.len = p->dsize - mmsfd->get_mms_offset();
+    b.is_accumulated = false;
+    return true;
+}
+
 //-------------------------------------------------------------------------
 // flow stuff
 //-------------------------------------------------------------------------
@@ -83,6 +109,11 @@ public:
     {
         return new MmsSplitter(c2s);
     }
+
+    bool get_buf(unsigned id, snort::Packet* p, snort::InspectionBuffer& b) override
+    {
+        return (id == MMS_DATA_BUFID) ? get_buf_mms_data(p, b) : false;
+    }
 };
 
 void Mms::eval(Packet* p)
diff --git a/src/service_inspectors/mms/mms.h b/src/service_inspectors/mms/mms.h
index b47f40b56..d30af4614 100644
--- a/src/service_inspectors/mms/mms.h
+++ b/src/service_inspectors/mms/mms.h
@@ -108,6 +108,7 @@ enum MmsMsgType
 };
 
 extern THREAD_LOCAL MmsStats mms_stats;
+bool get_buf_mms_data(snort::Packet* p, snort::InspectionBuffer& b);
 
 #endif
 
diff --git a/src/service_inspectors/modbus/ips_modbus_data.cc b/src/service_inspectors/modbus/ips_modbus_data.cc
index d7d719226..1a1f05920 100644
--- a/src/service_inspectors/modbus/ips_modbus_data.cc
+++ b/src/service_inspectors/modbus/ips_modbus_data.cc
@@ -30,6 +30,7 @@
 #include "profiler/profiler.h"
 #include "protocols/packet.h"
 
+#include "modbus.h"
 #include "modbus_decode.h"
 
 using namespace snort;
@@ -75,16 +76,11 @@ IpsOption::EvalStatus ModbusDataOption::eval(Cursor& c, Packet* p)
 {
     RuleProfile profile(modbus_data_prof);  // cppcheck-suppress unreadVariable
 
-    if ( !p->flow )
+    InspectionBuffer b;
+    if (!get_buf_modbus_data(p, b))
         return NO_MATCH;
 
-    if ( !p->is_full_pdu() )
-        return NO_MATCH;
-
-    if ( p->dsize < MODBUS_MIN_LEN )
-        return NO_MATCH;
-
-    c.set(s_name, p->data + MODBUS_MIN_LEN, p->dsize - MODBUS_MIN_LEN);
+    c.set(s_name, b.data, b.len);
 
     return MATCH;
 }
diff --git a/src/service_inspectors/modbus/modbus.cc b/src/service_inspectors/modbus/modbus.cc
index 80efed276..35a3c4b54 100644
--- a/src/service_inspectors/modbus/modbus.cc
+++ b/src/service_inspectors/modbus/modbus.cc
@@ -36,6 +36,23 @@ using namespace snort;
 
 THREAD_LOCAL ModbusStats modbus_stats;
 
+// Indices in the buffer array exposed by InspectApi
+// Must remain synchronized with modbus_bufs
+enum ModbusBufId
+{
+     MODBUS_DATA_BUFID = 1
+};
+
+bool get_buf_modbus_data(Packet* p, InspectionBuffer& b)
+{
+    if ( !p->is_full_pdu() or p->dsize < MODBUS_MIN_LEN )
+        return false;
+
+    b.data = p->data + MODBUS_MIN_LEN;
+    b.len = p->dsize - MODBUS_MIN_LEN;
+    return true;
+}
+
 //-------------------------------------------------------------------------
 // flow stuff
 //-------------------------------------------------------------------------
@@ -70,7 +87,10 @@ class Modbus : public Inspector
 public:
     // default ctor / dtor
     void eval(Packet*) override;
-    bool get_buf(InspectionBuffer::Type, Packet*, InspectionBuffer&) override;
+    bool get_buf(InspectionBuffer::Type ibt, Packet* p, InspectionBuffer& b) override
+    { return (ibt == InspectionBuffer::IBT_BODY) ? get_buf_modbus_data(p, b) : false; }
+    bool get_buf(unsigned id, snort::Packet* p, snort::InspectionBuffer& b) override
+    { return (id == MODBUS_DATA_BUFID) ? get_buf_modbus_data(p, b) : false; }
 
     int get_message_type(int version, const char* name);
     int get_info_type(int version, const char* name);
@@ -118,16 +138,6 @@ void Modbus::eval(Packet* p)
         mfd->reset();
 }
 
-bool Modbus::get_buf(InspectionBuffer::Type ibt, Packet* p, InspectionBuffer& b)
-{
-    if ( ibt !=  InspectionBuffer::IBT_BODY or p->dsize <= MODBUS_MIN_LEN )
-        return false;
-
-    b.data = p->data + MODBUS_MIN_LEN;
-    b.len = p->dsize - MODBUS_MIN_LEN;
-    return true;
-}
-
 //-------------------------------------------------------------------------
 // plugin stuff
 //-------------------------------------------------------------------------
diff --git a/src/service_inspectors/modbus/modbus.h b/src/service_inspectors/modbus/modbus.h
index 35a85ae3b..3a4c13f27 100644
--- a/src/service_inspectors/modbus/modbus.h
+++ b/src/service_inspectors/modbus/modbus.h
@@ -63,6 +63,7 @@ int get_message_type(int version, const char* name);
 int get_info_type(int version, const char* name);
 
 extern THREAD_LOCAL ModbusStats modbus_stats;
+bool get_buf_modbus_data(snort::Packet* p, snort::InspectionBuffer& b);
 
 #endif
 
diff --git a/src/service_inspectors/pop/pop.cc b/src/service_inspectors/pop/pop.cc
index cfea8852a..fc82f8a44 100644
--- a/src/service_inspectors/pop/pop.cc
+++ b/src/service_inspectors/pop/pop.cc
@@ -40,6 +40,13 @@
 
 using namespace snort;
 
+// Indices in the buffer array exposed by InspectApi
+// Must remain synchronized with pop_bufs
+enum PopBufId
+{
+    POP_FILE_DATA_ID = 1, POP_VBA_DATA_ID, POP_JS_DATA_ID
+};
+
 THREAD_LOCAL ProfileStats popPerfStats;
 THREAD_LOCAL PopStats popstats;
 
@@ -699,8 +706,7 @@ public:
     { return true; }
 
     bool get_buf(InspectionBuffer::Type, Packet*, InspectionBuffer&) override;
-    bool get_fp_buf(snort::InspectionBuffer::Type ibt, snort::Packet* p,
-        snort::InspectionBuffer& b) override;
+    bool get_buf(unsigned id, snort::Packet* p, snort::InspectionBuffer& b) override;
 
 private:
     POP_PROTO_CONF* config;
@@ -791,9 +797,19 @@ bool Pop::get_buf(InspectionBuffer::Type ibt, Packet* p, InspectionBuffer& b)
     return dst && dst_len;
 }
 
-bool Pop::get_fp_buf(InspectionBuffer::Type ibt, Packet* p, InspectionBuffer& b)
+bool Pop::get_buf(unsigned id, snort::Packet* p, snort::InspectionBuffer& b)
 {
-    return get_buf(ibt, p, b);
+    switch (id)
+    {
+    case POP_FILE_DATA_ID:
+        return false;
+    case POP_VBA_DATA_ID:
+        return get_buf(InspectionBuffer::IBT_VBA, p, b);
+    case POP_JS_DATA_ID:
+        return get_buf(InspectionBuffer::IBT_JS_DATA, p, b);
+    default:
+        return false;
+    }
 }
 
 //-------------------------------------------------------------------------
diff --git a/src/service_inspectors/s7commplus/ips_s7comm_content.cc b/src/service_inspectors/s7commplus/ips_s7comm_content.cc
index 15405ab3f..5756e36a5 100644
--- a/src/service_inspectors/s7commplus/ips_s7comm_content.cc
+++ b/src/service_inspectors/s7commplus/ips_s7comm_content.cc
@@ -30,6 +30,7 @@
 #include "profiler/profiler.h"
 #include "protocols/packet.h"
 
+#include "s7comm.h"
 #include "s7comm_decode.h"
 
 using namespace snort;
@@ -75,16 +76,11 @@ IpsOption::EvalStatus S7commplusContentOption::eval(Cursor& c, Packet* p)
 {
     RuleProfile profile(s7commplus_content_prof);   // cppcheck-suppress unreadVariable
 
-    if ( !p->flow )
+    InspectionBuffer b;
+    if (!get_buf_s7commplus_content(p, b))
         return NO_MATCH;
 
-    if ( !p->is_full_pdu() )
-        return NO_MATCH;
-
-    if ( p->dsize < S7COMMPLUS_MIN_HDR_LEN )
-        return NO_MATCH;
-
-    c.set(s_name, p->data + S7COMMPLUS_MIN_HDR_LEN, p->dsize - S7COMMPLUS_MIN_HDR_LEN);
+    c.set(s_name, b.data, b.len);
 
     return MATCH;
 }
diff --git a/src/service_inspectors/s7commplus/s7comm.cc b/src/service_inspectors/s7commplus/s7comm.cc
index 007e430a6..e3b30005d 100644
--- a/src/service_inspectors/s7commplus/s7comm.cc
+++ b/src/service_inspectors/s7commplus/s7comm.cc
@@ -35,8 +35,21 @@
 
 using namespace snort;
 
+#define S7COMMPLUS_CONTENT_BUFID 1
+
 THREAD_LOCAL S7commplusStats s7commplus_stats;
 
+bool get_buf_s7commplus_content(Packet* p, InspectionBuffer& b)
+{
+    if ( !p->is_full_pdu() or p->dsize < S7COMMPLUS_MIN_HDR_LEN )
+        return false;
+
+    b.data = p->data + S7COMMPLUS_MIN_HDR_LEN;
+    b.len = p->dsize - S7COMMPLUS_MIN_HDR_LEN;
+    b.is_accumulated = false;
+    return true;
+}
+
 //-------------------------------------------------------------------------
 // flow stuff
 //-------------------------------------------------------------------------
@@ -76,6 +89,9 @@ public:
 
     StreamSplitter* get_splitter(bool c2s) override
     { return new S7commplusSplitter(c2s); }
+
+    bool get_buf(unsigned id, snort::Packet* p, snort::InspectionBuffer& b) override
+    { return ( id == S7COMMPLUS_CONTENT_BUFID ) ? get_buf_s7commplus_content(p, b) : false; }
 };
 
 void S7commplus::eval(Packet* p)
@@ -113,7 +129,7 @@ void S7commplus::eval(Packet* p)
     // evaluating on the first PDU. Setting this flag stops the caching.
     p->packet_flags |= PKT_ALLOW_MULTIPLE_DETECT;
 
-    if ( !S7commplusDecode(p, mfd))
+    if ( !S7commplusDecode(p, mfd) )
         mfd->reset();
 }
 
diff --git a/src/service_inspectors/s7commplus/s7comm.h b/src/service_inspectors/s7commplus/s7comm.h
index b77b936ed..2a0a7db9e 100644
--- a/src/service_inspectors/s7commplus/s7comm.h
+++ b/src/service_inspectors/s7commplus/s7comm.h
@@ -73,6 +73,7 @@ int get_message_type(int version, const char* name);
 int get_info_type(int version, const char* name);
 
 extern THREAD_LOCAL S7commplusStats s7commplus_stats;
+bool get_buf_s7commplus_content(snort::Packet* p, snort::InspectionBuffer& b);
 
 #endif
 
diff --git a/src/service_inspectors/sip/ips_sip.cc b/src/service_inspectors/sip/ips_sip.cc
index 58e271d41..1a1ec8f87 100644
--- a/src/service_inspectors/sip/ips_sip.cc
+++ b/src/service_inspectors/sip/ips_sip.cc
@@ -91,6 +91,7 @@ public:
     EvalStatus eval(Cursor&, Packet*) override;
 
 private:
+    unsigned get_bufid() const { return (idx == SIP_HEADER) ? SIP_HEADER_ID : SIP_BODY_ID; }
     const char* key;
     CursorActionType cat;
     SipIdx idx;
@@ -98,42 +99,14 @@ private:
 
 IpsOption::EvalStatus SipIpsOption::eval(Cursor& c, Packet* p)
 {
-    RuleProfile profile(sip_ps[idx]);
+    RuleProfile profile(sip_ps[idx]);  // cppcheck-suppress unreadVariable
 
-    if ((!p->has_tcp_data() && !p->is_udp()) || !p->flow || !p->dsize)
-        return NO_MATCH;
+    InspectionBuffer b;
+    if (!get_buf_sip(get_bufid(), p, b))
+         return NO_MATCH;
 
-    // FIXIT-P cache id at parse time for runtime use
-    SIPData* sd = get_sip_session_data(p->flow);
-
-    if (!sd)
-        return NO_MATCH;
-
-    SIP_Roptions* ropts = &sd->ropts;
-    const uint8_t* data = nullptr;
-    unsigned len = 0;
-
-    switch (idx)
-    {
-    case SIP_HEADER:
-        data = ropts->header_data;
-        len = ropts->header_len;
-        break;
-    case SIP_BODY:
-        data = ropts->body_data;
-        len = ropts->body_len;
-        break;
-    default:
-        break;
-    }
-
-    if (data != nullptr)
-    {
-        c.set(key, data, len);
-        return MATCH;
-    }
-
-    return NO_MATCH;
+    c.set(key, b.data, b.len);
+    return MATCH;
 }
 
 //-------------------------------------------------------------------------
diff --git a/src/service_inspectors/sip/ips_sip_method.cc b/src/service_inspectors/sip/ips_sip_method.cc
index bff4af3c0..ed3a62c65 100644
--- a/src/service_inspectors/sip/ips_sip_method.cc
+++ b/src/service_inspectors/sip/ips_sip_method.cc
@@ -102,7 +102,7 @@ IpsOption::EvalStatus SipMethodOption::eval(Cursor&, Packet* p)
 {
     RuleProfile profile(sipMethodRuleOptionPerfStats);  // cppcheck-suppress unreadVariable
 
-    if ( !p->flow )
+    if ((!p->has_tcp_data() && !p->is_udp()) || !p->flow || !p->dsize)
         return NO_MATCH;
 
     SIPData* sd = get_sip_session_data(p->flow);
diff --git a/src/service_inspectors/sip/sip.cc b/src/service_inspectors/sip/sip.cc
index ba3c2a824..e04afbae4 100644
--- a/src/service_inspectors/sip/sip.cc
+++ b/src/service_inspectors/sip/sip.cc
@@ -44,6 +44,33 @@ static void FreeSipData(void*);
 unsigned SipFlowData::inspector_id = 0;
 unsigned SIPData::pub_id = 0;
 
+bool get_buf_sip(unsigned id, snort::Packet* p, snort::InspectionBuffer& b)
+{
+    if (id != SIP_HEADER_ID and id != SIP_BODY_ID)
+        return false;
+
+    if ((!p->has_tcp_data() and !p->is_udp()) or !p->flow or !p->dsize)
+        return false;
+
+    if (p->has_tcp_data() and !p->is_full_pdu())
+        return false;
+
+    SIPData* sd = get_sip_session_data(p->flow);
+    if (!sd)
+        return false;
+
+    const SIP_Roptions& ropts = sd->ropts;
+    const uint8_t* data = (id == SIP_HEADER_ID) ? ropts.header_data : ropts.body_data;
+    unsigned len = (id == SIP_HEADER_ID) ? ropts.header_len : ropts.body_len;
+    if (!data or !len)
+        return false;
+
+    b.data = data;
+    b.len = len;
+    b.is_accumulated = false;
+    return true;
+}
+
 SipFlowData::SipFlowData() : FlowData(inspector_id)
 {
     memset(&session, 0, sizeof(session));
@@ -115,7 +142,7 @@ static inline int SIP_Process(Packet* p, SIPData* sessp, SIP_PROTO_CONF* config)
     bool status;
     const char* sip_buff = (const char*)p->data;
     const char* end;
-    SIP_Roptions* pRopts;
+    SIP_Roptions* pRopts = &(sessp->ropts);
     SIPMsg sipMsg;
 
     memset(&sipMsg, 0, SIPMSG_ZERO_LEN);
@@ -127,13 +154,13 @@ static inline int SIP_Process(Packet* p, SIPData* sessp, SIP_PROTO_CONF* config)
 
     status = sip_parse(&sipMsg, sip_buff, end, config);
 
+    memset(pRopts, 0, sizeof(*pRopts));
     if (true == status)
     {
         /*Update the dialog state*/
         SIP_updateDialog(sipMsg, &(sessp->dialogs), p, config);
     }
     /*Update the session data*/
-    pRopts = &(sessp->ropts);
     pRopts->method_data = sipMsg.method;
     pRopts->method_len = sipMsg.methodLen;
     pRopts->header_data = sipMsg.header;
@@ -194,6 +221,9 @@ public:
     bool is_control_channel() const override
     { return true; }
 
+    bool get_buf(unsigned id, snort::Packet* p, snort::InspectionBuffer& b) override
+    { return get_buf_sip(id, p, b); }
+
 private:
     SIP_PROTO_CONF* config;
 };
diff --git a/src/service_inspectors/sip/sip.h b/src/service_inspectors/sip/sip.h
index a89cd38ea..6b7d50271 100644
--- a/src/service_inspectors/sip/sip.h
+++ b/src/service_inspectors/sip/sip.h
@@ -57,4 +57,13 @@ public:
 SIPData* get_sip_session_data(const snort::Flow*);
 SIPMethodNode* add_sip_method(const char*);
 
+// Indices in the buffer array exposed by InspectApi
+// Must remain synchronized with sip_bufs
+enum SipBufId
+{
+    SIP_HEADER_ID = 1, SIP_BODY_ID
+};
+
+bool get_buf_sip(unsigned id, snort::Packet* p, snort::InspectionBuffer& b);
+
 #endif
diff --git a/src/service_inspectors/smtp/smtp.cc b/src/service_inspectors/smtp/smtp.cc
index 337533629..cdcff88b1 100644
--- a/src/service_inspectors/smtp/smtp.cc
+++ b/src/service_inspectors/smtp/smtp.cc
@@ -49,6 +49,13 @@
 
 using namespace snort;
 
+// Indices in the buffer array exposed by InspectApi
+// Must remain synchronized with smtp_bufs
+enum SmtpBufId
+{
+    SMTP_FILE_DATA_ID = 1, SMTP_VBA_DATA_ID, SMTP_JS_DATA_ID
+};
+
 THREAD_LOCAL ProfileStats smtpPerfStats;
 THREAD_LOCAL SmtpStats smtpstats;
 THREAD_LOCAL bool smtp_normalizing;
@@ -1531,7 +1538,8 @@ public:
 
     void ProcessSmtpCmdsList(const SmtpCmd*);
 
-    bool get_fp_buf(snort::InspectionBuffer::Type, snort::Packet*, snort::InspectionBuffer&) override;
+    bool get_buf(snort::InspectionBuffer::Type, snort::Packet*, snort::InspectionBuffer&) override;
+    bool get_buf(unsigned id, snort::Packet* p, snort::InspectionBuffer& b) override;
 
 private:
     SmtpProtoConf* config;
@@ -1623,7 +1631,7 @@ void Smtp::ProcessSmtpCmdsList(const SmtpCmd* sc)
         config->cmd_config[id].max_line_len = sc->number;
 }
 
-bool Smtp::get_fp_buf(InspectionBuffer::Type ibt, Packet* p, InspectionBuffer& b)
+bool Smtp::get_buf(InspectionBuffer::Type ibt, Packet* p, InspectionBuffer& b)
 {
     SMTPData* smtp_ssn = get_session_data(p->flow);
 
@@ -1667,6 +1675,21 @@ bool Smtp::get_fp_buf(InspectionBuffer::Type ibt, Packet* p, InspectionBuffer& b
     return dst && dst_len;
 }
 
+bool Smtp::get_buf(unsigned id, snort::Packet* p, snort::InspectionBuffer& b)
+{
+    switch (id)
+    {
+    case SMTP_FILE_DATA_ID:
+        return false;
+    case SMTP_VBA_DATA_ID:
+        return get_buf(InspectionBuffer::IBT_VBA, p, b);
+    case SMTP_JS_DATA_ID:
+        return get_buf(InspectionBuffer::IBT_JS_DATA, p, b);
+    default:
+        return false;
+    }
+}
+
 //-------------------------------------------------------------------------
 // api stuff
 //-------------------------------------------------------------------------