Pull request #5050: iec104: validate Type I length to prevent ASDU out-of-bounds...

Merge in SNORT/snort3 from ~YEFURMAN/snort3:CSCws05701_fix to master

Squashed commit of the following:

commit ff30b6a527ad21f96071e785772c56156c3ebf36
Author: yefurman <yefurman@cisco.com>
Date:   Thu Dec 11 12:03:33 2025 -0500

    iec104: validate Type I length to prevent ASDU out-of-bounds read

diff --git a/src/service_inspectors/iec104/iec104_decode.cc b/src/service_inspectors/iec104/iec104_decode.cc
index 60ef0a3d184665205b245290115eb6d4a9c86b68..ae2175e5c8f7b79844f9061549d7d47703416aa7 100644 (file)
--- a/src/service_inspectors/iec104/iec104_decode.cc
+++ b/src/service_inspectors/iec104/iec104_decode.cc
@@ -137,6 +137,11 @@ bool Iec104Decode(Packet* p, Iec104FlowData* iec104fd)
 
         case IEC104_APCI_TYPE_I:
         {
+            if (p->dsize < IEC104_APCI_TYPE_I_MIN_LEN)
+            {
+                return false;
+            }
+
             // build up the APCI
             const Iec104ApciI* apci = (const Iec104ApciI*) p->data;
 

diff --git a/src/service_inspectors/iec104/test/CMakeLists.txt b/src/service_inspectors/iec104/test/CMakeLists.txt
index 2bd86e34c7375bd10aa21386a33cb3d5ff4c3a92..d99f1f891f470b531f3971e9d57d03e264981b0f 100644 (file)
--- a/src/service_inspectors/iec104/test/CMakeLists.txt
+++ b/src/service_inspectors/iec104/test/CMakeLists.txt
@@ -1,3 +1,5 @@
 add_cpputest( iec104_parse_apdu_test
     SOURCES
-    ../iec104_parse_information_object_elements.cc )
\ No newline at end of file
+    ../iec104_parse_information_object_elements.cc )
+
+add_cpputest( iec104_decode_test )

