From: Simon Dugas <simon.dugas@cyber.gc.ca>
Date: Thu, 11 Feb 2021 20:49:06 +0000 (-0500)
Subject: modbus: fix app-layer test cases
X-Git-Tag: suricata-7.0.0-beta1~1643
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7c99fe3689d43858cd18524e1161f59cc306b803;p=thirdparty%2Fsuricata.git

modbus: fix app-layer test cases

invalidFunctionCode: make protocol id valid since we are only testing
the function code here.

readCoilsErrorRsp: changed to different invalid response code.

ModbusParserTest10: wrong length was passed to AppLayerParserParse.

ModbusParserTest11: allocate the entire buffer.
---

diff --git a/src/app-layer-modbus.c b/src/app-layer-modbus.c
index 3840b15ff0..43da199a46 100644
--- a/src/app-layer-modbus.c
+++ b/src/app-layer-modbus.c
@@ -1557,11 +1557,13 @@ void RegisterModbusParsers(void)
 #include "stream-tcp-private.h"
 
 /* Modbus Application Protocol Specification V1.1b3 6.1: Read Coils */
-static uint8_t invalidFunctionCode[] = {/* Transaction ID */    0x00, 0x00,
-                                         /* Protocol ID */       0x00, 0x01,
-                                         /* Length */            0x00, 0x02,
-                                         /* Unit ID */           0x00,
-                                         /* Function code */     0x00};
+static uint8_t invalidFunctionCode[] = {
+    /* Transaction ID */ 0x00, 0x00,
+    /* Protocol ID */ 0x00, 0x00,
+    /* Length */ 0x00, 0x02,
+    /* Unit ID */ 0x00,
+    /* Function code */ 0x00
+};
 
 /* Modbus Application Protocol Specification V1.1b3 6.1: Read Coils */
 /* Example of a request to read discrete outputs 20-38 */
@@ -1581,12 +1583,15 @@ static uint8_t readCoilsRsp[] = {/* Transaction ID */    0x00, 0x00,
                                  /* Byte count */        0x03,
                                  /* Coil Status */       0xCD, 0x6B, 0x05 };
 
-static uint8_t readCoilsErrorRsp[] = {/* Transaction ID */    0x00, 0x00,
-                                      /* Protocol ID */       0x00, 0x00,
-                                      /* Length */            0x00, 0x03,
-                                      /* Unit ID */           0x00,
-                                      /* Function code */     0x81,
-                                      /* Exception code */    0x05};
+static uint8_t readCoilsErrorRsp[] = {
+    /* Transaction ID */ 0x00, 0x00,
+    /* Protocol ID */ 0x00, 0x00,
+    /* Length */ 0x00, 0x03,
+    /* Unit ID */ 0x00,
+    /* Function code */ 0x81,
+    /* Invalid Exception code: should trigger the InvalidExceptionCode ModbusEvent */
+    0xFF
+};
 
 /* Modbus Application Protocol Specification V1.1b3 6.6: Write Single register */
 /* Example of a request to write register 2 to 00 03 hex */
@@ -2401,8 +2406,7 @@ static int ModbusParserTest10(void) {
     memcpy(input + sizeof(readCoilsRsp), writeMultipleRegistersRsp, sizeof(writeMultipleRegistersRsp));
 
     FLOWLOCK_WRLOCK(&f);
-    r = AppLayerParserParse(NULL, alp_tctx, &f, ALPROTO_MODBUS,
-                            STREAM_TOCLIENT, input, sizeof(input_len));
+    r = AppLayerParserParse(NULL, alp_tctx, &f, ALPROTO_MODBUS, STREAM_TOCLIENT, input, input_len);
     FAIL_IF_NOT(r == 0);
     FLOWLOCK_UNLOCK(&f);
 
@@ -2423,6 +2427,14 @@ static int ModbusParserTest11(void) {
     TcpSession ssn;
     ThreadVars tv;
 
+    size_t input_len = 65536;
+    uint8_t *input = SCCalloc(1, input_len);
+
+    FAIL_IF(input == NULL);
+
+    memcpy(input, exceededLengthWriteMultipleRegistersReq,
+            sizeof(exceededLengthWriteMultipleRegistersReq));
+
     FAIL_IF(alp_tctx == NULL);
 
     memset(&tv, 0, sizeof(ThreadVars));
@@ -2459,10 +2471,8 @@ static int ModbusParserTest11(void) {
     DetectEngineThreadCtxInit(&tv, (void *)de_ctx, (void *)&det_ctx);
 
     FLOWLOCK_WRLOCK(&f);
-    int r = AppLayerParserParse(NULL, alp_tctx, &f, ALPROTO_MODBUS,
-                                STREAM_TOSERVER,
-                                exceededLengthWriteMultipleRegistersReq,
-                                sizeof(exceededLengthWriteMultipleRegistersReq) + 65523 * sizeof(uint8_t));
+    int r = AppLayerParserParse(
+            NULL, alp_tctx, &f, ALPROTO_MODBUS, STREAM_TOSERVER, input, input_len);
     FAIL_IF_NOT(r == 0);
     FLOWLOCK_UNLOCK(&f);
 
diff --git a/src/detect-engine-modbus.c b/src/detect-engine-modbus.c
index 1e251bdee3..6c1551457a 100644
--- a/src/detect-engine-modbus.c
+++ b/src/detect-engine-modbus.c
@@ -336,11 +336,13 @@ static uint8_t encapsulatedInterfaceTransport[] = {
                                         /* MEI Type */           0x0F,
                                         /* Data */               0x00, 0x00};
 
-static uint8_t unassigned[] = {/* Transaction ID */     0x00, 0x0A,
-                               /* Protocol ID */        0x00, 0x00,
-                               /* Length */             0x00, 0x02,
-                               /* Unit ID */            0x00,
-                               /* Function code */      0x12};
+static uint8_t unassigned[] = {
+    /* Transaction ID */ 0x00, 0x0A,
+    /* Protocol ID */ 0x00, 0x00,
+    /* Length */ 0x00, 0x02,
+    /* Unit ID */ 0x00,
+    /* Function code */ 0x3F
+};
 
 /** \test Test code function. */
 static int DetectEngineInspectModbusTest01(void)