From: Victor Julien <vjulien@oisf.net>
Date: Sat, 11 Feb 2023 12:14:53 +0000 (+0100)
Subject: stream/tcp: fix wrong ACK trigger FIN1 to FIN2
X-Git-Tag: suricata-6.0.11~63
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=237e06a991fcc46145e809d747686afaa545d29c;p=thirdparty%2Fsuricata.git

stream/tcp: fix wrong ACK trigger FIN1 to FIN2

An ACK that ACK'd older data while still being in-window could
lead to FIN_WAIT1 to FIN_WAIT2 state transition. Detect this
case and generally harden the check.

Bug: #5877.
(cherry picked from commit 67af94f2e07352807cfedb722ea8c79e156da5fc)
---

diff --git a/src/stream-tcp.c b/src/stream-tcp.c
index cba69eba1d..8b5be8669b 100644
--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@@ -3215,27 +3215,36 @@ static int StreamTcpPacketStateFinWait1(ThreadVars *tv, Packet *p,
                 return -1;
             }
 
-            if (!retransmission) {
-                if (SEQ_LEQ(TCP_GET_SEQ(p) + p->payload_len, ssn->client.next_win) ||
-                        (ssn->flags & (STREAMTCP_FLAG_MIDSTREAM|STREAMTCP_FLAG_ASYNC)))
-                {
-                    SCLogDebug("ssn %p: seq %"PRIu32" in window, ssn->client.next_win "
-                            "%" PRIu32 "", ssn, TCP_GET_SEQ(p), ssn->client.next_win);
+            if (SEQ_LT(TCP_GET_ACK(p), ssn->server.next_seq)) {
+                SCLogDebug("ssn %p: ACK's older segment as %u < %u", ssn, TCP_GET_ACK(p),
+                        ssn->server.next_seq);
+                retransmission = 1;
+            }
 
-                    if (TCP_GET_SEQ(p) == ssn->client.next_seq) {
-                        StreamTcpPacketSetState(p, ssn, TCP_FIN_WAIT2);
-                        SCLogDebug("ssn %p: state changed to TCP_FIN_WAIT2", ssn);
+            if (!retransmission) {
+                if (SEQ_EQ(TCP_GET_ACK(p), ssn->server.next_seq + 1)) {
+                    if (SEQ_LEQ(TCP_GET_SEQ(p) + p->payload_len, ssn->client.next_win) ||
+                            (ssn->flags & (STREAMTCP_FLAG_MIDSTREAM | STREAMTCP_FLAG_ASYNC))) {
+                        SCLogDebug("ssn %p: seq %" PRIu32 " in window, ssn->client.next_win "
+                                   "%" PRIu32 "",
+                                ssn, TCP_GET_SEQ(p), ssn->client.next_win);
+                        SCLogDebug(
+                                "seq %u client.next_seq %u", TCP_GET_SEQ(p), ssn->client.next_seq);
+                        if (TCP_GET_SEQ(p) == ssn->client.next_seq) {
+                            StreamTcpPacketSetState(p, ssn, TCP_FIN_WAIT2);
+                            SCLogDebug("ssn %p: state changed to TCP_FIN_WAIT2", ssn);
+                        }
+                    } else {
+                        SCLogDebug("ssn %p: -> SEQ mismatch, packet SEQ %" PRIu32 ""
+                                   " != %" PRIu32 " from stream",
+                                ssn, TCP_GET_SEQ(p), ssn->client.next_seq);
+
+                        StreamTcpSetEvent(p, STREAM_FIN1_ACK_WRONG_SEQ);
+                        return -1;
                     }
-                } else {
-                    SCLogDebug("ssn %p: -> SEQ mismatch, packet SEQ %" PRIu32 ""
-                            " != %" PRIu32 " from stream", ssn,
-                            TCP_GET_SEQ(p), ssn->client.next_seq);
 
-                    StreamTcpSetEvent(p, STREAM_FIN1_ACK_WRONG_SEQ);
-                    return -1;
+                    ssn->server.window = TCP_GET_WINDOW(p) << ssn->server.wscale;
                 }
-
-                ssn->server.window = TCP_GET_WINDOW(p) << ssn->server.wscale;
             }
 
             StreamTcpUpdateLastAck(ssn, &ssn->server, TCP_GET_ACK(p));