From: Masud Hasan (mashasan) <mashasan@cisco.com>
Date: Wed, 19 Jan 2022 21:44:24 +0000 (+0000)
Subject: Pull request #3237: Single finish2
X-Git-Tag: 3.1.21.0~6
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=67ca7d4c59fc2c50d44ea2c1843c446e0e4c1b84;p=thirdparty%2Fsnort3.git

Pull request #3237: Single finish2

Merge in SNORT/snort3 from ~SMINUT/snort3:single_finish2 to master

Squashed commit of the following:

commit 56d6b7e2091d7752f955af1a2d4cc97c18e19bd0
Author: Silviu Minut <sminut@cisco.com>
Date:   Thu Jan 13 20:15:50 2022 -0500

    stream_tcp: ensure that we call splitter finish() only once per flow, per direction
---

diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc
index c8ffe48ee..f46f3defe 100644
--- a/src/stream/tcp/tcp_reassembler.cc
+++ b/src/stream/tcp/tcp_reassembler.cc
@@ -787,7 +787,7 @@ void TcpReassembler::flush_queued_segments(
     TcpReassemblerState& trs, Flow* flow, bool clear, Packet* p)
 {
     bool pending = clear and paf_initialized(&trs.paf_state)
-        and (!trs.tracker->get_splitter() || trs.tracker->get_splitter()->finish(flow) );
+        and trs.tracker->splitter_finish(flow);
 
     if ( pending and !(flow->ssn_state.ignore_direction & trs.ignore_dir) )
         final_flush(trs, p, trs.packet_dir);
@@ -1145,7 +1145,7 @@ int TcpReassembler::flush_on_ack_policy(TcpReassemblerState& trs, Packet* p)
                 break;
 
             if ( trs.paf_state.paf == StreamSplitter::ABORT )
-                trs.tracker->get_splitter()->finish(p->flow);
+                trs.tracker->splitter_finish(p->flow);
 
             // for consistency with other cases, should return total
             // but that breaks flushing pipelined pdus
diff --git a/src/stream/tcp/tcp_stream_tracker.cc b/src/stream/tcp/tcp_stream_tracker.cc
index d370b008a..156f683f1 100644
--- a/src/stream/tcp/tcp_stream_tracker.cc
+++ b/src/stream/tcp/tcp_stream_tracker.cc
@@ -222,6 +222,7 @@ void TcpStreamTracker::init_tcp_state()
     held_packet = null_iterator;
     flush_policy = STREAM_FLPOLICY_IGNORE;
     reassembler.reset();
+    splitter_finish_flag = false;
 }
 
 //-------------------------------------------------------------------------
@@ -264,6 +265,21 @@ void TcpStreamTracker::set_splitter(const Flow* flow)
         set_splitter(new AtomSplitter(!client_tracker) );
 }
 
+bool TcpStreamTracker::splitter_finish(snort::Flow* flow)
+{
+    if (!splitter)
+        return true;
+
+    if (!splitter_finish_flag)
+    {
+        splitter_finish_flag = true;
+        return splitter->finish(flow);
+    }
+    // there shouldn't be any un-flushed data beyond this point,
+    // returning false here, discards it
+    return false;
+}
+
 void TcpStreamTracker::init_on_syn_sent(TcpSegmentDescriptor& tsd)
 {
     tsd.get_flow()->set_session_flags(SSNFLAG_SEEN_CLIENT);
diff --git a/src/stream/tcp/tcp_stream_tracker.h b/src/stream/tcp/tcp_stream_tracker.h
index b0f0708fc..5d5b8a5c1 100644
--- a/src/stream/tcp/tcp_stream_tracker.h
+++ b/src/stream/tcp/tcp_stream_tracker.h
@@ -261,6 +261,8 @@ public:
     bool is_splitter_paf() const
     { return splitter && splitter->is_paf(); }
 
+    bool splitter_finish(snort::Flow* flow);
+
     bool is_reassembly_enabled() const
     { return  ( splitter and (flush_policy != STREAM_FLPOLICY_IGNORE) ); }
 
@@ -354,6 +356,7 @@ protected:
     FlushPolicy flush_policy = STREAM_FLPOLICY_IGNORE;
     bool mac_addr_valid = false;
     bool fin_seq_set = false;  // FIXIT-M should be obviated by tcp state
+    bool splitter_finish_flag = false;
 };
 
 // <--- note -- the 'state' parameter must be a reference