From 7dfc840dfc8c906d556c9847a19f6833bb85c8f2 Mon Sep 17 00:00:00 2001 From: Steve Karg Date: Wed, 24 Sep 2025 09:35:10 -0500 Subject: [PATCH] Bugfix/using-uint16-for-units-property-storage (#1107) * Fixed units property declaration in basic Analog Input header file to be uint16_t instead of uint8_t. * Added range checking of units property in example objects WriteProperty handler. --- ports/atmega328/av.c | 10 +++++-- ports/xplained/ai.c | 10 +++++-- src/bacnet/basic/object/ai.c | 8 +++++- src/bacnet/basic/object/ai.h | 2 +- src/bacnet/basic/object/ao.c | 8 ++++-- src/bacnet/basic/object/av.c | 8 +++++- src/bacnet/basic/object/iv.c | 14 +++++++++ src/bacnet/basic/object/piv.c | 54 ++++++++++++++++++++++++++--------- 8 files changed, 91 insertions(+), 23 deletions(-) diff --git a/ports/atmega328/av.c b/ports/atmega328/av.c index b1139dfa..bd685400 100644 --- a/ports/atmega328/av.c +++ b/ports/atmega328/av.c @@ -559,8 +559,14 @@ bool Analog_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) break; case PROP_UNITS: if (value.tag == BACNET_APPLICATION_TAG_ENUMERATED) { - status = Analog_Value_Units_Set( - wp_data->object_instance, value.type.Enumerated); + if (value.type.Enumerated <= UINT16_MAX) { + Analog_Value_Units_Set( + wp_data->object_instance, value.type.Enumerated); + } else { + status = false; + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } } else { wp_data->error_class = ERROR_CLASS_PROPERTY; wp_data->error_code = ERROR_CODE_INVALID_DATA_TYPE; diff --git a/ports/xplained/ai.c b/ports/xplained/ai.c index 48ed7c34..1ecde9aa 100644 --- a/ports/xplained/ai.c +++ b/ports/xplained/ai.c @@ -279,8 +279,14 @@ bool Analog_Input_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) status = write_property_type_valid(wp_data, &value, BACNET_APPLICATION_TAG_ENUMERATED); if (status) { - Analog_Input_Out_Of_Service_Set( - wp_data->object_instance, value.type.Enumerated); + if (value.type.Enumerated <= UINT16_MAX) { + Analog_Input_Units_Set( + wp_data->object_instance, value.type.Enumerated); + } else { + status = false; + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } } break; case PROP_OBJECT_IDENTIFIER: diff --git a/src/bacnet/basic/object/ai.c b/src/bacnet/basic/object/ai.c index 7383e388..2bd170fa 100644 --- a/src/bacnet/basic/object/ai.c +++ b/src/bacnet/basic/object/ai.c @@ -958,7 +958,13 @@ bool Analog_Input_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) status = write_property_type_valid( wp_data, &value, BACNET_APPLICATION_TAG_ENUMERATED); if (status) { - pObject->Units = value.type.Enumerated; + if (value.type.Enumerated <= UINT16_MAX) { + pObject->Units = value.type.Enumerated; + } else { + status = false; + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } } break; case PROP_COV_INCREMENT: diff --git a/src/bacnet/basic/object/ai.h b/src/bacnet/basic/object/ai.h index a5698226..d7294922 100644 --- a/src/bacnet/basic/object/ai.h +++ b/src/bacnet/basic/object/ai.h @@ -28,7 +28,7 @@ typedef struct analog_input_descr { float Present_Value; BACNET_RELIABILITY Reliability; bool Out_Of_Service; - uint8_t Units; + uint16_t Units; float Prior_Value; float COV_Increment; bool Changed; diff --git a/src/bacnet/basic/object/ao.c b/src/bacnet/basic/object/ao.c index 6e220379..b6f9b363 100644 --- a/src/bacnet/basic/object/ao.c +++ b/src/bacnet/basic/object/ao.c @@ -1191,9 +1191,11 @@ bool Analog_Output_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) status = write_property_type_valid( wp_data, &value, BACNET_APPLICATION_TAG_ENUMERATED); if (status) { - status = Analog_Output_Units_Set( - wp_data->object_instance, value.type.Enumerated); - if (!status) { + if (value.type.Enumerated <= UINT16_MAX) { + Analog_Output_Units_Set( + wp_data->object_instance, value.type.Enumerated); + } else { + status = false; wp_data->error_class = ERROR_CLASS_PROPERTY; wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; } diff --git a/src/bacnet/basic/object/av.c b/src/bacnet/basic/object/av.c index 4371405b..47227885 100644 --- a/src/bacnet/basic/object/av.c +++ b/src/bacnet/basic/object/av.c @@ -997,7 +997,13 @@ bool Analog_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) status = write_property_type_valid( wp_data, &value, BACNET_APPLICATION_TAG_ENUMERATED); if (status) { - CurrentAV->Units = value.type.Enumerated; + if (value.type.Enumerated <= UINT16_MAX) { + CurrentAV->Units = value.type.Enumerated; + } else { + status = false; + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } } break; case PROP_COV_INCREMENT: diff --git a/src/bacnet/basic/object/iv.c b/src/bacnet/basic/object/iv.c index b1e79ded..005efdc6 100644 --- a/src/bacnet/basic/object/iv.c +++ b/src/bacnet/basic/object/iv.c @@ -586,6 +586,20 @@ bool Integer_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) wp_data->object_instance, value.type.Boolean); } break; + case PROP_UNITS: + status = write_property_type_valid( + wp_data, &value, BACNET_APPLICATION_TAG_ENUMERATED); + if (status) { + if (value.type.Enumerated <= UINT16_MAX) { + Integer_Value_Units_Set( + wp_data->object_instance, value.type.Enumerated); + } else { + status = false; + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } + } + break; default: if (property_lists_member( Integer_Value_Properties_Required, diff --git a/src/bacnet/basic/object/piv.c b/src/bacnet/basic/object/piv.c index 5c347f4e..cad8954c 100644 --- a/src/bacnet/basic/object/piv.c +++ b/src/bacnet/basic/object/piv.c @@ -164,7 +164,16 @@ bool PositiveInteger_Value_Object_Name( return status; } -/* return apdu len, or BACNET_STATUS_ERROR on error */ +/** + * ReadProperty handler for this object. For the given ReadProperty + * data, the application_data is loaded or the error flags are set. + * + * @param rpdata - BACNET_READ_PROPERTY_DATA data, including + * requested data and space for the reply, or error response. + * + * @return number of APDU bytes in the response, or + * BACNET_STATUS_ERROR on error. + */ int PositiveInteger_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) { int apdu_len = 0; /* return value */ @@ -254,7 +263,15 @@ int PositiveInteger_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) return apdu_len; } -/* returns true if successful */ +/** + * WriteProperty handler for this object. For the given WriteProperty + * data, the application_data is loaded or the error flags are set. + * + * @param wp_data - BACNET_WRITE_PROPERTY_DATA data, including + * requested data and space for the reply, or error response. + * + * @return false if an error is loaded, true if no errors + */ bool PositiveInteger_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) { bool status = false; /* return value */ @@ -280,7 +297,6 @@ bool PositiveInteger_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) } else { return false; } - switch (wp_data->object_property) { case PROP_PRESENT_VALUE: status = write_property_type_valid( @@ -305,7 +321,6 @@ bool PositiveInteger_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) } } break; - case PROP_OUT_OF_SERVICE: status = write_property_type_valid( wp_data, &value, BACNET_APPLICATION_TAG_BOOLEAN); @@ -313,18 +328,31 @@ bool PositiveInteger_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) CurrentAV->Out_Of_Service = value.type.Boolean; } break; - - case PROP_OBJECT_IDENTIFIER: - case PROP_OBJECT_NAME: - case PROP_OBJECT_TYPE: - case PROP_STATUS_FLAGS: case PROP_UNITS: - wp_data->error_class = ERROR_CLASS_PROPERTY; - wp_data->error_code = ERROR_CODE_WRITE_ACCESS_DENIED; + status = write_property_type_valid( + wp_data, &value, BACNET_APPLICATION_TAG_ENUMERATED); + if (status) { + if (value.type.Enumerated <= UINT16_MAX) { + CurrentAV->Units = (uint16_t)value.type.Enumerated; + } else { + status = false; + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } + } break; default: - wp_data->error_class = ERROR_CLASS_PROPERTY; - wp_data->error_code = ERROR_CODE_UNKNOWN_PROPERTY; + if (property_lists_member( + PositiveInteger_Value_Properties_Required, + PositiveInteger_Value_Properties_Optional, + PositiveInteger_Value_Properties_Proprietary, + wp_data->object_property)) { + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_WRITE_ACCESS_DENIED; + } else { + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_UNKNOWN_PROPERTY; + } break; }