From e143066b29978613e163886132f92560660de885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= Date: Wed, 18 Oct 2023 18:10:17 +0200 Subject: [PATCH] object/nc: Fix incorrect apdu_len calculation (#517) * object/nc: Fix incorrect apdu_len calculation when encoding Recipient_List which had resulted in malformed APDU. * enabled unit testing on notification-class object example --------- Co-authored-by: Tomasz Kazimierz Motyl Co-authored-by: Steve Karg --- src/bacnet/basic/object/nc.c | 20 +++-- test/CMakeLists.txt | 1 + test/bacnet/basic/object/nc/src/main.c | 105 +++++++++++++++++++++---- 3 files changed, 103 insertions(+), 23 deletions(-) diff --git a/src/bacnet/basic/object/nc.c b/src/bacnet/basic/object/nc.c index 85cda48b..ba3719ad 100644 --- a/src/bacnet/basic/object/nc.c +++ b/src/bacnet/basic/object/nc.c @@ -267,6 +267,7 @@ int Notification_Class_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) break; } /* size fits, therefore, encode all entry of Recipient_List */ + apdu_len = 0; for (idx = 0; idx < NC_MAX_RECIPIENTS; idx++) { BACNET_DESTINATION *Destination; BACNET_RECIPIENT *Recipient; @@ -447,7 +448,11 @@ bool Notification_Class_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) status = true; break; + case PROP_OBJECT_IDENTIFIER: case PROP_OBJECT_NAME: + case PROP_OBJECT_TYPE: + case PROP_DESCRIPTION: + case PROP_NOTIFICATION_CLASS: wp_data->error_class = ERROR_CLASS_PROPERTY; wp_data->error_code = ERROR_CODE_WRITE_ACCESS_DENIED; break; @@ -704,8 +709,7 @@ void Notification_Class_find_recipient(void) * the BACnetLIST all of the specified elements, or to neither add nor * update any elements at all. */ -int Notification_Class_Add_List_Element( - BACNET_LIST_ELEMENT_DATA * list_element) +int Notification_Class_Add_List_Element(BACNET_LIST_ELEMENT_DATA *list_element) { NOTIFICATION_CLASS_INFO *notification = NULL; BACNET_DESTINATION recipient_list[NC_MAX_RECIPIENTS] = { 0 }; @@ -755,8 +759,8 @@ int Notification_Class_Add_List_Element( application_data = list_element->application_data; application_data_len = list_element->application_data_len; while (application_data_len > 0) { - len = bacnet_destination_decode(application_data, - application_data_len, &recipient_list[index]); + len = bacnet_destination_decode( + application_data, application_data_len, &recipient_list[index]); if (len > 0) { new_element_count++; application_data_len -= len; @@ -793,7 +797,7 @@ int Notification_Class_Add_List_Element( same_element_count++; } else { added_element_count++; - if ((added_element_count+element_count) > NC_MAX_RECIPIENTS) { + if ((added_element_count + element_count) > NC_MAX_RECIPIENTS) { list_element->first_failed_element_number = 1 + i; list_element->error_class = ERROR_CLASS_RESOURCES; list_element->error_code = @@ -862,7 +866,7 @@ int Notification_Class_Add_List_Element( * a 'Result(-)' response primitive shall be issued. */ int Notification_Class_Remove_List_Element( - BACNET_LIST_ELEMENT_DATA * list_element) + BACNET_LIST_ELEMENT_DATA *list_element) { NOTIFICATION_CLASS_INFO *notification = NULL; uint32_t notify_index = 0; @@ -911,8 +915,8 @@ int Notification_Class_Remove_List_Element( application_data = list_element->application_data; application_data_len = list_element->application_data_len; while (application_data_len > 0) { - len = bacnet_destination_decode(application_data, - application_data_len, &recipient_list[index]); + len = bacnet_destination_decode( + application_data, application_data_len, &recipient_list[index]); if (len > 0) { remove_element_count++; application_data_len -= len; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index b3da4f14..cd14bbb0 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -125,6 +125,7 @@ list(APPEND testdirs bacnet/basic/object/mso bacnet/basic/object/msv bacnet/basic/object/netport + bacnet/basic/object/nc bacnet/basic/object/objects bacnet/basic/object/osv bacnet/basic/object/piv diff --git a/test/bacnet/basic/object/nc/src/main.c b/test/bacnet/basic/object/nc/src/main.c index 34674bbd..b092924a 100644 --- a/test/bacnet/basic/object/nc/src/main.c +++ b/test/bacnet/basic/object/nc/src/main.c @@ -6,9 +6,10 @@ * * SPDX-License-Identifier: MIT */ -#include +#include #include #include +#include #include /** @@ -25,29 +26,103 @@ ZTEST(notification_class_tests, test_Notification_Class) static void test_Notification_Class(void) #endif { - BACNET_READ_PROPERTY_DATA rpdata; uint8_t apdu[MAX_APDU] = { 0 }; int len = 0; - uint32_t len_value = 0; - uint8_t tag_number = 0; - BACNET_OBJECT_TYPE decoded_type = 0; - uint32_t decoded_instance = 0; + int test_len = 0; + BACNET_READ_PROPERTY_DATA rpdata = { 0 }; + BACNET_APPLICATION_DATA_VALUE value = { 0 }; + const int *pRequired = NULL; + const int *pOptional = NULL; + const int *pProprietary = NULL; + const uint32_t instance = 1; + BACNET_WRITE_PROPERTY_DATA wpdata = { 0 }; + bool status = false; + unsigned index; Notification_Class_Init(); + status = Notification_Class_Valid_Instance(instance); + zassert_true(status, NULL); + index = Notification_Class_Instance_To_Index(instance); + zassert_equal(index, instance, "index=%u", index); + rpdata.application_data = &apdu[0]; rpdata.application_data_len = sizeof(apdu); rpdata.object_type = OBJECT_NOTIFICATION_CLASS; - rpdata.object_instance = 1; + rpdata.object_instance = instance; rpdata.object_property = PROP_OBJECT_IDENTIFIER; - rpdata.array_index = BACNET_ARRAY_ALL; + + Notification_Class_Property_Lists(&pRequired, &pOptional, &pProprietary); + while ((*pRequired) >= 0) { + rpdata.object_property = *pRequired; + rpdata.array_index = BACNET_ARRAY_ALL; + len = Notification_Class_Read_Property(&rpdata); + zassert_not_equal(len, BACNET_STATUS_ERROR, + "property '%s': failed to ReadProperty!\n", + bactext_property_name(rpdata.object_property)); + if (len >= 0) { + test_len = bacapp_decode_known_property(rpdata.application_data, + len, &value, rpdata.object_type, rpdata.object_property); + if ((rpdata.object_property != PROP_PRIORITY) && + (rpdata.object_property != PROP_RECIPIENT_LIST)) { + zassert_equal(len, test_len, "property '%s': failed to decode!\n", + bactext_property_name(rpdata.object_property)); + } + /* check WriteProperty properties */ + wpdata.object_type = rpdata.object_type; + wpdata.object_instance = rpdata.object_instance; + wpdata.object_property = rpdata.object_property; + wpdata.array_index = rpdata.array_index; + memcpy(&wpdata.application_data, rpdata.application_data, MAX_APDU); + wpdata.application_data_len = len; + wpdata.error_code = ERROR_CODE_SUCCESS; + status = Notification_Class_Write_Property(&wpdata); + if (!status) { + /* verify WriteProperty property is known */ + zassert_not_equal(wpdata.error_code, + ERROR_CODE_UNKNOWN_PROPERTY, + "property '%s': WriteProperty Unknown!\n", + bactext_property_name(rpdata.object_property)); + } + } + pRequired++; + } + while ((*pOptional) != -1) { + rpdata.object_property = *pOptional; + rpdata.array_index = BACNET_ARRAY_ALL; + len = Notification_Class_Read_Property(&rpdata); + zassert_not_equal(len, BACNET_STATUS_ERROR, + "property '%s': failed to ReadProperty!\n", + bactext_property_name(rpdata.object_property)); + if (len > 0) { + test_len = bacapp_decode_application_data(rpdata.application_data, + (uint8_t)rpdata.application_data_len, &value); + zassert_equal(len, test_len, "property '%s': failed to decode!\n", + bactext_property_name(rpdata.object_property)); + /* check WriteProperty properties */ + wpdata.object_type = rpdata.object_type; + wpdata.object_instance = rpdata.object_instance; + wpdata.object_property = rpdata.object_property; + wpdata.array_index = rpdata.array_index; + memcpy(&wpdata.application_data, rpdata.application_data, MAX_APDU); + wpdata.application_data_len = len; + wpdata.error_code = ERROR_CODE_SUCCESS; + status = Notification_Class_Write_Property(&wpdata); + if (!status) { + /* verify WriteProperty property is known */ + zassert_not_equal(wpdata.error_code, + ERROR_CODE_UNKNOWN_PROPERTY, + "property '%s': WriteProperty Unknown!\n", + bactext_property_name(rpdata.object_property)); + } + } + pOptional++; + } + rpdata.object_property = PROP_ALL; len = Notification_Class_Read_Property(&rpdata); - zassert_not_equal(len, 0, NULL); - len = bacnet_decode_tag_number_and_value( - apdu, sizeof(apdu), &tag_number, &len_value); - zassert_equal(tag_number, BACNET_APPLICATION_TAG_OBJECT_ID, NULL); - len = decode_object_id(&apdu[len], &decoded_type, &decoded_instance); - zassert_equal(decoded_type, rpdata.object_type, NULL); - zassert_equal(decoded_instance, rpdata.object_instance, NULL); + zassert_equal(len, BACNET_STATUS_ERROR, NULL); + wpdata.object_property = PROP_ALL; + status = Notification_Class_Write_Property(&wpdata); + zassert_false(status, NULL); return; }