Pull request #3739: stream_tcp: fix passive pickups with missing packets

Merge in SNORT/snort3 from ~RUCOMBS/snort3:tcp_fix to master

Squashed commit of the following:

commit 0da36c1f5a12f6d3d74447fc1afc6409f46d83a9
Author: Russ Combs <rucombs@cisco.com>
Date:   Wed Jan 18 10:25:07 2023 -0500

    stream_tcp: fix passive pickups with missing packets

    Thanks to nagmtuc and hedayat for reporting and helping debug the issue.

diff --git a/src/stream/tcp/tcp_reassemblers.h b/src/stream/tcp/tcp_reassemblers.h
index be03244f6e5e0002075eca5b0096f8821599024b..fd1d576abd5fe169fa31028ae6b66f15b5885686 100644 (file)
--- a/src/stream/tcp/tcp_reassemblers.h
+++ b/src/stream/tcp/tcp_reassemblers.h
@@ -97,6 +97,9 @@ public:
     uint32_t get_xtradata_mask() const
     { return trs.xtradata_mask; }
 
+    bool data_was_queued() const
+    { return trs.sos.total_bytes_queued > 0; }
+
     uint32_t get_seg_count() const
     { return trs.sos.seg_count; }
 

diff --git a/src/stream/tcp/tcp_stream_tracker.cc b/src/stream/tcp/tcp_stream_tracker.cc
index 9a64fbf776cff828b8f075a7f796dac844018c39..f02119afcbc7856a0aaccd07af4f6267611c1bb0 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.cc
+++ b/src/stream/tcp/tcp_stream_tracker.cc
@@ -465,12 +465,18 @@ void TcpStreamTracker::finish_server_init(TcpSegmentDescriptor& tsd)
 void TcpStreamTracker::finish_client_init(TcpSegmentDescriptor& tsd)
 {
     Flow* flow = tsd.get_flow();
-
     rcv_nxt = tsd.get_end_seq();
 
+    if ( reassembler.data_was_queued() )
+        return;  // we already have state, don't mess it up
+
     if ( !( flow->session_state & STREAM_STATE_MIDSTREAM ) )
     {
-        reassembler.set_seglist_base_seq(tsd.get_seq() + 1);
+        if ( tsd.get_tcph()->is_syn() )
+            reassembler.set_seglist_base_seq(tsd.get_seq() + 1);
+        else
+            reassembler.set_seglist_base_seq(tsd.get_seq());
+
         r_win_base = tsd.get_end_seq();
     }
     else
@@ -530,7 +536,7 @@ bool TcpStreamTracker::update_on_3whs_ack(TcpSegmentDescriptor& tsd)
 
     if ( good_ack )
     {
-        if (!irs)
+        if (!irs)  // FIXIT-L zero is a valid seq# so this kind of check is incorrect
             irs = tsd.get_seq();
         finish_client_init(tsd);
         update_tracker_ack_recv(tsd);