From: Philippe Antoine <contact@catenacyber.fr>
Date: Mon, 22 Nov 2021 08:13:54 +0000 (+0100)
Subject: dnp3: fix int warnings
X-Git-Tag: suricata-7.0.0-beta1~1153
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=86b5c81ea2606de66bf08b00fdfb8ec0735e7df4;p=thirdparty%2Fsuricata.git

dnp3: fix int warnings

There is a hack to know the type of an integer
and do an explicit cast in the python script
generating the C file

Also extends some bounds check against negative values
---

diff --git a/scripts/dnp3-gen/dnp3-gen.py b/scripts/dnp3-gen/dnp3-gen.py
index 4a308de6f9..9a9c730677 100755
--- a/scripts/dnp3-gen/dnp3-gen.py
+++ b/scripts/dnp3-gen/dnp3-gen.py
@@ -501,7 +501,10 @@ static int DNP3DecodeObjectG{{object.group}}V{{object.variation}}(const uint8_t
         *len -= 4;
 {% elif field.type == "bytearray" %}
 {% if field.len_from_prefix %}
-        object->{{field.len_field}} = prefix - (offset - *len);
+        if (prefix < (offset - *len)) {
+            goto error;
+        }
+        object->{{field.len_field}} = (uint16_t) (prefix - (offset - *len));
 {% endif %}
         if (object->{{field.len_field}} > 0) {
             if (*len < object->{{field.len_field}}) {
@@ -518,10 +521,14 @@ static int DNP3DecodeObjectG{{object.group}}V{{object.variation}}(const uint8_t
         }
 {% elif field.type == "chararray" %}
 {% if field.len_from_prefix %}
-        if (prefix - (offset - *len) >= {{field.size}}) {
+        if (prefix - (offset - *len) >= {{field.size}} || prefix < (offset - *len)) {
             goto error;
         }
-        object->{{field.len_field}} = prefix - (offset - *len);
+{% if field.size == 255 %}
+        object->{{field.len_field}} = (uint8_t) (prefix - (offset - *len));
+{% else %}
+        object->{{field.len_field}} = (uint16_t) (prefix - (offset - *len));
+{% endif %}
 {% endif %}
         if (object->{{field.len_field}} > 0) {
             if (*len < object->{{field.len_field}}) {
diff --git a/src/app-layer-dnp3-objects.c b/src/app-layer-dnp3-objects.c
index a0159ac1d4..9c3d9a53c9 100644
--- a/src/app-layer-dnp3-objects.c
+++ b/src/app-layer-dnp3-objects.c
@@ -7153,10 +7153,10 @@ static int DNP3DecodeObjectG70V4(const uint8_t **buf, uint32_t *len,
         if (!DNP3ReadUint8(buf, len, &object->status_code)) {
             goto error;
         }
-        if (prefix - (offset - *len) >= 255) {
+        if (prefix - (offset - *len) >= 255 || prefix < (offset - *len)) {
             goto error;
         }
-        object->optional_text_len = prefix - (offset - *len);
+        object->optional_text_len = (uint8_t)(prefix - (offset - *len));
         if (object->optional_text_len > 0) {
             if (*len < object->optional_text_len) {
                 /* Not enough data. */
@@ -7220,10 +7220,10 @@ static int DNP3DecodeObjectG70V5(const uint8_t **buf, uint32_t *len,
         if (!DNP3ReadUint32(buf, len, &object->block_number)) {
             goto error;
         }
-        if (prefix - (offset - *len) >= 255) {
+        if (prefix - (offset - *len) >= 255 || prefix < (offset - *len)) {
             goto error;
         }
-        object->file_data_len = prefix - (offset - *len);
+        object->file_data_len = (uint8_t)(prefix - (offset - *len));
         if (object->file_data_len > 0) {
             if (*len < object->file_data_len) {
                 /* Not enough data. */
@@ -7290,10 +7290,10 @@ static int DNP3DecodeObjectG70V6(const uint8_t **buf, uint32_t *len,
         if (!DNP3ReadUint8(buf, len, &object->status_code)) {
             goto error;
         }
-        if (prefix - (offset - *len) >= 255) {
+        if (prefix - (offset - *len) >= 255 || prefix < (offset - *len)) {
             goto error;
         }
-        object->optional_text_len = prefix - (offset - *len);
+        object->optional_text_len = (uint8_t)(prefix - (offset - *len));
         if (object->optional_text_len > 0) {
             if (*len < object->optional_text_len) {
                 /* Not enough data. */
@@ -7422,10 +7422,10 @@ static int DNP3DecodeObjectG70V8(const uint8_t **buf, uint32_t *len,
 
         offset = *len;
 
-        if (prefix - (offset - *len) >= 65535) {
+        if (prefix - (offset - *len) >= 65535 || prefix < (offset - *len)) {
             goto error;
         }
-        object->file_specification_len = prefix - (offset - *len);
+        object->file_specification_len = (uint16_t)(prefix - (offset - *len));
         if (object->file_specification_len > 0) {
             if (*len < object->file_specification_len) {
                 /* Not enough data. */
@@ -7764,7 +7764,10 @@ static int DNP3DecodeObjectG120V1(const uint8_t **buf, uint32_t *len,
         if (!DNP3ReadUint8(buf, len, &object->reason)) {
             goto error;
         }
-        object->challenge_data_len = prefix - (offset - *len);
+        if (prefix < (offset - *len)) {
+            goto error;
+        }
+        object->challenge_data_len = (uint16_t)(prefix - (offset - *len));
         if (object->challenge_data_len > 0) {
             if (*len < object->challenge_data_len) {
                 /* Not enough data. */
@@ -7834,7 +7837,10 @@ static int DNP3DecodeObjectG120V2(const uint8_t **buf, uint32_t *len,
         if (!DNP3ReadUint16(buf, len, &object->usr)) {
             goto error;
         }
-        object->mac_value_len = prefix - (offset - *len);
+        if (prefix < (offset - *len)) {
+            goto error;
+        }
+        object->mac_value_len = (uint16_t)(prefix - (offset - *len));
         if (object->mac_value_len > 0) {
             if (*len < object->mac_value_len) {
                 /* Not enough data. */
@@ -8018,7 +8024,10 @@ static int DNP3DecodeObjectG120V5(const uint8_t **buf, uint32_t *len,
             *buf += object->challenge_data_len;
             *len -= object->challenge_data_len;
         }
-        object->mac_value_len = prefix - (offset - *len);
+        if (prefix < (offset - *len)) {
+            goto error;
+        }
+        object->mac_value_len = (uint16_t)(prefix - (offset - *len));
         if (object->mac_value_len > 0) {
             if (*len < object->mac_value_len) {
                 /* Not enough data. */
@@ -8091,7 +8100,10 @@ static int DNP3DecodeObjectG120V6(const uint8_t **buf, uint32_t *len,
         if (!DNP3ReadUint16(buf, len, &object->usr)) {
             goto error;
         }
-        object->wrapped_key_data_len = prefix - (offset - *len);
+        if (prefix < (offset - *len)) {
+            goto error;
+        }
+        object->wrapped_key_data_len = (uint16_t)(prefix - (offset - *len));
         if (object->wrapped_key_data_len > 0) {
             if (*len < object->wrapped_key_data_len) {
                 /* Not enough data. */
@@ -8170,10 +8182,10 @@ static int DNP3DecodeObjectG120V7(const uint8_t **buf, uint32_t *len,
         if (!DNP3ReadUint48(buf, len, &object->time_of_error)) {
             goto error;
         }
-        if (prefix - (offset - *len) >= 65535) {
+        if (prefix - (offset - *len) >= 65535 || prefix < (offset - *len)) {
             goto error;
         }
-        object->error_text_len = prefix - (offset - *len);
+        object->error_text_len = (uint16_t)(prefix - (offset - *len));
         if (object->error_text_len > 0) {
             if (*len < object->error_text_len) {
                 /* Not enough data. */
@@ -8237,7 +8249,10 @@ static int DNP3DecodeObjectG120V8(const uint8_t **buf, uint32_t *len,
         if (!DNP3ReadUint8(buf, len, &object->certificate_type)) {
             goto error;
         }
-        object->certificate_len = prefix - (offset - *len);
+        if (prefix < (offset - *len)) {
+            goto error;
+        }
+        object->certificate_len = (uint16_t)(prefix - (offset - *len));
         if (object->certificate_len > 0) {
             if (*len < object->certificate_len) {
                 /* Not enough data. */
@@ -8297,7 +8312,10 @@ static int DNP3DecodeObjectG120V9(const uint8_t **buf, uint32_t *len,
 
         offset = *len;
 
-        object->mac_value_len = prefix - (offset - *len);
+        if (prefix < (offset - *len)) {
+            goto error;
+        }
+        object->mac_value_len = (uint16_t)(prefix - (offset - *len));
         if (object->mac_value_len > 0) {
             if (*len < object->mac_value_len) {
                 /* Not enough data. */
@@ -8688,7 +8706,10 @@ static int DNP3DecodeObjectG120V14(const uint8_t **buf, uint32_t *len,
 
         offset = *len;
 
-        object->digital_signature_len = prefix - (offset - *len);
+        if (prefix < (offset - *len)) {
+            goto error;
+        }
+        object->digital_signature_len = (uint16_t)(prefix - (offset - *len));
         if (object->digital_signature_len > 0) {
             if (*len < object->digital_signature_len) {
                 /* Not enough data. */
@@ -8752,7 +8773,10 @@ static int DNP3DecodeObjectG120V15(const uint8_t **buf, uint32_t *len,
 
         offset = *len;
 
-        object->mac_len = prefix - (offset - *len);
+        if (prefix < (offset - *len)) {
+            goto error;
+        }
+        object->mac_len = (uint16_t)(prefix - (offset - *len));
         if (object->mac_len > 0) {
             if (*len < object->mac_len) {
                 /* Not enough data. */
diff --git a/src/app-layer-dnp3.c b/src/app-layer-dnp3.c
index 45be2dc2a4..e1ff4d1dba 100644
--- a/src/app-layer-dnp3.c
+++ b/src/app-layer-dnp3.c
@@ -97,9 +97,6 @@ SCEnumCharMap dnp3_decoder_event_table[] = {
     {NULL, -1},
 };
 
-/* Some DNP3 servers start with a banner. */
-static const char banner[] = "DNP3";
-
 /* Calculate the next transport sequence number. */
 #define NEXT_TH_SEQNO(current) ((current + 1) % DNP3_MAX_TRAN_SEQNO)
 
@@ -251,6 +248,9 @@ static int DNP3CheckStartBytes(const DNP3LinkHeader *header)
         header->start_byte1 == DNP3_START_BYTE1;
 }
 
+/* Some DNP3 servers start with a banner. */
+#define DNP3_BANNER "DNP3"
+
 /**
  * \brief Check if a frame contains a banner.
  *
@@ -261,7 +261,7 @@ static int DNP3CheckStartBytes(const DNP3LinkHeader *header)
  */
 static int DNP3ContainsBanner(const uint8_t *input, uint32_t len)
 {
-    return BasicSearch(input, len, (uint8_t *)banner, strlen(banner)) != NULL;
+    return BasicSearch(input, len, (uint8_t *)DNP3_BANNER, strlen(DNP3_BANNER)) != NULL;
 }
 
 /**