From 2b0fac263c03e3d2dc833a016353504f8e3072cc Mon Sep 17 00:00:00 2001 From: skarg Date: Mon, 3 Oct 2011 19:18:27 +0000 Subject: [PATCH] Added Added more explicit error checking on WritePropertyMultiple decoding (untested). --- bacnet-stack/demo/handler/h_wpm.c | 74 ++++++++++++++------------- bacnet-stack/src/wpm.c | 84 ++++++++++++++++++++++--------- 2 files changed, 101 insertions(+), 57 deletions(-) diff --git a/bacnet-stack/demo/handler/h_wpm.c b/bacnet-stack/demo/handler/h_wpm.c index f5c7d270..1c4dae6c 100644 --- a/bacnet-stack/demo/handler/h_wpm.c +++ b/bacnet-stack/demo/handler/h_wpm.c @@ -36,6 +36,7 @@ #include "npdu.h" #include "abort.h" #include "wp.h" +#include "reject.h" #include "wpm.h" /* device object has the handling for all objects */ #include "device.h" @@ -61,9 +62,6 @@ * @param service_data [in] The BACNET_CONFIRMED_SERVICE_DATA information * decoded from the APDU header of this message. */ - - - void handler_write_property_multiple( uint8_t * service_request, uint16_t service_len, @@ -81,13 +79,9 @@ void handler_write_property_multiple( BACNET_ADDRESS my_address; int bytes_sent = 0; - - if (service_data->segmented_message) { - len = - abort_encode_apdu(&Handler_Transmit_Buffer[npdu_len], - service_data->invoke_id, ABORT_REASON_SEGMENTATION_NOT_SUPPORTED, - true); + wp_data.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; + len = BACNET_STATUS_ABORT; #if PRINT_ENABLED fprintf(stderr, "WPM: Segmented message. Sending Abort!\n"); #endif @@ -127,16 +121,14 @@ void handler_write_property_multiple( #endif if (Device_Write_Property(&wp_data) == false) { error = true; - break; /* do while (decoding List of Properties) */ + goto WPM_ABORT; } } else { #if PRINT_ENABLED fprintf(stderr, "WPM: Bad Encoding!\n"); #endif - wp_data.error_class = ERROR_CLASS_PROPERTY; - wp_data.error_code = ERROR_CODE_OTHER; error = true; - break; /* do while (decoding List of Properties) */ + goto WPM_ABORT; } /* Closing tag 1 - List of Properties */ @@ -150,49 +142,63 @@ void handler_write_property_multiple( } while (tag_number != 1); /* end decoding List of Properties for "that" object */ - if (error) - break; /*do while (decode service request) */ + if (error) { + goto WPM_ABORT; + } } } else { #if PRINT_ENABLED fprintf(stderr, "WPM: Bad Encoding!\n"); #endif - wp_data.error_class = ERROR_CLASS_OBJECT; - wp_data.error_code = ERROR_CODE_OTHER; error = true; - break; /*do while (decode service request) */ + goto WPM_ABORT; } - } - while (decode_len < service_len); - + } while (decode_len < service_len); +WPM_ABORT: /* encode the NPDU portion of the packet */ datalink_get_my_address(&my_address); npdu_encode_npdu_data(&npdu_data, false, MESSAGE_PRIORITY_NORMAL); npdu_len = npdu_encode_pdu(&Handler_Transmit_Buffer[0], src, &my_address, &npdu_data); - apdu_len = 0; - - if (error == false) { + /* handle any errors */ + if (error) { + if (len == BACNET_STATUS_ABORT) { + apdu_len = + abort_encode_apdu(&Handler_Transmit_Buffer[npdu_len], + service_data->invoke_id, + abort_convert_error_code(wp_data.error_code), true); +#if PRINT_ENABLED + fprintf(stderr, "WPM: Sending Abort!\n"); +#endif + } else if (len == BACNET_STATUS_ERROR) { + apdu_len = + bacerror_encode_apdu(&Handler_Transmit_Buffer[npdu_len], + service_data->invoke_id, SERVICE_CONFIRMED_WRITE_PROP_MULTIPLE, + wp_data.error_class, wp_data.error_code); +#if PRINT_ENABLED + fprintf(stderr, "WPM: Sending Error!\n"); +#endif + } else if (len == BACNET_STATUS_REJECT) { + apdu_len = + reject_encode_apdu(&Handler_Transmit_Buffer[npdu_len], + service_data->invoke_id, + reject_convert_error_code(wp_data.error_code)); +#if PRINT_ENABLED + fprintf(stderr, "WPM: Sending Reject!\n"); +#endif + } + } else { apdu_len = wpm_ack_encode_apdu_init(&Handler_Transmit_Buffer[npdu_len], service_data->invoke_id); #if PRINT_ENABLED - fprintf(stderr, "WPM: Sending Simple Ack!\n"); -#endif - } else { - apdu_len = - wpm_error_ack_encode_apdu(&Handler_Transmit_Buffer[npdu_len], - service_data->invoke_id, &wp_data); -#if PRINT_ENABLED - fprintf(stderr, "WPM: Sending Error!\n"); + fprintf(stderr, "WPM: Sending Ack!\n"); #endif } - WPM_ABORT: - pdu_len = npdu_len + apdu_len; bytes_sent = datalink_send_pdu(src, &npdu_data, &Handler_Transmit_Buffer[0], diff --git a/bacnet-stack/src/wpm.c b/bacnet-stack/src/wpm.c index 3e0fc7ce..ad2a369e 100644 --- a/bacnet-stack/src/wpm.c +++ b/bacnet-stack/src/wpm.c @@ -33,11 +33,25 @@ /** @file wpm.c Encode/Decode BACnet Write Property Multiple APDUs */ -/* decode service */ +/** Decoding for WritePropertyMultiple service, object ID. + * @ingroup DSWPM + * This handler will be invoked by write_property_multiple handler + * if it has been enabled by a call to apdu_set_confirmed_handler(). + * This function decodes only the first tagged entity, which is + * an object identifier. This function will return an error if: + * - the tag is not the right value + * - the number of bytes is not enough to decode for this entity + * - the subsequent tag number is incorrect + * + * @param apdu [in] The contents of the APDU buffer. + * @param apdu_len [in] The length of the APDU buffer. + * @param data [out] The BACNET_WRITE_PROPERTY_DATA structure + * which will contain the reponse values or error. + */ int wpm_decode_object_id( uint8_t * apdu, uint16_t apdu_len, - BACNET_WRITE_PROPERTY_DATA * data) + BACNET_WRITE_PROPERTY_DATA * wp_data) { uint8_t tag_number = 0; uint32_t len_value = 0; @@ -45,19 +59,36 @@ int wpm_decode_object_id( uint16_t object_type = 0; uint16_t len = 0; - if ((apdu) && (apdu_len)) { + if (apdu && (apdu_len > 5) && wp_data) { /* Context tag 0 - Object ID */ len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); - if (tag_number == 0) { - len += - decode_object_id(&apdu[len], &object_type, &object_instance); - data->object_type = object_type; - data->object_instance = object_instance; - } else - return -1; - } else - return -1; + if ((tag_number == 0) && (apdu_len > len)) { + apdu_len -= len; + if (apdu_len >= 4) { + len += + decode_object_id(&apdu[len], &object_type, &object_instance); + wp_data->object_type = object_type; + wp_data->object_instance = object_instance; + apdu_len -= len; + } else { + wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER; + return BACNET_STATUS_REJECT; + } + } else { + wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG; + return BACNET_STATUS_REJECT; + } + /* just test for the next tag - no need to decode it here */ + /* Context tag 1: sequence of BACnetPropertyValue */ + if (apdu_len && !decode_is_opening_tag_number(&apdu[len], 1)) { + wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG; + return BACNET_STATUS_REJECT; + } + } else { + wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER; + return BACNET_STATUS_REJECT; + } return (int)len; } @@ -78,15 +109,16 @@ int wpm_decode_object_property( wp_data->array_index = BACNET_ARRAY_ALL; wp_data->priority = BACNET_MAX_PRIORITY; wp_data->application_data_len = 0; - /* tag 0 - Property Identifier */ len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); if (tag_number == 0) { len += decode_enumerated(&apdu[len], len_value, &ulVal); wp_data->object_property = ulVal; - } else - return -1; + } else { + wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG; + return BACNET_STATUS_REJECT; + } /* tag 1 - Property Array Index - optional */ len += @@ -108,18 +140,22 @@ int wpm_decode_object_property( len++; /* copy application data */ - for (i = 0; i < wp_data->application_data_len; i++) + for (i = 0; i < wp_data->application_data_len; i++) { wp_data->application_data[i] = apdu[len + i]; + } len += wp_data->application_data_len; - len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); /* closing tag 2 */ - if ((tag_number != 2) && (decode_is_closing_tag(&apdu[len - 1]))) - return -1; - } else - return -1; + if ((tag_number != 2) && (decode_is_closing_tag(&apdu[len - 1]))) { + wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG; + return BACNET_STATUS_REJECT; + } + } else { + wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG; + return BACNET_STATUS_REJECT; + } /* tag 3 - Priority - optional */ len += @@ -129,8 +165,10 @@ int wpm_decode_object_property( wp_data->priority = ulVal; } else len--; - } else - return -1; + } else { + wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER; + return BACNET_STATUS_REJECT; + } return len; }