From: Masud Hasan (mashasan) <mashasan@cisco.com>
Date: Tue, 24 Nov 2020 15:41:57 +0000 (+0000)
Subject: Merge pull request #2631 in SNORT/snort3 from ~DAVMCPHE/snort3:stream_init_alerts_arr... 
X-Git-Tag: 3.0.3-6~36
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6fca209c46a30349d0b447231d6a452f1ae0835c;p=thirdparty%2Fsnort3.git

Merge pull request #2631 in SNORT/snort3 from ~DAVMCPHE/snort3:stream_init_alerts_array to master

Squashed commit of the following:

commit 64ec6d368b42815ad17ae05c6871490e034c80ee
Author: davis mcpherson <davmcphe@cisco.com>
Date:   Fri Nov 20 09:39:25 2020 -0500

    stream_tcp: initialize the alerts array to empty when a TcpReassembler instance is initialized or reset
---

diff --git a/src/stream/tcp/segment_overlap_editor.h b/src/stream/tcp/segment_overlap_editor.h
index ee7f0688c..a04075832 100644
--- a/src/stream/tcp/segment_overlap_editor.h
+++ b/src/stream/tcp/segment_overlap_editor.h
@@ -22,6 +22,8 @@
 #ifndef SEGMENT_OVERLAP_EDITOR_H
 #define SEGMENT_OVERLAP_EDITOR_H
 
+#include <vector>
+
 #include "normalize/normalize.h"
 #include "stream/paf.h"
 #include "tcp_segment_node.h"
@@ -68,11 +70,12 @@ struct SegmentOverlapState
     void init_soe(TcpSegmentDescriptor& tsd, TcpSegmentNode* left, TcpSegmentNode* right);
 };
 
-/* Only track a maximum number of alerts per session */
-#define MAX_SESSION_ALERTS 8
 struct StreamAlertInfo
 {
-    /* For storing alerts that have already been seen on the session */
+    StreamAlertInfo(uint32_t gid, uint32_t sid, uint32_t seq, uint32_t id, uint32_t sec)
+        : gid(gid), sid(sid), seq(seq), event_id(id), event_second(sec)
+    {}
+
     uint32_t gid;
     uint32_t sid;
     uint32_t seq;
@@ -87,8 +90,7 @@ struct TcpReassemblerState
     TcpStreamTracker* tracker;
     uint32_t flush_count;   // number of flushed queued segments
     uint32_t xtradata_mask; // extra data available to log
-    StreamAlertInfo alerts[MAX_SESSION_ALERTS];
-    uint8_t alert_count = 0;
+    std::vector<StreamAlertInfo> alerts;
     uint8_t ignore_dir;
     uint8_t packet_dir;
     bool server_side;
diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc
index 56f411bfb..590eb0478 100644
--- a/src/stream/tcp/tcp_reassembler.cc
+++ b/src/stream/tcp/tcp_reassembler.cc
@@ -221,31 +221,15 @@ void TcpReassembler::dup_reassembly_segment(
 
 bool TcpReassembler::add_alert(TcpReassemblerState& trs, uint32_t gid, uint32_t sid)
 {
-    if ( trs.alert_count >= MAX_SESSION_ALERTS)
-        return false;
-
-    StreamAlertInfo* ai = trs.alerts + trs.alert_count;
-    ai->gid = gid;
-    ai->sid = sid;
-    ai->seq = 0;
-    ai->event_id = 0;
-    ai->event_second = 0;
-
-    trs.alert_count++;
-
+    trs.alerts.emplace_back(gid, sid, 0, 0, 0);
     return true;
 }
 
 bool TcpReassembler::check_alerted(TcpReassemblerState& trs, uint32_t gid, uint32_t sid)
 {
-    for (int i = 0; i < trs.alert_count; i++)
-    {
-        /*  This is a rebuilt packet and if we've seen this alert before,
-         *  return that we have previously alerted on original packet.
-         */
-        if (trs.alerts[i].gid == gid && trs.alerts[i].sid == sid)
+    for ( auto& alert : trs.alerts )
+       if (alert.gid == gid && alert.sid == sid)
             return true;
-    }
 
     return false;
 }
@@ -256,17 +240,13 @@ int TcpReassembler::update_alert(TcpReassemblerState& trs, uint32_t gid, uint32_
     // FIXIT-M comparison of seq_num is wrong, compare value is always 0, should be seq_num of wire packet
     uint32_t seq_num = 0;
 
-    for (unsigned i = 0; i < trs.alert_count; i++)
-    {
-        StreamAlertInfo* ai = &trs.alerts[i];
-
-        if (ai->gid == gid && ai->sid == sid && SEQ_EQ(ai->seq, seq_num))
-        {
-            ai->event_id = event_id;
-            ai->event_second = event_second;
-            return 0;
-        }
-    }
+    for ( auto& alert : trs.alerts )
+       if (alert.gid == gid && alert.sid == sid && SEQ_EQ(alert.seq, seq_num))
+       {
+           alert.event_id = event_id;
+           alert.event_second = event_second;
+           return 0;
+       }
 
     return -1;
 }
@@ -275,13 +255,11 @@ void TcpReassembler::purge_alerts(TcpReassemblerState& trs)
 {
     Flow* flow = trs.sos.session->flow;
 
-    for (int i = 0; i < trs.alert_count; i++)
-    {
-        StreamAlertInfo* ai = trs.alerts + i;
-        Stream::log_extra_data(flow, trs.xtradata_mask, ai->event_id, ai->event_second);
-    }
+    for ( auto& alert : trs.alerts )
+        Stream::log_extra_data(flow, trs.xtradata_mask, alert.event_id, alert.event_second);
+
     if ( !flow->is_suspended() )
-        trs.alert_count = 0;
+        trs.alerts.clear();
 }
 
 void TcpReassembler::purge_to_seq(TcpReassemblerState& trs, uint32_t flush_seq)
diff --git a/src/stream/tcp/tcp_reassemblers.cc b/src/stream/tcp/tcp_reassemblers.cc
index 4b720f792..4b24d8fee 100644
--- a/src/stream/tcp/tcp_reassemblers.cc
+++ b/src/stream/tcp/tcp_reassemblers.cc
@@ -271,6 +271,7 @@ void TcpReassemblerPolicy::init(TcpSession* ssn, TcpStreamTracker* trk, StreamPo
 
     trs.flush_count = 0;
     trs.xtradata_mask = 0;
+    trs.alerts.clear();
 
     reassembler = TcpReassemblerFactory::get_instance(pol);
 }
diff --git a/src/stream/tcp/tcp_stream_session.cc b/src/stream/tcp/tcp_stream_session.cc
index 383b846bf..03c93b847 100644
--- a/src/stream/tcp/tcp_stream_session.cc
+++ b/src/stream/tcp/tcp_stream_session.cc
@@ -240,7 +240,7 @@ bool TcpStreamSession::add_alert(Packet* p, uint32_t gid, uint32_t sid)
 
 bool TcpStreamSession::check_alerted(Packet* p, uint32_t gid, uint32_t sid)
 {
-    // only check alerts on rebuilt packets
+    // only check for alert on wire packet if this when processing a rebuilt packet
     if ( !(p->packet_flags & PKT_REBUILT_STREAM) )
         return false;