From 6b078e4f51800ac4cba3660dedfe210474491bc6 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 30 Aug 2016 19:35:18 +0200 Subject: [PATCH] detect: fix ICMP error handling issue The first packet in both directions of a flow looks up the rule group (sgh) and stores it in the flow. This makes sure the lookup doesn't have to be performed for each packet. ICMPv4 error messages are connected to the TCP or UDP flow they apply to. In the case of such an ICMP error being the first packet in a flow's direction, this would lead to an issue. The packet would look up the rule group based on the ICMP protocol, not based on the embedded TCP/UDP. This makes sense, as the ICMP packet is inspected as ICMP packet. The consequence however, was that this rule group pointer (sgh) would be stored in the flow. This is wrong, as TCP/UDP packets that follow the ICMP packet would have no sgh or the wrong sgh. In normal traffic this shouldn't normally happen, but it could be used to evade Suricata's inspection. --- src/detect.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/detect.c b/src/detect.c index a507e4716d..3ae6e0f860 100644 --- a/src/detect.c +++ b/src/detect.c @@ -1816,7 +1816,12 @@ end: } } - if (!(sms_runflags & SMS_USE_FLOW_SGH)) { + /* HACK: prevent the wrong sgh (or NULL) from being stored in the + * flow's sgh pointers */ + if (PKT_IS_ICMPV4(p) && ICMPV4_DEST_UNREACH_IS_VALID(p)) { + ; /* no-op */ + + } else if (!(sms_runflags & SMS_USE_FLOW_SGH)) { if ((p->flowflags & FLOW_PKT_TOSERVER) && !(pflow->flags & FLOW_SGH_TOSERVER)) { /* first time we see this toserver sgh, store it */ pflow->sgh_toserver = det_ctx->sgh;