diff --git a/src/service_inspectors/iec104/test/iec104_decode_test.cc b/src/service_inspectors/iec104/test/iec104_decode_test.cc
new file mode 100644 (file)
index 0000000..aaf48dd
--- /dev/null
+++ b/src/service_inspectors/iec104/test/iec104_decode_test.cc
@@ -0,0 +1,132 @@
+//--------------------------------------------------------------------------
+// Copyright (C) 2024-2025 Cisco and/or its affiliates. All rights reserved.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License Version 2 as published
+// by the Free Software Foundation.  You may not use, modify or distribute
+// this program under any other version of the GNU General Public License.
+//
+// This program is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//--------------------------------------------------------------------------
+// iec104_parse_apdu_test.cc author Yehor Furman <yefurman@cisco.com>
+
+#include "../iec104_decode.cc"
+
+#include <CppUTest/CommandLineTestRunner.h>
+#include <CppUTest/TestHarness.h>
+#include <CppUTestExt/MockSupport.h>
+
+THREAD_LOCAL const snort::Trace* iec104_trace = nullptr;
+
+namespace snort
+{
+    uint8_t TraceApi::get_constraints_generation() { return 0; }
+    void TraceApi::filter(const Packet&) { }
+    void trace_vprintf(const char*, uint8_t, const char*, const Packet*, const char*, va_list) { }
+
+    Packet::Packet(bool) { memset(this, 0, sizeof(*this)); }
+    Packet::~Packet() = default;
+
+    FlowData::FlowData(unsigned u, Inspector*) : id(u), handler(nullptr) { }
+    FlowData::~FlowData() = default;
+}
+
+unsigned Iec104FlowData::inspector_id = 0;
+Iec104FlowData::Iec104FlowData() : snort::FlowData(inspector_id) { }
+Iec104FlowData::~Iec104FlowData() = default;
+
+void parseIec104ApciU(const Iec104ApciU*) { }
+void parseIec104ApciS(const Iec104ApciS*) { }
+void parseIec104ApciI(const Iec104ApciI*, const uint16_t&) { }
+
+TEST_GROUP(iec104_decode_type_i_min_len)
+{
+    // cppcheck-suppress constVariablePointer
+    snort::Packet* packet = nullptr;
+    // cppcheck-suppress constVariablePointer
+    Iec104FlowData* flow_data = nullptr;
+
+    void setup() override
+    {
+        // cppcheck-suppress unreadVariable
+        packet = new snort::Packet(false);
+        // cppcheck-suppress unreadVariable
+        flow_data = new Iec104FlowData();
+    }
+
+    void teardown() override
+    {
+        delete packet;
+        delete flow_data;
+    }
+};
+
+TEST(iec104_decode_type_i_min_len, type_i_6_bytes_rejected)
+{
+    const uint8_t type_i_6_bytes[] = {0x68, 0x04, 0x00, 0x00, 0x00, 0x00};
+    snort::Packet* const local_packet = packet;
+    Iec104FlowData* const local_flow_data = flow_data;
+    local_packet->data = type_i_6_bytes;
+    local_packet->dsize = sizeof(type_i_6_bytes);
+    bool result = Iec104Decode(local_packet, local_flow_data);
+    CHECK_FALSE(result);
+}
+
+TEST(iec104_decode_type_i_min_len, type_i_11_bytes_rejected)
+{
+    const uint8_t type_i_11_bytes[] = {0x68, 0x09, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01, 0x03, 0x01, 0x00};
+    snort::Packet* const local_packet = packet;
+    Iec104FlowData* const local_flow_data = flow_data;
+    local_packet->data = type_i_11_bytes;
+    local_packet->dsize = sizeof(type_i_11_bytes);
+    bool result = Iec104Decode(local_packet, local_flow_data);
+    CHECK_FALSE(result);
+}
+
+TEST(iec104_decode_type_i_min_len, type_i_12_bytes_accepted)
+{
+    const uint8_t type_i_12_bytes[] = {0x68, 0x0A, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01, 0x03, 0x01, 0x00, 0x00};
+    snort::Packet* const local_packet = packet;
+    Iec104FlowData* const local_flow_data = flow_data;
+    local_packet->data = type_i_12_bytes;
+    local_packet->dsize = sizeof(type_i_12_bytes);
+    bool result = Iec104Decode(local_packet, local_flow_data);
+    CHECK_TRUE(result);
+    CHECK_EQUAL(IEC104_APCI_TYPE_I, local_flow_data->ssn_data.iec104_apci_type);
+}
+
+TEST(iec104_decode_type_i_min_len, type_s_6_bytes_accepted)
+{
+    const uint8_t type_s_6_bytes[] = {0x68, 0x04, 0x01, 0x00, 0x00, 0x00};
+    snort::Packet* const local_packet = packet;
+    Iec104FlowData* const local_flow_data = flow_data;
+    local_packet->data = type_s_6_bytes;
+    local_packet->dsize = sizeof(type_s_6_bytes);
+    bool result = Iec104Decode(local_packet, local_flow_data);
+    CHECK_TRUE(result);
+    CHECK_EQUAL(IEC104_APCI_TYPE_S, local_flow_data->ssn_data.iec104_apci_type);
+}
+
+TEST(iec104_decode_type_i_min_len, type_u_6_bytes_accepted)
+{
+    const uint8_t type_u_6_bytes[] = {0x68, 0x04, 0x03, 0x00, 0x00, 0x00};
+    snort::Packet* const local_packet = packet;
+    Iec104FlowData* const local_flow_data = flow_data;
+    local_packet->data = type_u_6_bytes;
+    local_packet->dsize = sizeof(type_u_6_bytes);
+    bool result = Iec104Decode(local_packet, local_flow_data);
+    CHECK_TRUE(result);
+    CHECK_EQUAL(IEC104_APCI_TYPE_U, local_flow_data->ssn_data.iec104_apci_type);
+}
+
+int main(int argc, char** argv)
+{
+    return CommandLineTestRunner::RunAllTests(argc, argv);
+}