]> 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>
Fri, 24 Feb 2023 09:45:06 +0000 (10:45 +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.

src/stream-tcp.c

index cfdc4b7ec7b3e107143d0b9f7298177ef8571a86..89790511ae3c06001f27f1846225e32c9433f92c 100644 (file)
@@ -3557,27 +3557,36 @@ static int StreamTcpPacketStateFinWait1(
                 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));