Skip to content

Commit

Permalink
stream: fix SYN_SENT RST/FIN injection
Browse files Browse the repository at this point in the history
RST injection during the SYN_SENT state could trick Suricata into marking
a session as CLOSED. The way this was done is: using invalid TSECR value
in RST+ACK packet. The ACK was needed to force Linux into considering the
TSECR value and compare it to the TSVAL from the SYN packet.

The second works only against Windows. The client would not use a TSVAL
but the RST packet would. Windows will reject this, but Suricata considered
the RST valid and triggered the CLOSED logic.

This patch addresses both. When the SYN packet used timestamp support
the timestamp of incoming packet is validated. Otherwise, packet responding
should not have a timestamp.

Bug #3286

Reported-by: Nicolas Adba
(cherry picked from commit 9f0294f)
  • Loading branch information
victorjulien committed Dec 13, 2019
1 parent 268a79c commit ea0659d
Showing 1 changed file with 37 additions and 0 deletions.
37 changes: 37 additions & 0 deletions src/stream-tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1349,6 +1349,39 @@ static void StreamTcp3whsSynAckUpdate(TcpSession *ssn, Packet *p, TcpStateQueue
ssn->flags &=~ STREAMTCP_FLAG_4WHS;
}

/** \internal
* \brief detect timestamp anomalies when processing responses to the
* SYN packet.
* \retval true packet is ok
* \retval false packet is bad
*/
static inline bool StateSynSentValidateTimestamp(TcpSession *ssn, Packet *p)
{
/* we only care about evil server here, so skip TS packets */
if (PKT_IS_TOSERVER(p) || !(TCP_HAS_TS(p))) {
return true;
}

TcpStream *receiver_stream = &ssn->client;
uint32_t ts_echo = TCP_GET_TSECR(p);
if ((receiver_stream->flags & STREAMTCP_STREAM_FLAG_TIMESTAMP) != 0) {
if (receiver_stream->last_ts != 0 && ts_echo != 0 &&
ts_echo != receiver_stream->last_ts)
{
SCLogDebug("ssn %p: BAD TSECR echo %u recv %u", ssn,
ts_echo, receiver_stream->last_ts);
return false;
}
} else {
if (receiver_stream->last_ts == 0 && ts_echo != 0) {
SCLogDebug("ssn %p: BAD TSECR echo %u recv %u", ssn,
ts_echo, receiver_stream->last_ts);
return false;
}
}
return true;
}

/**
* \brief Function to handle the TCP_SYN_SENT state. The function handles
* SYN, SYN/ACK, RST packets and correspondingly changes the connection
Expand All @@ -1368,6 +1401,10 @@ static int StreamTcpPacketStateSynSent(ThreadVars *tv, Packet *p,
SCLogDebug("ssn %p: pkt received: %s", ssn, PKT_IS_TOCLIENT(p) ?
"toclient":"toserver");

/* check for bad responses */
if (StateSynSentValidateTimestamp(ssn, p) == false)
return -1;

/* RST */
if (p->tcph->th_flags & TH_RST) {
if (!StreamTcpValidateRst(ssn, p))
Expand Down

0 comments on commit ea0659d

Please sign in to comment.