]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
stream/tcp: fix wrong ACK trigger FIN1 to FIN2
authorVictor Julien <vjulien@oisf.net>
Sat, 11 Feb 2023 12:14:53 +0000 (13:14 +0100)
committerVictor Julien <vjulien@oisf.net>
Wed, 1 Mar 2023 14:00:56 +0000 (15:00 +0100)
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)

src/stream-tcp.c

index cba69eba1dc7ce65b923bb90c5fd56f8bc43771c..8b5be8669b9e02bac39e8ebfbf9c5dfb3ba6ad48 100644 (file)
@@ -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));