From: Victor Julien <victor@inliniac.net>
Date: Tue, 30 Aug 2016 17:35:18 +0000 (+0200)
Subject: detect: fix ICMP error handling issue
X-Git-Tag: suricata-3.1.2~23
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6b078e4f51800ac4cba3660dedfe210474491bc6;p=thirdparty%2Fsuricata.git

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.
---

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;