From f59a460292392a2cfeb958ec989edaccaa9925d9 Mon Sep 17 00:00:00 2001 From: Steve Karg Date: Tue, 9 May 2023 15:58:28 -0500 Subject: [PATCH] Fixed array index out of range for binary-value, multistate-output, and lighting-output objects. Thanks, Roy! (#430) Co-authored-by: Steve Karg --- src/bacnet/basic/object/bv.c | 86 +++++++------- src/bacnet/basic/object/lo.c | 136 ++++++---------------- src/bacnet/basic/object/mso.c | 207 +++++++++++++--------------------- 3 files changed, 154 insertions(+), 275 deletions(-) diff --git a/src/bacnet/basic/object/bv.c b/src/bacnet/basic/object/bv.c index 484b03c7..b1ee4c00 100644 --- a/src/bacnet/basic/object/bv.c +++ b/src/bacnet/basic/object/bv.c @@ -196,6 +196,36 @@ BACNET_BINARY_PV Binary_Value_Present_Value(uint32_t object_instance) return value; } +/** + * @brief Encode a BACnetARRAY property element + * @param object_instance [in] BACnet network port object instance number + * @param priority [in] array index requested: + * 0 to N for individual array members + * @param apdu [out] Buffer in which the APDU contents are built, or NULL to + * return the length of buffer if it had been built + * @return The length of the apdu encoded or + * BACNET_STATUS_ERROR for ERROR_CODE_INVALID_ARRAY_INDEX + */ +static int Binary_Value_Priority_Array_Encode( + uint32_t object_instance, BACNET_ARRAY_INDEX priority, uint8_t *apdu) +{ + int apdu_len = BACNET_STATUS_ERROR; + BACNET_BINARY_PV value = RELINQUISH_DEFAULT; + unsigned index = 0; + + index = Binary_Value_Instance_To_Index(object_instance); + if ((index < MAX_BINARY_VALUES) && (priority < BACNET_MAX_PRIORITY)) { + if (Binary_Value_Level[index][priority] != BINARY_NULL) { + apdu_len = encode_application_null(apdu); + } else { + value = Binary_Value_Level[index][priority]; + apdu_len = encode_application_enumerated(apdu, value); + } + } + + return apdu_len; +} + /** * For a given object instance-number, return the name. * @@ -266,13 +296,12 @@ void Binary_Value_Out_Of_Service_Set(uint32_t instance, bool oos_flag) */ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) { - int len = 0; int apdu_len = 0; /* return value */ + int apdu_size = 0; BACNET_BIT_STRING bit_string; BACNET_CHARACTER_STRING char_string; BACNET_BINARY_PV present_value = BINARY_INACTIVE; unsigned object_index = 0; - unsigned i = 0; bool state = false; uint8_t *apdu = NULL; @@ -294,6 +323,7 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) } apdu = rpdata->application_data; + apdu_size = rpdata->application_data_len; switch (rpdata->object_property) { case PROP_OBJECT_IDENTIFIER: apdu_len = encode_application_object_id( @@ -337,49 +367,15 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) apdu_len = encode_application_boolean(&apdu[0], state); break; case PROP_PRIORITY_ARRAY: - /* Array element zero is the number of elements in the array */ - if (rpdata->array_index == 0) { - apdu_len = - encode_application_unsigned(&apdu[0], BACNET_MAX_PRIORITY); - /* if no index was specified, then try to encode the entire list - */ - /* into one packet. */ - } else if (rpdata->array_index == BACNET_ARRAY_ALL) { - for (i = 0; i < BACNET_MAX_PRIORITY; i++) { - /* FIXME: check if we have room before adding it to APDU */ - if (Binary_Value_Level[object_index][i] == BINARY_NULL) { - len = encode_application_null(&apdu[apdu_len]); - } else { - present_value = Binary_Value_Level[object_index][i]; - len = encode_application_enumerated( - &apdu[apdu_len], present_value); - } - /* add it if we have room */ - if ((apdu_len + len) < MAX_APDU) { - apdu_len += len; - } else { - rpdata->error_code = - ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; - apdu_len = BACNET_STATUS_ABORT; - break; - } - } - } else { - if (rpdata->array_index <= BACNET_MAX_PRIORITY) { - if (Binary_Value_Level[object_index][rpdata->array_index] == - BINARY_NULL) { - apdu_len = encode_application_null(&apdu[apdu_len]); - } else { - present_value = Binary_Value_Level[object_index] - [rpdata->array_index]; - apdu_len = encode_application_enumerated( - &apdu[apdu_len], present_value); - } - } else { - rpdata->error_class = ERROR_CLASS_PROPERTY; - rpdata->error_code = ERROR_CODE_INVALID_ARRAY_INDEX; - apdu_len = BACNET_STATUS_ERROR; - } + apdu_len = bacnet_array_encode(rpdata->object_instance, + rpdata->array_index, Binary_Value_Priority_Array_Encode, + BACNET_MAX_PRIORITY, apdu, apdu_size); + if (apdu_len == BACNET_STATUS_ABORT) { + rpdata->error_code = + ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; + } else if (apdu_len == BACNET_STATUS_ERROR) { + rpdata->error_class = ERROR_CLASS_PROPERTY; + rpdata->error_code = ERROR_CODE_INVALID_ARRAY_INDEX; } break; case PROP_RELINQUISH_DEFAULT: diff --git a/src/bacnet/basic/object/lo.c b/src/bacnet/basic/object/lo.c index b295c81d..75183814 100644 --- a/src/bacnet/basic/object/lo.c +++ b/src/bacnet/basic/object/lo.c @@ -216,58 +216,33 @@ float Lighting_Output_Present_Value(uint32_t object_instance) } /** - * For a given object instance-number, determines the value at a - * given priority. - * - * @param object_instance - object-instance number of the object - * @param priority - priority 1..16 - * - * @return priority-value of the object + * @brief Encode a BACnetARRAY property element + * @param object_instance [in] BACnet network port object instance number + * @param priority [in] array index requested: + * 0 to N for individual array members + * @param apdu [out] Buffer in which the APDU contents are built, or NULL to + * return the length of buffer if it had been built + * @return The length of the apdu encoded or + * BACNET_STATUS_ERROR for ERROR_CODE_INVALID_ARRAY_INDEX */ -static float Lighting_Output_Priority_Value( - uint32_t object_instance, unsigned priority) +static int Lighting_Output_Priority_Array_Encode( + uint32_t object_instance, BACNET_ARRAY_INDEX priority, uint8_t *apdu) { - float value = 0.0; + int apdu_len = BACNET_STATUS_ERROR; + float real_value = 0.0; unsigned index = 0; index = Lighting_Output_Instance_To_Index(object_instance); - if (index < MAX_LIGHTING_OUTPUTS) { - if (priority && (priority <= BACNET_MAX_PRIORITY)) { - priority--; - value = Lighting_Output[index].Priority_Array[priority]; + if ((index < MAX_LIGHTING_OUTPUTS) && (priority < BACNET_MAX_PRIORITY)) { + if (BIT_CHECK(Lighting_Output[index].Priority_Active_Bits, priority)) { + real_value = Lighting_Output[index].Priority_Array[priority]; + apdu_len = encode_application_real(apdu, real_value); + } else { + apdu_len = encode_application_null(apdu); } } - return value; -} - -/** - * For a given object instance-number, determines if the given priority - * is active or NULL. - * - * @param object_instance - object-instance number of the object - * @param priority - priority 1..16 - * - * @return true if the priority slot is active - */ -static bool Lighting_Output_Priority_Active( - uint32_t object_instance, unsigned priority) -{ - bool status = false; - unsigned index = 0; - - index = Lighting_Output_Instance_To_Index(object_instance); - if (index < MAX_LIGHTING_OUTPUTS) { - if (priority && (priority <= BACNET_MAX_PRIORITY)) { - priority--; - if (BIT_CHECK( - Lighting_Output[index].Priority_Active_Bits, priority)) { - status = true; - } - } - } - - return status; + return apdu_len; } /** @@ -908,8 +883,8 @@ bool Lighting_Output_Relinquish_Default_Set( */ int Lighting_Output_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) { - int len = 0; int apdu_len = 0; /* return value */ + int apdu_size = 0; BACNET_BIT_STRING bit_string; BACNET_CHARACTER_STRING char_string; BACNET_LIGHTING_COMMAND lighting_command; @@ -924,6 +899,7 @@ int Lighting_Output_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) return 0; } apdu = rpdata->application_data; + apdu_size = rpdata->application_data_len; switch (rpdata->object_property) { case PROP_OBJECT_IDENTIFIER: apdu_len = encode_application_object_id( @@ -999,59 +975,15 @@ int Lighting_Output_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) apdu_len = encode_application_real(&apdu[0], real_value); break; case PROP_PRIORITY_ARRAY: - /* Array element zero is the number of elements in the array */ - if (rpdata->array_index == 0) { - apdu_len = - encode_application_unsigned(&apdu[0], BACNET_MAX_PRIORITY); - /* if no index was specified, then try to encode the entire list - */ - /* into one packet. */ - } else if (rpdata->array_index == BACNET_ARRAY_ALL) { - for (i = 1; i <= BACNET_MAX_PRIORITY; i++) { - if (Lighting_Output_Priority_Active( - rpdata->object_instance, i)) { - real_value = Lighting_Output_Priority_Value( - rpdata->object_instance, i); - len = encode_application_real( - &apdu[apdu_len], real_value); - } else { - len = encode_application_null(&apdu[apdu_len]); - } - /* add it if we have room */ - if ((apdu_len + len) < MAX_APDU) { - apdu_len += len; - } else { - rpdata->error_code = - ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; - apdu_len = BACNET_STATUS_ABORT; - break; - } - } - } else { - if (rpdata->array_index <= BACNET_MAX_PRIORITY) { - if (Lighting_Output_Priority_Active( - rpdata->object_instance, rpdata->array_index)) { - real_value = Lighting_Output_Priority_Value( - rpdata->object_instance, rpdata->array_index); - len = encode_application_real( - &apdu[apdu_len], real_value); - } else { - len = encode_application_null(&apdu[apdu_len]); - } - /* add it if we have room */ - if ((apdu_len + len) < MAX_APDU) { - apdu_len += len; - } else { - rpdata->error_code = - ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; - apdu_len = BACNET_STATUS_ABORT; - break; - } - } else { - rpdata->error_class = ERROR_CLASS_PROPERTY; - rpdata->error_code = ERROR_CODE_INVALID_ARRAY_INDEX; - apdu_len = BACNET_STATUS_ERROR; - } + apdu_len = bacnet_array_encode(rpdata->object_instance, + rpdata->array_index, Lighting_Output_Priority_Array_Encode, + BACNET_MAX_PRIORITY, apdu, apdu_size); + if (apdu_len == BACNET_STATUS_ABORT) { + rpdata->error_code = + ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; + } else if (apdu_len == BACNET_STATUS_ERROR) { + rpdata->error_class = ERROR_CLASS_PROPERTY; + rpdata->error_code = ERROR_CODE_INVALID_ARRAY_INDEX; } break; case PROP_RELINQUISH_DEFAULT: @@ -1229,7 +1161,7 @@ static void Lighting_Output_Ramp_Handler(struct lighting_output_object *pLight, uint16_t milliseconds) { if (pLight && pCommand) { - /* FIXME: add ramp functionality */ + /* FIXME: add ramp functionality */ (void)pLight; (void)pCommand; (void)milliseconds; @@ -1248,11 +1180,11 @@ static void Lighting_Output_Fade_Handler(struct lighting_output_object *pLight, BACNET_LIGHTING_COMMAND *pCommand, uint16_t milliseconds) { - if (pLight && pCommand) { - /* FIXME: add fade functionality */ + if (pLight && pCommand) { + /* FIXME: add fade functionality */ (void)pLight; (void)pCommand; - (void)milliseconds; + (void)milliseconds; } } diff --git a/src/bacnet/basic/object/mso.c b/src/bacnet/basic/object/mso.c index c619f663..4379c7cb 100644 --- a/src/bacnet/basic/object/mso.c +++ b/src/bacnet/basic/object/mso.c @@ -273,6 +273,36 @@ uint32_t Multistate_Output_Present_Value(uint32_t object_instance) return value; } +/** + * @brief Encode a BACnetARRAY property element + * @param object_instance [in] BACnet network port object instance number + * @param priority [in] array index requested: + * 0 to N for individual array members + * @param apdu [out] Buffer in which the APDU contents are built, or NULL to + * return the length of buffer if it had been built + * @return The length of the apdu encoded or + * BACNET_STATUS_ERROR for ERROR_CODE_INVALID_ARRAY_INDEX + */ +static int Multistate_Output_Priority_Array_Encode( + uint32_t object_instance, BACNET_ARRAY_INDEX priority, uint8_t *apdu) +{ + int apdu_len = BACNET_STATUS_ERROR; + uint32_t value = 1; + struct object_data *pObject; + + pObject = Keylist_Data(Object_List, object_instance); + if (pObject && (priority < BACNET_MAX_PRIORITY)) { + if (pObject->Relinquished[priority]) { + apdu_len = encode_application_null(apdu); + } else { + value = pObject->Priority_Array[priority]; + apdu_len = encode_application_enumerated(apdu, value); + } + } + + return apdu_len; +} + /** * @brief For a given object instance-number, determines the active priority * @param object_instance - object-instance number of the object @@ -297,54 +327,6 @@ unsigned Multistate_Output_Present_Value_Priority(uint32_t object_instance) return priority; } -/** - * @brief For a given object instance-number and priority 1..16, determines the - * priority-array value - * @param object_instance - object-instance number of the object - * @param priority - priority-array index value 1..16 - * @return priority-array value of the object - */ -static uint32_t Multistate_Output_Priority_Array( - uint32_t object_instance, unsigned priority) -{ - uint32_t value = 1; - struct object_data *pObject; - - pObject = Keylist_Data(Object_List, object_instance); - if (pObject) { - if ((priority >= 1) && (priority <= BACNET_MAX_PRIORITY)) { - value = pObject->Priority_Array[priority - 1]; - } - } - - return value; -} - -/** - * @brief For a given object instance-number and priority 1..16, determines - * if the priority-array slot is NULL - * @param object_instance - object-instance number of the object - * @param priority - priority-array index value 1..16 - * @return true if the priority array slot is NULL - */ -static bool Multistate_Output_Priority_Array_Null( - uint32_t object_instance, unsigned priority) -{ - bool null_value = false; - struct object_data *pObject; - - pObject = Keylist_Data(Object_List, object_instance); - if (pObject) { - if ((priority >= 1) && (priority <= BACNET_MAX_PRIORITY)) { - if (pObject->Relinquished[priority - 1]) { - null_value = true; - } - } - } - - return null_value; -} - /** * @brief For a given object instance-number, determines the * relinquish-default value @@ -694,6 +676,35 @@ char *Multistate_Output_State_Text( return pName; } +/** + * @brief Encode a BACnetARRAY property element + * @param object_instance [in] BACnet network port object instance number + * @param index [in] array index requested: + * 0 to N for individual array members + * @param apdu [out] Buffer in which the APDU contents are built, or NULL to + * return the length of buffer if it had been built + * @return The length of the apdu encoded or + * BACNET_STATUS_ERROR for ERROR_CODE_INVALID_ARRAY_INDEX + */ +static int Multistate_Output_State_Text_Encode( + uint32_t object_instance, BACNET_ARRAY_INDEX index, uint8_t *apdu) +{ + int apdu_len = BACNET_STATUS_ERROR; + char *pName = NULL; /* return value */ + BACNET_CHARACTER_STRING char_string = { 0 }; + uint32_t state_index = 1; + + state_index += index; + pName = Multistate_Output_State_Text(object_instance, state_index); + if (pName) { + characterstring_init_ansi(&char_string, pName); + apdu_len = encode_application_character_string( + apdu, &char_string); + } + + return apdu_len; +} + /** * @brief For a given object instance-number, sets the list of state-text from * a C string array. The state_text_list consists of C strings separated @@ -914,8 +925,8 @@ bool Multistate_Output_Encode_Value_List( */ int Multistate_Output_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) { - int len = 0; int apdu_len = 0; /* return value */ + int apdu_size = 0; BACNET_BIT_STRING bit_string; BACNET_CHARACTER_STRING char_string; uint32_t present_value = 0; @@ -929,6 +940,7 @@ int Multistate_Output_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) return 0; } apdu = rpdata->application_data; + apdu_size = rpdata->application_data_len; switch (rpdata->object_property) { case PROP_OBJECT_IDENTIFIER: apdu_len = encode_application_object_id( @@ -976,49 +988,15 @@ int Multistate_Output_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) Multistate_Output_Max_States(rpdata->object_instance)); break; case PROP_PRIORITY_ARRAY: - if (rpdata->array_index == 0) { - /* Array element zero = the number of elements in the array */ - apdu_len = - encode_application_unsigned(&apdu[0], BACNET_MAX_PRIORITY); - } else if (rpdata->array_index == BACNET_ARRAY_ALL) { - /* no index was specified; try to encode the entire list */ - for (i = 1; i <= BACNET_MAX_PRIORITY; i++) { - if (Multistate_Output_Priority_Array_Null( - rpdata->object_instance, i)) { - len = encode_application_null(&apdu[apdu_len]); - } else { - present_value = Multistate_Output_Priority_Array( - rpdata->object_instance, i); - len = encode_application_unsigned( - &apdu[apdu_len], present_value); - } - /* add it if we have room */ - if ((apdu_len + len) < MAX_APDU) { - apdu_len += len; - } else { - /* Abort response */ - rpdata->error_code = - ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; - apdu_len = BACNET_STATUS_ABORT; - break; - } - } - } else { - if (rpdata->array_index <= BACNET_MAX_PRIORITY) { - if (Multistate_Output_Priority_Array_Null( - rpdata->object_instance, rpdata->array_index)) { - apdu_len = encode_application_null(&apdu[apdu_len]); - } else { - present_value = Multistate_Output_Priority_Array( - rpdata->object_instance, rpdata->array_index); - apdu_len = encode_application_unsigned( - &apdu[apdu_len], present_value); - } - } else { - rpdata->error_class = ERROR_CLASS_PROPERTY; - rpdata->error_code = ERROR_CODE_INVALID_ARRAY_INDEX; - apdu_len = BACNET_STATUS_ERROR; - } + apdu_len = bacnet_array_encode(rpdata->object_instance, + rpdata->array_index, Multistate_Output_Priority_Array_Encode, + BACNET_MAX_PRIORITY, apdu, apdu_size); + if (apdu_len == BACNET_STATUS_ABORT) { + rpdata->error_code = + ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; + } else if (apdu_len == BACNET_STATUS_ERROR) { + rpdata->error_class = ERROR_CLASS_PROPERTY; + rpdata->error_code = ERROR_CODE_INVALID_ARRAY_INDEX; } break; case PROP_RELINQUISH_DEFAULT: @@ -1028,42 +1006,15 @@ int Multistate_Output_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) break; case PROP_STATE_TEXT: max_states = Multistate_Output_Max_States(rpdata->object_instance); - if (rpdata->array_index == 0) { - /* Array element zero is the number of elements in the array */ - apdu_len = encode_application_unsigned(&apdu[0], max_states); - } else if (rpdata->array_index == BACNET_ARRAY_ALL) { - /* if no index was specified, then try to encode the entire list - */ - /* into one packet. */ - for (i = 1; i <= max_states; i++) { - characterstring_init_ansi(&char_string, - Multistate_Output_State_Text( - rpdata->object_instance, i)); - /* FIXME: this might go beyond MAX_APDU length! */ - len = encode_application_character_string( - &apdu[apdu_len], &char_string); - /* add it if we have room */ - if ((apdu_len + len) < MAX_APDU) { - apdu_len += len; - } else { - rpdata->error_class = ERROR_CLASS_SERVICES; - rpdata->error_code = ERROR_CODE_NO_SPACE_FOR_OBJECT; - apdu_len = BACNET_STATUS_ERROR; - break; - } - } - } else { - if (rpdata->array_index <= max_states) { - characterstring_init_ansi(&char_string, - Multistate_Output_State_Text( - rpdata->object_instance, rpdata->array_index)); - apdu_len = encode_application_character_string( - &apdu[0], &char_string); - } else { - rpdata->error_class = ERROR_CLASS_PROPERTY; - rpdata->error_code = ERROR_CODE_INVALID_ARRAY_INDEX; - apdu_len = BACNET_STATUS_ERROR; - } + apdu_len = bacnet_array_encode(rpdata->object_instance, + rpdata->array_index, Multistate_Output_State_Text_Encode, + max_states, apdu, apdu_size); + if (apdu_len == BACNET_STATUS_ABORT) { + rpdata->error_code = + ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; + } else if (apdu_len == BACNET_STATUS_ERROR) { + rpdata->error_class = ERROR_CLASS_PROPERTY; + rpdata->error_code = ERROR_CODE_INVALID_ARRAY_INDEX; } break; case PROP_DESCRIPTION: