From adff1f9c0f6bd65d98cbd8149888d8796d4151f6 Mon Sep 17 00:00:00 2001 From: Steve Karg Date: Thu, 13 Feb 2025 12:58:54 -0600 Subject: [PATCH] Fixed the ReinitializeDevice and DeviceCommunicationControl password length checking for non-UTF8 passwords. (#914) --- src/bacnet/basic/object/device.c | 8 +++- src/bacnet/dcc.c | 73 ++++++++++++-------------------- src/bacnet/rd.c | 10 ++++- 3 files changed, 41 insertions(+), 50 deletions(-) diff --git a/src/bacnet/basic/object/device.c b/src/bacnet/basic/object/device.c index a09834bb..35031897 100644 --- a/src/bacnet/basic/object/device.c +++ b/src/bacnet/basic/object/device.c @@ -668,6 +668,7 @@ bool Device_Reinitialize(BACNET_REINITIALIZE_DEVICE_DATA *rd_data) bool status = false; bool password_success = false; unsigned i; + size_t length; /* From 16.4.1.1.2 Password This optional parameter shall be a CharacterString of up to @@ -676,7 +677,12 @@ bool Device_Reinitialize(BACNET_REINITIALIZE_DEVICE_DATA *rd_data) is absent or if the password is incorrect. For those devices that do not require a password, this parameter shall be ignored.*/ if (Reinit_Password && strlen(Reinit_Password) > 0) { - if (characterstring_utf8_length(&rd_data->password) > 20) { + if (characterstring_encoding(&rd_data->password) == CHARACTER_UTF8) { + length = characterstring_utf8_length(&rd_data->password); + } else { + length = characterstring_length(&rd_data->password); + } + if (length > 20) { rd_data->error_class = ERROR_CLASS_SERVICES; rd_data->error_code = ERROR_CODE_PARAMETER_OUT_OF_RANGE; } else if (characterstring_ansi_same( diff --git a/src/bacnet/dcc.c b/src/bacnet/dcc.c index 4dbe1c79..4989e816 100644 --- a/src/bacnet/dcc.c +++ b/src/bacnet/dcc.c @@ -265,10 +265,9 @@ int dcc_decode_service_request( { int apdu_len = 0; int len = 0; - uint8_t tag_number = 0; - uint32_t len_value_type = 0; BACNET_UNSIGNED_INTEGER decoded_unsigned = 0; uint32_t decoded_enum = 0; + size_t password_length; if (apdu && apdu_len_max) { /* Tag 0: timeDuration, in minutes --optional-- */ @@ -290,55 +289,35 @@ int dcc_decode_service_request( results in no timeout */ *timeDuration = 0; } - if ((unsigned)apdu_len < apdu_len_max) { - /* Tag 1: enable_disable */ - len = bacnet_enumerated_context_decode( - &apdu[apdu_len], apdu_len_max - apdu_len, 1, &decoded_enum); - if (len > 0) { - apdu_len += len; - if (enable_disable) { - *enable_disable = - (BACNET_COMMUNICATION_ENABLE_DISABLE)decoded_enum; - } - } else { - return BACNET_STATUS_ABORT; + /* Tag 1: enable_disable */ + len = bacnet_enumerated_context_decode( + &apdu[apdu_len], apdu_len_max - apdu_len, 1, &decoded_enum); + if (len > 0) { + apdu_len += len; + if (enable_disable) { + *enable_disable = + (BACNET_COMMUNICATION_ENABLE_DISABLE)decoded_enum; } + } else { + return BACNET_STATUS_ABORT; } - if ((unsigned)apdu_len < apdu_len_max) { - /* Tag 2: password --optional-- */ - if (!decode_is_context_tag(&apdu[apdu_len], 2)) { - /* since this is the last value of the packet, - if there is data here it must be the specific - context tag number or result in an error */ - return BACNET_STATUS_ABORT; - } - len = bacnet_tag_number_and_value_decode( - &apdu[apdu_len], apdu_len_max - apdu_len, &tag_number, - &len_value_type); - if (len > 0) { - apdu_len += len; - if ((unsigned)apdu_len < apdu_len_max) { - len = bacnet_character_string_decode( - &apdu[apdu_len], apdu_len_max - apdu_len, - len_value_type, password); - if (len > 0) { - size_t password_length = - characterstring_utf8_length(password); - /* UTF-8 code points can be up to 4 bytes long */ - if ((password_length >= 1) && (password_length <= 20)) { - apdu_len += len; - } else { - return BACNET_STATUS_REJECT; - } - } else { - return BACNET_STATUS_ABORT; - } - } else { - return BACNET_STATUS_ABORT; - } + /* Tag 2: password --optional-- */ + len = bacnet_character_string_context_decode( + &apdu[apdu_len], apdu_len_max - apdu_len, 2, password); + if (len > 0) { + if (characterstring_encoding(password) == CHARACTER_UTF8) { + /* UTF-8 code points can be up to 4 bytes long */ + password_length = characterstring_utf8_length(password); } else { - return BACNET_STATUS_ABORT; + password_length = characterstring_length(password); } + if ((password_length >= 1) && (password_length <= 20)) { + apdu_len += len; + } else { + return BACNET_STATUS_REJECT; + } + } else if (len < 0) { + return BACNET_STATUS_ABORT; } else if (password) { /* no optional password - set to NULL */ characterstring_init_ansi(password, NULL); diff --git a/src/bacnet/rd.c b/src/bacnet/rd.c index 367a3a1b..3005a4ef 100644 --- a/src/bacnet/rd.c +++ b/src/bacnet/rd.c @@ -42,6 +42,7 @@ int reinitialize_device_encode( { int len = 0; /* length of each encoding */ int apdu_len = 0; /* total length of the apdu, return value */ + size_t length; /* reinitialized-state-of-device [0] ENUMERATED */ len = encode_context_enumerated(apdu, 0, state); @@ -51,8 +52,13 @@ int reinitialize_device_encode( } /* password [1] CharacterString (SIZE (1..20)) OPTIONAL */ if (password) { - if ((password->length >= 1) && - (characterstring_utf8_length(password) <= 20)) { + if (characterstring_encoding(password) == CHARACTER_UTF8) { + /* UTF-8 code points can be up to 4 bytes long */ + length = characterstring_utf8_length(password); + } else { + length = characterstring_length(password); + } + if ((length >= 1) && (length <= 20)) { len = encode_context_character_string(apdu, 1, password); apdu_len += len; }