From: Philippe Antoine <contact@catenacyber.fr>
Date: Tue, 9 Mar 2021 14:54:16 +0000 (+0100)
Subject: enip: improve probing parser
X-Git-Tag: suricata-6.0.4~53
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=209feffcdac558893ea474b60aea7679d2818762;p=thirdparty%2Fsuricata.git

enip: improve probing parser

Strict length for register sessions
NOP command must have options=0

(cherry picked from commit 0c948142b93a2de0ede0a65e6ddb650f2a2239bc)
---

diff --git a/src/app-layer-enip.c b/src/app-layer-enip.c
index 0bb841d67c..99ce6d3ce6 100644
--- a/src/app-layer-enip.c
+++ b/src/app-layer-enip.c
@@ -368,7 +368,7 @@ static AppLayerResult ENIPParse(Flow *f, void *state, AppLayerParserState *pstat
     SCReturnStruct(APP_LAYER_OK);
 }
 
-
+#define ENIP_LEN_REGISTER_SESSION 4 // protocol u16, options u16
 
 static uint16_t ENIPProbingParser(Flow *f, uint8_t direction,
         const uint8_t *input, uint32_t input_len, uint8_t *rdir)
@@ -380,43 +380,90 @@ static uint16_t ENIPProbingParser(Flow *f, uint8_t direction,
         return ALPROTO_UNKNOWN;
     }
     uint16_t cmd;
+    uint16_t enip_len;
     uint32_t status;
-    int ret = ByteExtractUint16(&cmd, BYTE_LITTLE_ENDIAN, sizeof(uint16_t),
-                                (const uint8_t *) (input));
+    uint32_t option;
+    uint16_t nbitems;
+
+    int ret = ByteExtractUint16(
+            &enip_len, BYTE_LITTLE_ENDIAN, sizeof(uint16_t), (const uint8_t *)(input + 2));
+    if (ret < 0) {
+        return ALPROTO_FAILED;
+    }
+    if (enip_len < sizeof(ENIPEncapHdr)) {
+        return ALPROTO_FAILED;
+    }
+    ret = ByteExtractUint32(
+            &status, BYTE_LITTLE_ENDIAN, sizeof(uint32_t), (const uint8_t *)(input + 8));
+    if (ret < 0) {
+        return ALPROTO_FAILED;
+    }
+    switch (status) {
+        case SUCCESS:
+        case INVALID_CMD:
+        case NO_RESOURCES:
+        case INCORRECT_DATA:
+        case INVALID_SESSION:
+        case INVALID_LENGTH:
+        case UNSUPPORTED_PROT_REV:
+        case ENCAP_HEADER_ERROR:
+            break;
+        default:
+            return ALPROTO_FAILED;
+    }
+    ret = ByteExtractUint16(&cmd, BYTE_LITTLE_ENDIAN, sizeof(uint16_t), (const uint8_t *)(input));
     if(ret < 0) {
         return ALPROTO_FAILED;
     }
+    ret = ByteExtractUint32(
+            &option, BYTE_LITTLE_ENDIAN, sizeof(uint32_t), (const uint8_t *)(input + 20));
+    if (ret < 0) {
+        return ALPROTO_FAILED;
+    }
+
     //ok for all the known commands
     switch(cmd) {
         case NOP:
-        case LIST_SERVICES:
-        case LIST_IDENTITY:
-        case LIST_INTERFACES:
+            if (option != 0) {
+                return ALPROTO_FAILED;
+            }
+            break;
         case REGISTER_SESSION:
+            if (enip_len != ENIP_LEN_REGISTER_SESSION) {
+                return ALPROTO_FAILED;
+            }
+            break;
         case UNREGISTER_SESSION:
+            if (enip_len != ENIP_LEN_REGISTER_SESSION && enip_len != 0) {
+                // 0 for request and 4 for response
+                return ALPROTO_FAILED;
+            }
+            break;
+        case LIST_SERVICES:
+        case LIST_IDENTITY:
         case SEND_RR_DATA:
         case SEND_UNIT_DATA:
         case INDICATE_STATUS:
         case CANCEL:
-            ret = ByteExtractUint32(&status, BYTE_LITTLE_ENDIAN,
-                                    sizeof(uint32_t),
-                                    (const uint8_t *) (input + 8));
+            break;
+        case LIST_INTERFACES:
+            if (input_len < sizeof(ENIPEncapHdr) + 2) {
+                SCLogDebug("length too small to be a ENIP LIST_INTERFACES");
+                return ALPROTO_UNKNOWN;
+            }
+            ret = ByteExtractUint16(
+                    &nbitems, BYTE_LITTLE_ENDIAN, sizeof(uint16_t), (const uint8_t *)(input));
             if(ret < 0) {
                 return ALPROTO_FAILED;
             }
-            switch(status) {
-                case SUCCESS:
-                case INVALID_CMD:
-                case NO_RESOURCES:
-                case INCORRECT_DATA:
-                case INVALID_SESSION:
-                case INVALID_LENGTH:
-                case UNSUPPORTED_PROT_REV:
-                case ENCAP_HEADER_ERROR:
-                    return ALPROTO_ENIP;
+            if (enip_len < sizeof(ENIPEncapHdr) + 2 * (size_t)nbitems) {
+                return ALPROTO_FAILED;
             }
+            break;
+        default:
+            return ALPROTO_FAILED;
     }
-    return ALPROTO_FAILED;
+    return ALPROTO_ENIP;
 }
 
 /**