diff --git a/src/bacnet/bacprop.c b/src/bacnet/bacprop.c index f826d23f..103ff544 100644 --- a/src/bacnet/bacprop.c +++ b/src/bacnet/bacprop.c @@ -62,6 +62,15 @@ PROP_TAG_DATA bacnet_object_device_property_tag_map[] = { { -1, -1 } }; +/** + * Search for the given index in the data list. + * + * @param data_list Pointer to the list. + * @param index Index to search for. + * @param default_ret Default return value. + * + * @return Value found or the default value. + */ signed bacprop_tag_by_index_default( PROP_TAG_DATA *data_list, signed index, signed default_ret) { @@ -80,6 +89,14 @@ signed bacprop_tag_by_index_default( return pUnsigned ? pUnsigned : default_ret; } +/** + * Return the value of the given property, if available. + * + * @param type Object type + * @param prop Property + * + * @return Value found or -1 + */ signed bacprop_property_tag(BACNET_OBJECT_TYPE type, signed prop) { switch (type) { diff --git a/src/bacnet/bacstr.c b/src/bacnet/bacstr.c index 2b31dc75..c1b0e66e 100644 --- a/src/bacnet/bacstr.c +++ b/src/bacnet/bacstr.c @@ -723,22 +723,23 @@ bool octetstring_init( { bool status = false; /* return value */ size_t i; /* counter */ + uint8_t *pb = NULL; if (octet_string && (length <= MAX_OCTET_STRING_BYTES)) { octet_string->length = 0; if (value) { + pb = octet_string->value; for (i = 0; i < MAX_OCTET_STRING_BYTES; i++) { if (i < length) { - octet_string->value[i] = value[i]; + *pb = value[i]; } else { - octet_string->value[i] = 0; + *pb = 0; } + pb++; } octet_string->length = length; } else { - for (i = 0; i < MAX_OCTET_STRING_BYTES; i++) { - octet_string->value[i] = 0; - } + memset(octet_string->value, 0, MAX_OCTET_STRING_BYTES); } status = true; } diff --git a/src/bacnet/basic/binding/address.c b/src/bacnet/basic/binding/address.c index 75de6290..e87c9d80 100644 --- a/src/bacnet/basic/binding/address.c +++ b/src/bacnet/basic/binding/address.c @@ -84,11 +84,23 @@ static struct Address_Cache_Entry { #define BAC_ADDR_SHORT_TIME BAC_ADDR_SECS_1HOUR #define BAC_ADDR_FOREVER 0xFFFFFFFF /* Permenant entry */ +/** + * @brief Set the index of the first (top) address being protected. + * + * @param top_protected_entry_index top protected index [0..n-1] + */ void address_protected_entry_index_set(uint32_t top_protected_entry_index) { - Top_Protected_Entry = top_protected_entry_index; + if (top_protected_entry_index <= (MAX_ADDRESS_CACHE - 1)) { + Top_Protected_Entry = top_protected_entry_index; + } } +/** + * @brief Set the address of our own device. + * + * @param own_id Own device id + */ void address_own_device_id_set(uint32_t own_id) { Own_Device_ID = own_id; @@ -182,9 +194,8 @@ void address_remove_device(uint32_t device_id) * entry available to free up. Does not check for free entries as it is * assumed we are calling this due to the lack of those. * - * @return Candidate for deletation. + * @return Pointer to the entry that has been removed or NULL. */ - static struct Address_Cache_Entry *address_remove_oldest(void) { struct Address_Cache_Entry *pMatch; @@ -383,11 +394,10 @@ static void address_file_init(const char *pFilename) } #endif -/**************************************************************************** - * Clear down the cache and make sure the full complement of entries are * - * available. Assume no persistance of memory. * - ****************************************************************************/ - +/** + * Clear down the cache and make sure the full complement of entries are + * available. Assume no persistance of memory. + */ void address_init(void) { struct Address_Cache_Entry *pMatch; @@ -405,14 +415,13 @@ void address_init(void) return; } -/**************************************************************************** - * Clear down the cache of any non bound, expired or reserved entries. * - * Leave static and unexpired bound entries alone. For use where the cache * - * is held in persistant memory which can survive a reset or power cycle. * - * This reduces the network traffic on restarts as the cache will have much * - * of its entries intact. * - ****************************************************************************/ - +/** + * Clear down the cache of any non bound, expired or reserved entries. + * Leave static and unexpired bound entries alone. For use where the cache + * is held in persistant memory which can survive a reset or power cycle. + * This reduces the network traffic on restarts as the cache will have much + * of its entries intact. + */ void address_init_partial(void) { struct Address_Cache_Entry *pMatch; @@ -441,13 +450,16 @@ void address_init_partial(void) return; } -/**************************************************************************** - * Set the TTL info for the given device entry. If it is a bound entry we * - * set it to static or normal and can change the TTL. If it is unbound we * - * can only set the TTL. This is done as a seperate function at the moment * - * to avoid breaking the current API. * - ****************************************************************************/ - +/** + * Set the TTL info for the given device entry. If it is a bound entry we + * set it to static or normal and can change the TTL. If it is unbound we + * can only set the TTL. This is done as a seperate function at the moment + * to avoid breaking the current API. + * + * @param device_id Device-Id + * @param TimeOut Timeout in seconds. + * @param StaticFlag Flag indicating if being a static address. + */ void address_set_device_TTL( uint32_t device_id, uint32_t TimeOut, bool StaticFlag) { @@ -476,6 +488,13 @@ void address_set_device_TTL( } } +/** + * Return the cached address for the given device-id + * + * @param device_id Device-Id + * @param max_apdu Pointer to a variable, taking the maximum APDU size. + * @param src Pointer to address structure for return. + */ bool address_get_by_device( uint32_t device_id, unsigned *max_apdu, BACNET_ADDRESS *src) { @@ -500,8 +519,14 @@ bool address_get_by_device( return found; } -/* find a device id from a given MAC address */ - +/** + * Find a device id from a given MAC address. + * + * @param src Pointer to address structure to search for. + * @param device_id Pointer to the device id variable for return. + * + * @return true/false + */ bool address_get_device_id(BACNET_ADDRESS *src, uint32_t *device_id) { struct Address_Cache_Entry *pMatch; @@ -525,6 +550,13 @@ bool address_get_device_id(BACNET_ADDRESS *src, uint32_t *device_id) return found; } +/** + * Add a device using the given id, max_apdu and address. + * + * @param device_id Pointer to the device id variable for return. + * @param max_apdu Maximum APDU size. + * @param src Pointer to address structure to add. + */ void address_add(uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src) { bool found = false; /* return value */ @@ -543,6 +575,7 @@ void address_add(uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src) /* existing device or bind request outstanding - update address */ pMatch = Address_Cache; while (pMatch <= &Address_Cache[MAX_ADDRESS_CACHE - 1]) { + /* Device already in the list, then update the values. */ if (((pMatch->Flags & BAC_ADDR_IN_USE) != 0) && (pMatch->device_id == device_id)) { bacnet_address_copy(&pMatch->address, src); @@ -572,7 +605,7 @@ void address_add(uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src) pMatch++; } - /* new device - add to cache if there is room */ + /* New device - add to cache if there is room. */ if (!found) { pMatch = Address_Cache; while (pMatch <= &Address_Cache[MAX_ADDRESS_CACHE - 1]) { @@ -591,7 +624,7 @@ void address_add(uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src) } } - /* See if we can squeeze it in */ + /* If adding has failed, see if we can squeeze it in by removed the oldest entry. */ if (!found) { pMatch = address_remove_oldest(); if (pMatch != NULL) { @@ -606,8 +639,19 @@ void address_add(uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src) return; } -/* returns true if device is already bound */ -/* also returns the address and max apdu if already bound */ +/** + * Check if the device is in the list. If yes, return the values. + * Otherwise add the device to the list. + * Returns true if device is already bound. + * Also returns the address and max apdu if already bound. + * + * @param device_id Pointer to the device id variable for return. + * @param device_ttl Pointer to a variable taking the time to live in seconds. + * @param max_apdu Max APDU size of the device. + * @param src Pointer to the BACnet address. + * + * @return true if device is already bound + */ bool address_device_bind_request(uint32_t device_id, uint32_t *device_ttl, unsigned *max_apdu, @@ -675,14 +719,31 @@ bool address_device_bind_request(uint32_t device_id, return (false); } -/* returns true if device is already bound */ -/* also returns the address and max apdu if already bound */ +/** + * Check if the device is in the list. If yes, return the values. + * Otherwise add the device to the list. + * Returns true if device is already bound. + * Also returns the address and max apdu if already bound. + * + * @param device_id Pointer to the device id variable for return. + * @param max_apdu Max APDU size of the device. + * @param src Pointer to the BACnet address. + * + * @return true if device is already bound + */ bool address_bind_request( uint32_t device_id, unsigned *max_apdu, BACNET_ADDRESS *src) { return address_device_bind_request(device_id, NULL, max_apdu, src); } +/** + * For an existing device add a binding. + * + * @param device_id Pointer to the device id variable for return. + * @param max_apdu Max APDU size of the device. + * @param src Pointer to the BACnet address. + */ void address_add_binding( uint32_t device_id, unsigned max_apdu, BACNET_ADDRESS *src) { @@ -709,6 +770,17 @@ void address_add_binding( return; } +/** + * Return the device information from the given index in the table. + * + * @param index Table index [0..MAX_ADDRESS_CACHE-1] + * @param device_id Pointer to the variable taking the device id. + * @param device_ttl Pointer to the variable taking the Time To Life for the device. + * @param max_apdu Pointer to the variable taking the max APDU size of the device. + * @param src Pointer to the BACnet address. + * + * @return true/false + */ bool address_device_get_by_index(unsigned index, uint32_t *device_id, uint32_t *device_ttl, @@ -741,6 +813,16 @@ bool address_device_get_by_index(unsigned index, return found; } +/** + * Return the device information from the given index in the table. + * + * @param index Table index [0..MAX_ADDRESS_CACHE-1] + * @param device_id Pointer to the variable taking the device id. + * @param max_apdu Pointer to the variable taking the max APDU size of the device. + * @param src Pointer to the BACnet address. + * + * @return true/false + */ bool address_get_by_index(unsigned index, uint32_t *device_id, unsigned *max_apdu, @@ -749,6 +831,11 @@ bool address_get_by_index(unsigned index, return address_device_get_by_index(index, device_id, NULL, max_apdu, src); } +/** + * Return the count of cached addresses. + * + * @return A value between zero and MAX_ADDRESS_CACHE. + */ unsigned address_count(void) { struct Address_Cache_Entry *pMatch; @@ -768,43 +855,55 @@ unsigned address_count(void) return count; } -/**************************************************************************** - * Build a list of the current bindings for the device address binding * - * property. * - ****************************************************************************/ - +/** + * Build a list of the current bindings for the device address binding + * property. Basically encode the address list to be send out. + * + * @param apdu Pointer to the APDU + * @param apdu_len Remaining buffer size. + * + * @return Count of encoded bytes. + */ int address_list_encode(uint8_t *apdu, unsigned apdu_len) { int iLen = 0; struct Address_Cache_Entry *pMatch; BACNET_OCTET_STRING MAC_Address; - /* FIXME: I really shouild check the length remaining here but it is - fairly pointless until we have the true length remaining in - the packet to work with as at the moment it is just MAX_APDU */ - (void)apdu_len; - /* look for matching address */ + /* Look for matching address. */ pMatch = Address_Cache; while (pMatch <= &Address_Cache[MAX_ADDRESS_CACHE - 1]) { if ((pMatch->Flags & (BAC_ADDR_IN_USE | BAC_ADDR_BIND_REQ)) == BAC_ADDR_IN_USE) { iLen += encode_application_object_id( - &apdu[iLen], OBJECT_DEVICE, pMatch->device_id); - iLen += - encode_application_unsigned(&apdu[iLen], pMatch->address.net); + &apdu[iLen], OBJECT_DEVICE, pMatch->device_id); + iLen += encode_application_unsigned(&apdu[iLen], pMatch->address.net); + if ((unsigned)iLen >= apdu_len) { + break; + } /* pick the appropriate type of entry from the cache */ if (pMatch->address.len != 0) { + /* BAC */ + if ((unsigned)(iLen + pMatch->address.len) >= apdu_len) { + break; + } octetstring_init( &MAC_Address, pMatch->address.adr, pMatch->address.len); - iLen += - encode_application_octet_string(&apdu[iLen], &MAC_Address); + iLen += encode_application_octet_string(&apdu[iLen], &MAC_Address); } else { + /* MAC*/ + if ((unsigned)(iLen + pMatch->address.mac_len) >= apdu_len) { + break; + } octetstring_init( &MAC_Address, pMatch->address.mac, pMatch->address.mac_len); - iLen += - encode_application_octet_string(&apdu[iLen], &MAC_Address); + iLen += encode_application_octet_string(&apdu[iLen], &MAC_Address); + } + /* Any space left? */ + if ((unsigned)iLen >= apdu_len) { + break; } } pMatch++; @@ -813,29 +912,33 @@ int address_list_encode(uint8_t *apdu, unsigned apdu_len) return (iLen); } -/**************************************************************************** - * Build a list of the current bindings for the device address binding * - * property as required for the ReadsRange functionality. * - * We assume we only get called for "Read All" or "By Position" requests. * - * * - * We need to treat the address cache as a contiguous array but in reality * - * it could be sparsely populated. We can get the count but we can only * - * extract entries by doing a linear scan starting from the first entry in * - * the cache and picking them off one by one. * - * * - * We do assume the list cannot change whilst we are accessing it so would * - * not be multithread safe if there are other tasks that change the cache. * - * * +/** + * Build a list of the current bindings for the device address binding + * property as required for the ReadsRange functionality. + * We assume we only get called for "Read All" or "By Position" requests. + * + * We need to treat the address cache as a contiguous array but in reality + * it could be sparsely populated. We can get the count but we can only + * extract entries by doing a linear scan starting from the first entry in + * the cache and picking them off one by one. + * + * We do assume the list cannot change whilst we are accessing it so would + * not be multithread safe if there are other tasks that change the cache. + * * We take the simple approach here to filling the buffer by taking a max * - * size for a single entry and then stopping if there is less than that * - * left in the buffer. You could build each entry in a seperate buffer and * - * determine the exact length before copying but this is time consuming, * - * requires more memory and would probably only let you sqeeeze one more * - * entry in on occasion. The value is calculated as 5 bytes for the device * - * ID + 3 bytes for the network number and nine bytes for the MAC address * - * oct string to give 17 bytes (the minimum possible is 5 + 2 + 3 = 10). * - ****************************************************************************/ - + * size for a single entry and then stopping if there is less than that + * left in the buffer. You could build each entry in a seperate buffer and + * determine the exact length before copying but this is time consuming, + * requires more memory and would probably only let you sqeeeze one more + * entry in on occasion. The value is calculated as 5 bytes for the device + * ID + 3 bytes for the network number and nine bytes for the MAC address + * oct string to give 17 bytes (the minimum possible is 5 + 2 + 3 = 10). + * + * @param apdu Pointer to the encode buffer. + * @param pRequest Pointer to the read_range request structure. + * + * @return Bytes encoded. + */ #define ACACHE_MAX_ENC 17 /* Maximum size of encoded cache entry, see above */ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest) @@ -851,6 +954,10 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest) uint32_t uiTarget = 0; /* Last entry we are required to encode */ uint32_t uiRemaining = 0; /* Amount of unused space in packet */ + if ((!pRequest) || (!apdu)) { + return 0; + } + /* Initialise result flags to all false */ bitstring_init(&pRequest->ResultFlags); bitstring_set_bit(&pRequest->ResultFlags, RESULT_FLAG_FIRST_ITEM, false); @@ -861,7 +968,7 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest) pRequest->ItemCount = 0; /* Start out with nothing */ uiTotal = address_count(); /* What do we have to work with here ? */ - if (uiTotal == 0) { /* Bail out now if nowt */ + if (uiTotal == 0) { /* Bail out now if not. */ return (0); } @@ -921,6 +1028,10 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest) while ((pMatch->Flags & (BAC_ADDR_IN_USE | BAC_ADDR_BIND_REQ)) != BAC_ADDR_IN_USE) { /* Find first bound entry */ pMatch++; + /* Shall not happen as the count has been checked first. */ + if (pMatch > &Address_Cache[MAX_ADDRESS_CACHE - 1]) { + return(0); /* Issue with the table. */ + } } /* Seek to start position */ @@ -932,6 +1043,10 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest) } else { pMatch++; } + /* Shall not happen as the count has been checked first. */ + if (pMatch > &Address_Cache[MAX_ADDRESS_CACHE - 1]) { + return(0); /* Issue with the table. */ + } } uiFirst = uiIndex; /* Record where we started from */ @@ -976,6 +1091,10 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest) while ((pMatch->Flags & (BAC_ADDR_IN_USE | BAC_ADDR_BIND_REQ)) != BAC_ADDR_IN_USE) { /* Find next bound entry */ pMatch++; + /* Can normally not happen. */ + if (pMatch > &Address_Cache[MAX_ADDRESS_CACHE - 1]) { + return(0); /* Issue with the table. */ + } } } @@ -991,15 +1110,16 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest) return (iLen); } -/**************************************************************************** - * Scan the cache and eliminate any expired entries. Should be called * - * periodically to ensure the cache is managed correctly. If this function * - * is never called at all the whole cache is effectivly rendered static and * - * entries never expire unless explictely deleted. * - ****************************************************************************/ - +/** + * Scan the cache and eliminate any expired entries. Should be called + * periodically to ensure the cache is managed correctly. If this function + * is never called at all the whole cache is effectivly rendered static and + * entries never expire unless explictely deleted. + * + * @param uSeconds Approximate number of seconds since last call to this function + */ void address_cache_timer(uint16_t uSeconds) -{ /* Approximate number of seconds since last call to this function */ +{ struct Address_Cache_Entry *pMatch; pMatch = Address_Cache; diff --git a/src/bacnet/basic/object/bv.c b/src/bacnet/basic/object/bv.c index fc5494db..90fbf5c8 100644 --- a/src/bacnet/basic/object/bv.c +++ b/src/bacnet/basic/object/bv.c @@ -422,7 +422,7 @@ bool Binary_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) if (wp_data == NULL) { return false; } - if ((wp_data->application_data == NULL) || \ + if ((wp_data->application_data == NULL) || (wp_data->application_data_len == 0)) { return false; } diff --git a/src/bacnet/basic/object/csv.c b/src/bacnet/basic/object/csv.c index bd2a6dab..02c6a71b 100644 --- a/src/bacnet/basic/object/csv.c +++ b/src/bacnet/basic/object/csv.c @@ -384,7 +384,7 @@ int CharacterString_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) if (rpdata == NULL) { return 0; } - if ((rpdata->application_data == NULL) || \ + if ((rpdata->application_data == NULL) || (rpdata->application_data_len == 0)) { return 0; } @@ -488,7 +488,7 @@ bool CharacterString_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) if (wp_data == NULL) { return false; } - if ((wp_data->application_data == NULL) || \ + if ((wp_data->application_data == NULL) || (wp_data->application_data_len == 0)) { return false; } diff --git a/src/bacnet/basic/service/h_rpm.c b/src/bacnet/basic/service/h_rpm.c index c76ad7e9..7a498c01 100644 --- a/src/bacnet/basic/service/h_rpm.c +++ b/src/bacnet/basic/service/h_rpm.c @@ -182,6 +182,7 @@ void handler_read_property_multiple(uint8_t *service_request, BACNET_ADDRESS *src, BACNET_CONFIRMED_SERVICE_DATA *service_data) { + bool berror = false; int len = 0; uint16_t copy_len = 0; uint16_t decode_len = 0; @@ -194,234 +195,258 @@ void handler_read_property_multiple(uint8_t *service_request, int npdu_len = 0; int error = 0; - /* jps_debug - see if we are utilizing all the buffer */ - /* memset(&Handler_Transmit_Buffer[0], 0xff, - * sizeof(Handler_Transmit_Buffer)); */ - /* 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); - if (service_data->segmented_message) { - rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; - error = BACNET_STATUS_ABORT; -#if PRINT_ENABLED - fprintf(stderr, "RPM: Segmented message. Sending Abort!\r\n"); -#endif - goto RPM_FAILURE; - } - /* decode apdu request & encode apdu reply - encode complex ack, invoke id, service choice */ - apdu_len = rpm_ack_encode_apdu_init( - &Handler_Transmit_Buffer[npdu_len], service_data->invoke_id); - for (;;) { - /* Start by looking for an object ID */ - len = rpm_decode_object_id( - &service_request[decode_len], service_len - decode_len, &rpmdata); - if (len >= 0) { - /* Got one so skip to next stage */ - decode_len += len; - } else { - /* bad encoding - skip to error/reject/abort handling */ -#if PRINT_ENABLED - fprintf(stderr, "RPM: Bad Encoding.\n"); -#endif - error = len; - goto RPM_FAILURE; - } + if (service_data && (service_len > 0)) { + /* jps_debug - see if we are utilizing all the buffer */ + /* memset(&Handler_Transmit_Buffer[0], 0xff, + * sizeof(Handler_Transmit_Buffer)); */ + /* 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); - /* Test for case of indefinite Device object instance */ - if ((rpmdata.object_type == OBJECT_DEVICE) && - (rpmdata.object_instance == BACNET_MAX_INSTANCE)) { - rpmdata.object_instance = Device_Object_Instance_Number(); - } - - /* Stick this object id into the reply - if it will fit */ - len = rpm_ack_encode_apdu_object_begin(&Temp_Buf[0], &rpmdata); - copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len], &Temp_Buf[0], - apdu_len, len, MAX_APDU); - if (copy_len == 0) { -#if PRINT_ENABLED - fprintf(stderr, "RPM: Response too big!\r\n"); -#endif + if (service_data->segmented_message) { rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; error = BACNET_STATUS_ABORT; - goto RPM_FAILURE; - } + #if PRINT_ENABLED + fprintf(stderr, "RPM: Segmented message. Sending Abort!\r\n"); + #endif + } else { + /* decode apdu request & encode apdu reply + encode complex ack, invoke id, service choice */ + apdu_len = rpm_ack_encode_apdu_init( + &Handler_Transmit_Buffer[npdu_len], service_data->invoke_id); - apdu_len += copy_len; - /* do each property of this object of the RPM request */ - for (;;) { - /* Fetch a property */ - len = rpm_decode_object_property(&service_request[decode_len], - service_len - decode_len, &rpmdata); - if (len < 0) { - /* bad encoding - skip to error/reject/abort handling */ -#if PRINT_ENABLED - fprintf(stderr, "RPM: Bad Encoding.\n"); -#endif - error = len; - goto RPM_FAILURE; - } - decode_len += len; - /* handle the special properties */ - if ((rpmdata.object_property == PROP_ALL) || - (rpmdata.object_property == PROP_REQUIRED) || - (rpmdata.object_property == PROP_OPTIONAL)) { - struct special_property_list_t property_list; - unsigned property_count = 0; - unsigned index = 0; - BACNET_PROPERTY_ID special_object_property; - - if (rpmdata.array_index != BACNET_ARRAY_ALL) { - /* No array index options for this special property. - Encode error for this object property response */ - len = rpm_ack_encode_apdu_object_property(&Temp_Buf[0], - rpmdata.object_property, rpmdata.array_index); - copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len], - &Temp_Buf[0], apdu_len, len, MAX_APDU); - if (copy_len == 0) { -#if PRINT_ENABLED - fprintf( - stderr, "RPM: Too full to encode property!\r\n"); -#endif - rpmdata.error_code = - ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; - error = BACNET_STATUS_ABORT; - goto RPM_FAILURE; - } - apdu_len += len; - len = rpm_ack_encode_apdu_object_property_error( - &Temp_Buf[0], ERROR_CLASS_PROPERTY, - ERROR_CODE_PROPERTY_IS_NOT_AN_ARRAY); - copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len], - &Temp_Buf[0], apdu_len, len, MAX_APDU); - if (copy_len == 0) { -#if PRINT_ENABLED - fprintf(stderr, "RPM: Too full to encode error!\r\n"); -#endif - rpmdata.error_code = - ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; - error = BACNET_STATUS_ABORT; - goto RPM_FAILURE; - } - apdu_len += len; + for (;;) { + /* Start by looking for an object ID */ + len = rpm_decode_object_id( + &service_request[decode_len], service_len - decode_len, &rpmdata); + if (len >= 0) { + /* Got one so skip to next stage */ + decode_len += len; } else { - special_object_property = rpmdata.object_property; - Device_Objects_Property_List(rpmdata.object_type, - rpmdata.object_instance, &property_list); - property_count = RPM_Object_Property_Count( - &property_list, special_object_property); - if (property_count == 0) { - /* this only happens with the OPTIONAL property */ - /* 135-2016bl-2. Clarify ReadPropertyMultiple - response on OPTIONAL when empty. */ - /* If no optional properties are supported then - an empty 'List of Results' shall be returned - for the specified property.*/ - } else { - for (index = 0; index < property_count; index++) { - rpmdata.object_property = RPM_Object_Property( - &property_list, special_object_property, index); - len = RPM_Encode_Property( - &Handler_Transmit_Buffer[npdu_len], - (uint16_t)apdu_len, MAX_APDU, &rpmdata); - if (len > 0) { - apdu_len += len; - } else { -#if PRINT_ENABLED + /* bad encoding - skip to error/reject/abort handling */ + #if PRINT_ENABLED + fprintf(stderr, "RPM: Bad Encoding.\n"); + #endif + error = len; + berror = true; + break; + } + + /* Test for case of indefinite Device object instance */ + if ((rpmdata.object_type == OBJECT_DEVICE) && + (rpmdata.object_instance == BACNET_MAX_INSTANCE)) { + rpmdata.object_instance = Device_Object_Instance_Number(); + } + + /* Stick this object id into the reply - if it will fit */ + len = rpm_ack_encode_apdu_object_begin(&Temp_Buf[0], &rpmdata); + copy_len = + memcopy(&Handler_Transmit_Buffer[npdu_len], &Temp_Buf[0], + apdu_len, len, MAX_APDU); + if (copy_len == 0) { + #if PRINT_ENABLED + fprintf(stderr, "RPM: Response too big!\r\n"); + #endif + rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; + error = BACNET_STATUS_ABORT; + berror = true; + break; + } + + apdu_len += copy_len; + /* do each property of this object of the RPM request */ + for (;;) { + /* Fetch a property */ + len = rpm_decode_object_property(&service_request[decode_len], + service_len - decode_len, &rpmdata); + if (len < 0) { + /* bad encoding - skip to error/reject/abort handling */ + #if PRINT_ENABLED + fprintf(stderr, "RPM: Bad Encoding.\n"); + #endif + error = len; + berror = true; + break; // The berror flag ensures that both loops will be broken! + } + decode_len += len; + /* handle the special properties */ + if ((rpmdata.object_property == PROP_ALL) || + (rpmdata.object_property == PROP_REQUIRED) || + (rpmdata.object_property == PROP_OPTIONAL)) { + struct special_property_list_t property_list; + unsigned property_count = 0; + unsigned index = 0; + BACNET_PROPERTY_ID special_object_property; + + if (rpmdata.array_index != BACNET_ARRAY_ALL) { + + /* No array index options for this special property. + Encode error for this object property response */ + len = rpm_ack_encode_apdu_object_property(&Temp_Buf[0], + rpmdata.object_property, rpmdata.array_index); + + copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len], + &Temp_Buf[0], apdu_len, len, MAX_APDU); + + if (copy_len == 0) { + #if PRINT_ENABLED fprintf( - stderr, "RPM: Too full for property!\r\n"); -#endif - error = len; - goto RPM_FAILURE; + stderr, "RPM: Too full to encode property!\r\n"); + #endif + rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; + error = BACNET_STATUS_ABORT; + berror = true; + break; // The berror flag ensures that both loops will be broken! + } + + apdu_len += len; + len = rpm_ack_encode_apdu_object_property_error( + &Temp_Buf[0], ERROR_CLASS_PROPERTY, + ERROR_CODE_PROPERTY_IS_NOT_AN_ARRAY); + + copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len], + &Temp_Buf[0], apdu_len, len, MAX_APDU); + + if (copy_len == 0) { + #if PRINT_ENABLED + fprintf(stderr, "RPM: Too full to encode error!\r\n"); + #endif + rpmdata.error_code = + ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; + error = BACNET_STATUS_ABORT; + berror = true; + break; // The berror flag ensures that both loops will be broken! + } + apdu_len += len; + } else { + special_object_property = rpmdata.object_property; + Device_Objects_Property_List(rpmdata.object_type, + rpmdata.object_instance, &property_list); + property_count = RPM_Object_Property_Count( + &property_list, special_object_property); + + if (property_count == 0) { + /* This only happens with the OPTIONAL property */ + /* 135-2016bl-2. Clarify ReadPropertyMultiple + response on OPTIONAL when empty. */ + /* If no optional properties are supported then + an empty 'List of Results' shall be returned + for the specified property.*/ + } else { + for (index = 0; index < property_count; index++) { + rpmdata.object_property = RPM_Object_Property( + &property_list, special_object_property, index); + len = RPM_Encode_Property( + &Handler_Transmit_Buffer[npdu_len], + (uint16_t)apdu_len, MAX_APDU, &rpmdata); + if (len > 0) { + apdu_len += len; + } else { + #if PRINT_ENABLED + fprintf( + stderr, "RPM: Too full for property!\r\n"); + #endif + error = len; + berror = true; + break; // The berror flag ensures that both loops will be broken! + } + } } } + } else { + /* handle an individual property */ + len = RPM_Encode_Property(&Handler_Transmit_Buffer[npdu_len], + (uint16_t)apdu_len, MAX_APDU, &rpmdata); + if (len > 0) { + apdu_len += len; + } else { + #if PRINT_ENABLED + fprintf( + stderr, "RPM: Too full for individual property!\r\n"); + #endif + error = len; + berror = true; + break; // The berror flag ensures that both loops will be broken! + } } + + if (decode_is_closing_tag_number(&service_request[decode_len], 1)) { + /* Reached end of property list so cap the result list */ + decode_len++; + len = rpm_ack_encode_apdu_object_end(&Temp_Buf[0]); + copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len], + &Temp_Buf[0], apdu_len, len, MAX_APDU); + if (copy_len == 0) { + #if PRINT_ENABLED + fprintf(stderr, "RPM: Too full to encode object end!\r\n"); + #endif + rpmdata.error_code = + ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; + error = BACNET_STATUS_ABORT; + berror = true; + break; // The berror flag ensures that both loops will be broken! + } else { + apdu_len += copy_len; + } + break; /* finished with this property list */ + } + } // for(;;) + if (berror) { + break; } - } else { - /* handle an individual property */ - len = RPM_Encode_Property(&Handler_Transmit_Buffer[npdu_len], - (uint16_t)apdu_len, MAX_APDU, &rpmdata); - if (len > 0) { - apdu_len += len; - } else { -#if PRINT_ENABLED - fprintf( - stderr, "RPM: Too full for individual property!\r\n"); -#endif - error = len; - goto RPM_FAILURE; + if (decode_len >= service_len) { + /* Reached the end so finish up */ + break; } - } - if (decode_is_closing_tag_number(&service_request[decode_len], 1)) { - /* Reached end of property list so cap the result list */ - decode_len++; - len = rpm_ack_encode_apdu_object_end(&Temp_Buf[0]); - copy_len = memcopy(&Handler_Transmit_Buffer[npdu_len], - &Temp_Buf[0], apdu_len, len, MAX_APDU); - if (copy_len == 0) { -#if PRINT_ENABLED - fprintf(stderr, "RPM: Too full to encode object end!\r\n"); -#endif - rpmdata.error_code = - ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; + } // for(;;) + + /* If not having an error so far, check the remaining space. */ + if (!berror) { + if (apdu_len > service_data->max_resp) { + /* too big for the sender - send an abort */ + rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; error = BACNET_STATUS_ABORT; - goto RPM_FAILURE; - } else { - apdu_len += copy_len; + #if PRINT_ENABLED + fprintf(stderr, "RPM: Message too large. Sending Abort!\n"); + #endif } - break; /* finished with this property list */ } } - if (decode_len >= service_len) { - /* Reached the end so finish up */ - break; + + /* Error fallback. */ + if (error) { + if (error == BACNET_STATUS_ABORT) { + apdu_len = abort_encode_apdu(&Handler_Transmit_Buffer[npdu_len], + service_data->invoke_id, + abort_convert_error_code(rpmdata.error_code), true); + #if PRINT_ENABLED + fprintf(stderr, "RPM: Sending Abort!\n"); + #endif + } else if (error == BACNET_STATUS_ERROR) { + apdu_len = bacerror_encode_apdu(&Handler_Transmit_Buffer[npdu_len], + service_data->invoke_id, SERVICE_CONFIRMED_READ_PROP_MULTIPLE, + rpmdata.error_class, rpmdata.error_code); + #if PRINT_ENABLED + fprintf(stderr, "RPM: Sending Error!\n"); + #endif + } else if (error == BACNET_STATUS_REJECT) { + apdu_len = reject_encode_apdu(&Handler_Transmit_Buffer[npdu_len], + service_data->invoke_id, + reject_convert_error_code(rpmdata.error_code)); + #if PRINT_ENABLED + fprintf(stderr, "RPM: Sending Reject!\n"); + #endif + } } - } - if (apdu_len > service_data->max_resp) { - /* too big for the sender - send an abort */ - rpmdata.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; - error = BACNET_STATUS_ABORT; -#if PRINT_ENABLED - fprintf(stderr, "RPM: Message too large. Sending Abort!\n"); -#endif - goto RPM_FAILURE; - } - -RPM_FAILURE: - if (error) { - if (error == BACNET_STATUS_ABORT) { - apdu_len = abort_encode_apdu(&Handler_Transmit_Buffer[npdu_len], - service_data->invoke_id, - abort_convert_error_code(rpmdata.error_code), true); -#if PRINT_ENABLED - fprintf(stderr, "RPM: Sending Abort!\n"); -#endif - } else if (error == BACNET_STATUS_ERROR) { - apdu_len = bacerror_encode_apdu(&Handler_Transmit_Buffer[npdu_len], - service_data->invoke_id, SERVICE_CONFIRMED_READ_PROP_MULTIPLE, - rpmdata.error_class, rpmdata.error_code); -#if PRINT_ENABLED - fprintf(stderr, "RPM: Sending Error!\n"); -#endif - } else if (error == BACNET_STATUS_REJECT) { - apdu_len = reject_encode_apdu(&Handler_Transmit_Buffer[npdu_len], - service_data->invoke_id, - reject_convert_error_code(rpmdata.error_code)); -#if PRINT_ENABLED - fprintf(stderr, "RPM: Sending Reject!\n"); -#endif + pdu_len = apdu_len + npdu_len; + bytes_sent = datalink_send_pdu(src, &npdu_data, &Handler_Transmit_Buffer[0], pdu_len); + if (bytes_sent <= 0) { + #if PRINT_ENABLED + fprintf(stderr, "RPM: Failed to send PDU (%s)!\n", strerror(errno)); + #endif } } - - pdu_len = apdu_len + npdu_len; - bytes_sent = datalink_send_pdu( - src, &npdu_data, &Handler_Transmit_Buffer[0], pdu_len); - if (bytes_sent <= 0) { -#if PRINT_ENABLED - fprintf(stderr, "RPM: Failed to send PDU (%s)!\n", strerror(errno)); -#endif - } } diff --git a/src/bacnet/basic/service/h_wp.c b/src/bacnet/basic/service/h_wp.c index c173c83f..27145cfd 100644 --- a/src/bacnet/basic/service/h_wp.c +++ b/src/bacnet/basic/service/h_wp.c @@ -136,7 +136,7 @@ void handler_write_property(uint8_t *service_request, /* Send PDU */ pdu_len += len; - bytes_sent = datalink_send_pdu( \ + bytes_sent = datalink_send_pdu( src, &npdu_data, &Handler_Transmit_Buffer[0], pdu_len); if (bytes_sent <= 0) { #if PRINT_ENABLED diff --git a/src/bacnet/basic/sys/keylist.c b/src/bacnet/basic/sys/keylist.c index 7478d6ba..39eb191e 100644 --- a/src/bacnet/basic/sys/keylist.c +++ b/src/bacnet/basic/sys/keylist.c @@ -56,21 +56,33 @@ /* Generic node routines */ /******************************************************************** */ -/* grab memory for a node */ +/** Grab memory for a node (Keylist_Node). + * + * @return Pointer to the allocated memory or + * NULL under an Out Of Memory situation. + */ static struct Keylist_Node *NodeCreate(void) { return calloc(1, sizeof(struct Keylist_Node)); } -/* grab memory for a list */ +/** Grab memory for a list (Keylist). + * + * @return Pointer to the allocated memory or + * NULL under an Out Of Memory situation. + */ static struct Keylist *KeylistCreate(void) { return calloc(1, sizeof(struct Keylist)); } -/* check to see if the array is big enough for an addition */ -/* or is too big when we are deleting and we can shrink */ -/* returns TRUE if success, FALSE if failed */ +/** Check to see if the array is big enough for an addition + * or is too big when we are deleting and we can shrink. + * + * @param list Pointer to the list to be tested. + * + * @return Returns TRUE if success, FALSE if failed + */ static int CheckArraySize(OS_Keylist list) { int new_size = 0; /* set it up so that no size change is the default */ @@ -112,13 +124,20 @@ static int CheckArraySize(OS_Keylist list) return TRUE; } -/* find the index of the key that we are looking for */ -/* since it is sorted, we can optimize the search */ -/* returns TRUE if found, and FALSE not found */ -/* returns the found key and the index where it was found in parameters */ -/* If the key is not found, the nearest index from the bottom will be returned, +/** Find the index of the key that we are looking for. + * Since it is sorted, we can optimize the search. + * returns TRUE if found, and FALSE not found. + * Returns the found key and the index where it was found in parameters. + * If the key is not found, the nearest index from the bottom will be returned, + * allowing the ability to find where an key should go into the list. + * + * @param list Pointer to the list + * @param key Key to search for + * @param pIndex Pointer to the variable taking the index were the key + * had been found. + * + * @return TRUE if found, and FALSE if not */ -/* allowing the ability to find where an key should go into the list. */ static int FindIndex(OS_Keylist list, KEY key, int *pIndex) { struct Keylist_Node *node; /* holds the new node */ @@ -127,7 +146,12 @@ static int FindIndex(OS_Keylist list, KEY key, int *pIndex) int index = 0; /* our current search place in the array */ KEY current_key = 0; /* place holder for current node key */ int status = FALSE; /* return value */ - if (!list || !list->array || !list->count) { + + if (!list) { + *pIndex = 0; + return (FALSE); + } + if (!list->array || !list->count) { *pIndex = 0; return (FALSE); } @@ -148,12 +172,12 @@ static int FindIndex(OS_Keylist list, KEY key, int *pIndex) left = index + 1; } } while ((key != current_key) && (left <= right)); + if (key == current_key) { status = TRUE; *pIndex = index; - } - else { + } else { /* where the index should be */ if (key > current_key) { *pIndex = index + 1; @@ -168,7 +192,15 @@ static int FindIndex(OS_Keylist list, KEY key, int *pIndex) /******************************************************************** */ /* list data functions */ /******************************************************************** */ -/* inserts a node into its sorted position */ +/** Inserts a node into its sorted position. + * + * @param list Pointer to the list + * @param key Key to be inserted + * @param data Pointer to the data hold by the key. + * This pointer needs to be poiting to static memory + * as it will be stored in the list and later used + * by retrieving the key again. + */ int Keylist_Data_Add(OS_Keylist list, KEY key, void *data) { struct Keylist_Node *node; /* holds the new node */ @@ -191,9 +223,7 @@ int Keylist_Data_Add(OS_Keylist list, KEY key, void *data) for (i = list->count; i > index; i--) { list->array[i] = list->array[i - 1]; } - } - - else { + } else { index = 0; } @@ -209,138 +239,199 @@ int Keylist_Data_Add(OS_Keylist list, KEY key, void *data) return index; } -/* deletes a node specified by its index */ -/* returns the data from the node */ +/** Deletes a node specified by its index + * returns the data from the node + * + * @param list Pointer to the list + * @param index Index of the key to be deleted + * + * @returns Pointer to the data of the deleted key or NULL. + */ void *Keylist_Data_Delete_By_Index(OS_Keylist list, int index) { struct Keylist_Node *node; void *data = NULL; - if (list && list->array && list->count && (index >= 0) && - (index < list->count)) { - node = list->array[index]; - if (node) - data = node->data; - - /* move the nodes to account for the deleted one */ - if (list->count == 1) { - /* There is no node shifting to do */ - } else if (index == (list->count - 1)) { - /* We are the last one */ - /* There is no node shifting to do */ - } else { - /* Move all the nodes down one */ - int i; /* counter */ - int count = list->count - 1; - for (i = index; i < count; i++) { - list->array[i] = list->array[i + 1]; + if (list) { + if (list->array && list->count && (index >= 0) && + (index < list->count)) { + node = list->array[index]; + if (node) { + data = node->data; + } + /* move the nodes to account for the deleted one */ + if (list->count == 1) { + /* There is no node shifting to do */ + } else if (index == (list->count - 1)) { + /* We are the last one */ + /* There is no node shifting to do */ + } else { + /* Move all the nodes down one */ + int i; /* counter */ + int count = list->count - 1; + for (i = index; i < count; i++) { + list->array[i] = list->array[i + 1]; + } + } + list->count--; + if (node) { + free(node); } - } - list->count--; - if (node) { - free(node); - } - /* potentially reduce the size of the array */ - (void)CheckArraySize(list); + /* potentially reduce the size of the array */ + (void)CheckArraySize(list); + } } return (data); } -/* deletes a node specified by its key */ -/* returns the data from the node */ +/** Deletes a node specified by its key/ + * returns the data from the node + * + * @param list Pointer to the list + * @param key Key to be deleted + * + * @returns Pointer to the data of the deleted key or NULL. + */ void *Keylist_Data_Delete(OS_Keylist list, KEY key) { void *data = NULL; /* return value */ int index; /* where the node is in the array */ if (list) { - if (FindIndex(list, key, &index)) + if (FindIndex(list, key, &index)) { data = Keylist_Data_Delete_By_Index(list, index); + } } return data; } -/* returns the data from last node, and removes it from the list */ +/** Returns the data from last node, and + * removes it from the list. + * + * @param list Pointer to the list + * + * @return Pointer to the data that might be NULL. + */ void *Keylist_Data_Pop(OS_Keylist list) { void *data = NULL; /* return value */ int index; /* position in the array */ - if (list && list->count) { - index = list->count - 1; - data = Keylist_Data_Delete_By_Index(list, index); + if (list) { + if (list->count) { + index = list->count - 1; + data = Keylist_Data_Delete_By_Index(list, index); + } } return data; } -/* returns the data from the node specified by key */ +/** Returns the data from the node specified by key. + * + * @param list Pointer to the list + * @param key Key to be deleted + * + * @returns Pointer to the data, that might be NULL. + */ void *Keylist_Data(OS_Keylist list, KEY key) { struct Keylist_Node *node = NULL; int index = 0; /* used to look up the index of node */ - if (list && list->array && list->count) { - if (FindIndex(list, key, &index)) - node = list->array[index]; + if (list) { + if (list->array && list->count) { + if (FindIndex(list, key, &index)) { + node = list->array[index]; + } + } } - return node ? node->data : NULL; } -/* returns the index from the node specified by key */ +/** Returns the index from the node specified by key. + * + * @param list Pointer to the list + * @param key Key whose index shall be retrieved. + * + * @return Index of the key or -1, if not found. + */ int Keylist_Index(OS_Keylist list, KEY key) { int index = -1; /* used to look up the index of node */ - if (list && list->array && list->count) { - if (!FindIndex(list, key, &index)) { - index = -1; + if (list) { + if (list->array && list->count) { + if (!FindIndex(list, key, &index)) { + index = -1; + } } } - return index; } -/* returns the data specified by index */ +/** Returns the data specified by index + * + * @param list Pointer to the list + * @param index Key whose data shall be retrieved. + * + * @return Pointer to the data that might be NULL. + */ void *Keylist_Data_Index(OS_Keylist list, int index) { struct Keylist_Node *node = NULL; - if (list && list->array && list->count && (index >= 0) && - (index < list->count)) - node = list->array[index]; - + if (list) { + if (list->array && list->count && (index >= 0) && + (index < list->count)) { + node = list->array[index]; + } + } return node ? node->data : NULL; } -/* return the key at the given index */ +/** Return the key at the given index. + * + * @param list Pointer to the list + * @param index Index that shall be returned + * + * @return Key for the index or 0. + */ KEY Keylist_Key(OS_Keylist list, int index) { KEY key = 0; /* return value */ struct Keylist_Node *node; - if (list && list->array && list->count && (index >= 0) && - (index < list->count)) { - node = list->array[index]; - if (node) - key = node->key; + if (list) { + if (list->array && list->count && (index >= 0) && + (index < list->count)) { + node = list->array[index]; + if (node) { + key = node->key; + } + } } - return key; } -/* returns the next empty key from the list */ +/** Returns the next empty key from the list. + * + * @param list Pointer to the list + * @param key Key whose index shall be retrieved. + * + * @return Next empty key or 'key=key' if there is none. + */ KEY Keylist_Next_Empty_Key(OS_Keylist list, KEY key) { int index; if (list) { while (FindIndex(list, key, &index)) { - if (KEY_LAST(key)) + if (KEY_LAST(key)) { break; + } key++; } } @@ -348,29 +439,47 @@ KEY Keylist_Next_Empty_Key(OS_Keylist list, KEY key) return key; } -/* return the number of nodes in this list */ +/** Return the number of nodes in this list. + * + * @param list Pointer to the list + * + * @return Count of key in the list. + */ int Keylist_Count(OS_Keylist list) { - return list->count; + int cnt = 0; + + if (list) { + cnt = list->count; + } + + return(cnt); } /******************************************************************** */ /* Public List functions */ /******************************************************************** */ -/* returns head of the list or NULL on failure. */ +/** Returns head of the list or NULL on failure. + * + * @return Pointer to the key list or NUL if creation failed. + */ OS_Keylist Keylist_Create(void) { struct Keylist *list; list = KeylistCreate(); - if (list) + if (list) { CheckArraySize(list); + } return list; } -/* delete specified list */ +/** Delete specified list. + * + * @param list Pointer to the list + */ void Keylist_Delete(OS_Keylist list) { /* list number to be deleted */ if (list) { @@ -378,8 +487,9 @@ void Keylist_Delete(OS_Keylist list) while (list->count) { (void)Keylist_Data_Delete_By_Index(list, 0); } - if (list->array) + if (list->array) { free(list->array); + } free(list); } diff --git a/src/bacnet/cov.c b/src/bacnet/cov.c index 9423ca2f..dcfc5499 100644 --- a/src/bacnet/cov.c +++ b/src/bacnet/cov.c @@ -48,6 +48,15 @@ COV Subscribe Property COV Notification Unconfirmed COV Notification */ + +/** + * Encode APDU for notification. + * + * @param apdu Pointer to the buffer. + * @param data Pointer to the data to encode. + * + * @return bytes encoded or zero on error. + */ static int notify_encode_apdu( uint8_t *apdu, unsigned max_apdu_len, BACNET_COV_DATA *data) { @@ -122,6 +131,16 @@ static int notify_encode_apdu( return apdu_len; } +/** + * Encode APDU for confirmed notification. + * + * @param apdu Pointer to the buffer. + * @param max_apdu_len Buffer size. + * @param invoke_id ID to invoke for notification + * @param data Pointer to the data to encode. + * + * @return bytes encoded or zero on error. + */ int ccov_notify_encode_apdu(uint8_t *apdu, unsigned max_apdu_len, uint8_t invoke_id, @@ -149,6 +168,15 @@ int ccov_notify_encode_apdu(uint8_t *apdu, return apdu_len; } +/** + * Encode APDU for unconfirmed notification. + * + * @param apdu Pointer to the buffer. + * @param max_apdu_len Buffer size. + * @param data Pointer to the data to encode. + * + * @return bytes encoded or zero on error. + */ int ucov_notify_encode_apdu( uint8_t *apdu, unsigned max_apdu_len, BACNET_COV_DATA *data) { @@ -172,8 +200,16 @@ int ucov_notify_encode_apdu( return apdu_len; } -/* decode the service request only */ -/* COV and Unconfirmed COV are the same */ +/** + * Decode the COV-service request only. + * Note: COV and Unconfirmed COV are the same. + * + * @param apdu Pointer to the buffer. + * @param apdu_len Count of valid bytes in the buffer. + * @param data Pointer to the data to store the decoded values. + * + * @return Bytes decoded or Zero/BACNET_STATUS_ERROR on error. + */ int cov_notify_decode_service_request( uint8_t *apdu, unsigned apdu_len, BACNET_COV_DATA *data) { @@ -187,7 +223,7 @@ int cov_notify_decode_service_request( BACNET_PROPERTY_VALUE *value = NULL; /* value in list */ BACNET_APPLICATION_DATA_VALUE *app_data = NULL; - if (apdu_len && data) { + if ((apdu_len > 2) && data) { /* tag 0 - subscriberProcessIdentifier */ if (decode_is_context_tag(&apdu[len], 0)) { len += decode_tag_number_and_value( @@ -198,6 +234,9 @@ int cov_notify_decode_service_request( return BACNET_STATUS_ERROR; } /* tag 1 - initiatingDeviceIdentifier */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_ERROR; + } if (decode_is_context_tag(&apdu[len], 1)) { len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value); @@ -210,6 +249,9 @@ int cov_notify_decode_service_request( return BACNET_STATUS_ERROR; } /* tag 2 - monitoredObjectIdentifier */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_ERROR; + } if (decode_is_context_tag(&apdu[len], 2)) { len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value); @@ -220,6 +262,9 @@ int cov_notify_decode_service_request( return BACNET_STATUS_ERROR; } /* tag 3 - timeRemaining */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_ERROR; + } if (decode_is_context_tag(&apdu[len], 3)) { len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value); @@ -242,6 +287,9 @@ int cov_notify_decode_service_request( } while (value != NULL) { /* tag 0 - propertyIdentifier */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_ERROR; + } if (decode_is_context_tag(&apdu[len], 0)) { len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value); @@ -251,6 +299,9 @@ int cov_notify_decode_service_request( return BACNET_STATUS_ERROR; } /* tag 1 - propertyArrayIndex OPTIONAL */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_ERROR; + } if (decode_is_context_tag(&apdu[len], 1)) { len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value); @@ -260,6 +311,9 @@ int cov_notify_decode_service_request( value->propertyArrayIndex = BACNET_ARRAY_ALL; } /* tag 2: opening context tag - value */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_ERROR; + } if (!decode_is_opening_tag_number(&apdu[len], 2)) { return BACNET_STATUS_ERROR; } @@ -283,6 +337,9 @@ int cov_notify_decode_service_request( /* a tag number of 2 is not extended so only one octet */ len++; /* tag 3 - priority OPTIONAL */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_ERROR; + } if (decode_is_context_tag(&apdu[len], 3)) { len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value); @@ -330,6 +387,16 @@ SubscribeCOV-Request ::= SEQUENCE { } */ +/** + * Encode the COV-service request. + * Note: COV and Unconfirmed COV are the same. + * + * @param apdu Pointer to the buffer. + * @param invoke_id Invoke ID + * @param data Pointer to the data to store the decoded values. + * + * @return Bytes encoded or zero on error. + */ int cov_subscribe_encode_apdu(uint8_t *apdu, unsigned max_apdu_len, uint8_t invoke_id, @@ -372,7 +439,15 @@ int cov_subscribe_encode_apdu(uint8_t *apdu, return apdu_len; } -/* decode the service request only */ +/** + * Decode the subscribe-service request only. + * + * @param apdu Pointer to the buffer. + * @param apdu_len Count of valid bytes in the buffer. + * @param data Pointer to the data to store the decoded values. + * + * @return Bytes decoded or Zero/BACNET_STATUS_ERROR on error. + */ int cov_subscribe_decode_service_request( uint8_t *apdu, unsigned apdu_len, BACNET_SUBSCRIBE_COV_DATA *data) { @@ -382,7 +457,7 @@ int cov_subscribe_decode_service_request( BACNET_UNSIGNED_INTEGER unsigned_value = 0; BACNET_OBJECT_TYPE decoded_type = OBJECT_NONE; - if (apdu_len && data) { + if ((apdu_len > 2) && data) { /* tag 0 - subscriberProcessIdentifier */ if (decode_is_context_tag(&apdu[len], 0)) { len += decode_tag_number_and_value( @@ -394,6 +469,9 @@ int cov_subscribe_decode_service_request( return BACNET_STATUS_REJECT; } /* tag 1 - monitoredObjectIdentifier */ + if ((unsigned) len >= apdu_len) { + return BACNET_STATUS_REJECT; + } if (decode_is_context_tag(&apdu[len], 1)) { len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value); @@ -418,11 +496,15 @@ int cov_subscribe_decode_service_request( data->cancellationRequest = true; } /* tag 3 - lifetime - optional */ - if (decode_is_context_tag(&apdu[len], 3)) { - len += decode_tag_number_and_value( - &apdu[len], &tag_number, &len_value); - len += decode_unsigned(&apdu[len], len_value, &unsigned_value); - data->lifetime = unsigned_value; + if ((unsigned) len < apdu_len) { + if (decode_is_context_tag(&apdu[len], 3)) { + len += decode_tag_number_and_value( + &apdu[len], &tag_number, &len_value); + len += decode_unsigned(&apdu[len], len_value, &unsigned_value); + data->lifetime = unsigned_value; + } else { + data->lifetime = 0; + } } else { data->lifetime = 0; } @@ -453,6 +535,16 @@ BACnetPropertyReference ::= SEQUENCE { */ +/** + * Encode the properties for subscription into the APDU. + * + * @param apdu Pointer to the buffer. + * @param max_apdu_len Buffer size. + * @param invoke_id Invoke Id. + * @param data Pointer to the data to encode. + * + * @return Bytes decoded or Zero/BACNET_STATUS_ERROR on error. + */ int cov_subscribe_property_encode_apdu(uint8_t *apdu, unsigned max_apdu_len, uint8_t invoke_id, @@ -509,7 +601,15 @@ int cov_subscribe_property_encode_apdu(uint8_t *apdu, return apdu_len; } -/* decode the service request only */ +/** + * Decode the COV-service property request only. + * + * @param apdu Pointer to the buffer. + * @param apdu_len Count of valid bytes in the buffer. + * @param data Pointer to the data to store the decoded values. + * + * @return Bytes decoded or Zero/BACNET_STATUS_ERROR on error. + */ int cov_subscribe_property_decode_service_request( uint8_t *apdu, unsigned apdu_len, BACNET_SUBSCRIBE_COV_DATA *data) { @@ -520,7 +620,7 @@ int cov_subscribe_property_decode_service_request( BACNET_OBJECT_TYPE decoded_type = OBJECT_NONE; /* for decoding */ uint32_t property = 0; /* for decoding */ - if (apdu_len && data) { + if ((apdu_len > 2) && data) { /* tag 0 - subscriberProcessIdentifier */ if (decode_is_context_tag(&apdu[len], 0)) { len += decode_tag_number_and_value( @@ -532,6 +632,9 @@ int cov_subscribe_property_decode_service_request( return BACNET_STATUS_REJECT; } /* tag 1 - monitoredObjectIdentifier */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_REJECT; + } if (decode_is_context_tag(&apdu[len], 1)) { len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value); @@ -543,6 +646,9 @@ int cov_subscribe_property_decode_service_request( return BACNET_STATUS_REJECT; } /* tag 2 - issueConfirmedNotifications - optional */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_REJECT; + } if (decode_is_context_tag(&apdu[len], 2)) { data->cancellationRequest = false; len += decode_tag_number_and_value( @@ -554,6 +660,9 @@ int cov_subscribe_property_decode_service_request( data->cancellationRequest = true; } /* tag 3 - lifetime - optional */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_REJECT; + } if (decode_is_context_tag(&apdu[len], 3)) { len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value); @@ -563,6 +672,9 @@ int cov_subscribe_property_decode_service_request( data->lifetime = 0; } /* tag 4 - monitoredPropertyIdentifier */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_REJECT; + } if (!decode_is_opening_tag_number(&apdu[len], 4)) { data->error_code = ERROR_CODE_REJECT_INVALID_TAG; return BACNET_STATUS_REJECT; @@ -570,6 +682,9 @@ int cov_subscribe_property_decode_service_request( /* a tag number of 4 is not extended so only one octet */ len++; /* the propertyIdentifier is tag 0 */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_REJECT; + } if (decode_is_context_tag(&apdu[len], 0)) { len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value); @@ -581,6 +696,9 @@ int cov_subscribe_property_decode_service_request( return BACNET_STATUS_REJECT; } /* the optional array index is tag 1 */ + if (len >= (int)apdu_len) { + return BACNET_STATUS_REJECT; + } if (decode_is_context_tag(&apdu[len], 1)) { len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value); @@ -597,11 +715,15 @@ int cov_subscribe_property_decode_service_request( /* a tag number of 4 is not extended so only one octet */ len++; /* tag 5 - covIncrement - optional */ - if (decode_is_context_tag(&apdu[len], 5)) { - data->covIncrementPresent = true; - len += decode_tag_number_and_value( - &apdu[len], &tag_number, &len_value); - len += decode_real(&apdu[len], &data->covIncrement); + if (len < (int)apdu_len) { + if (decode_is_context_tag(&apdu[len], 5)) { + data->covIncrementPresent = true; + len += decode_tag_number_and_value( + &apdu[len], &tag_number, &len_value); + len += decode_real(&apdu[len], &data->covIncrement); + } else { + data->covIncrementPresent = false; + } } else { data->covIncrementPresent = false; } diff --git a/src/bacnet/datetime.c b/src/bacnet/datetime.c index d14e238d..c493aa44 100644 --- a/src/bacnet/datetime.c +++ b/src/bacnet/datetime.c @@ -288,9 +288,16 @@ bool datetime_is_valid(BACNET_DATE *bdate, BACNET_TIME *btime) return datetime_date_is_valid(bdate) && datetime_time_is_valid(btime); } -/* if the date1 is the same as date2, return is 0 - if date1 is after date2, returns positive - if date1 is before date2, returns negative */ +/** + * If the date1 is the same as date2, return is 0. + * If date1 is after date2, returns positive. + * if date1 is before date2, returns negative. + * + * @param date1 - Pointer to a BACNET_DATE structure + * @param date2 - Pointer to a BACNET_DATE structure + * + * @return -/0/+ + */ int datetime_compare_date(BACNET_DATE *date1, BACNET_DATE *date2) { int diff = 0; @@ -308,9 +315,16 @@ int datetime_compare_date(BACNET_DATE *date1, BACNET_DATE *date2) return diff; } -/* if the time1 is the same as time2, return is 0 - if time1 is after time2, returns positive - if time1 is before time2, returns negative */ +/** + * If the time1 is the same as time2, return is 0. + * If time1 is after time2, returns positive. + * if time1 is before time2, returns negative. + * + * @param time1 - Pointer to a BACNET_TIME structure + * @param time2 - Pointer to a BACNET_TIME structure + * + * @return -/0/+ + */ int datetime_compare_time(BACNET_TIME *time1, BACNET_TIME *time2) { int diff = 0; @@ -331,9 +345,16 @@ int datetime_compare_time(BACNET_TIME *time1, BACNET_TIME *time2) return diff; } -/* if the datetime1 is the same as datetime2, return is 0 - if datetime1 is before datetime2, returns negative - if datetime1 is after datetime2, returns positive */ +/** + * If the datetime1 is the same datetime2, return is 0. + * If datetime1 is after datetime2, returns positive. + * if datetime1 is before datetime2, returns negative. + * + * @param datetime1 - Pointer to a BACNET_DATE_TIME structure + * @param datetime2 - Pointer to a BACNET_DATE_TIME structure + * + * @return -/0/+ + */ int datetime_compare(BACNET_DATE_TIME *datetime1, BACNET_DATE_TIME *datetime2) { int diff = 0; diff --git a/src/bacnet/dcc.c b/src/bacnet/dcc.c index 57228997..c8623688 100644 --- a/src/bacnet/dcc.c +++ b/src/bacnet/dcc.c @@ -49,46 +49,73 @@ static BACNET_COMMUNICATION_ENABLE_DISABLE DCC_Enable_Disable = COMMUNICATION_ENABLE; /* password is optionally supported */ +/** + * Returns, the network communications enable/disable status. + * + * @return BACnet communication status + */ BACNET_COMMUNICATION_ENABLE_DISABLE dcc_enable_status(void) { return DCC_Enable_Disable; } +/** + * Returns, if network communications is enabled. + * + * @return true, if communication has been enabled. + */ bool dcc_communication_enabled(void) { return (DCC_Enable_Disable == COMMUNICATION_ENABLE); } -/* When network communications are completely disabled, - only DeviceCommunicationControl and ReinitializeDevice APDUs - shall be processed and no messages shall be initiated.*/ +/** + * When network communications are completely disabled, + * only DeviceCommunicationControl and ReinitializeDevice APDUs + * shall be processed and no messages shall be initiated. + * + * @return true, if communication has been disabled, false otherwise. + */ bool dcc_communication_disabled(void) { return (DCC_Enable_Disable == COMMUNICATION_DISABLE); } -/* When the initiation of communications is disabled, - all APDUs shall be processed and responses returned as - required and no messages shall be initiated with the - exception of I-Am requests, which shall be initiated only in - response to Who-Is messages. In this state, a device that - supports I-Am request initiation shall send one I-Am request - for any Who-Is request that is received if and only if - the Who-Is request does not contain an address range or - the device is included in the address range. */ +/** + * When the initiation of communications is disabled, + * all APDUs shall be processed and responses returned as + * required and no messages shall be initiated with the + * exception of I-Am requests, which shall be initiated only in + * response to Who-Is messages. In this state, a device that + * supports I-Am request initiation shall send one I-Am request + * for any Who-Is request that is received if and only if + * the Who-Is request does not contain an address range or + * the device is included in the address range. + * + * @return true, if disabling initiation is set, false otherwise. + */ bool dcc_communication_initiation_disabled(void) { return (DCC_Enable_Disable == COMMUNICATION_DISABLE_INITIATION); } -/* note: 0 indicates either expired, or infinite duration */ +/** + * Returns the time duration in seconds. + * Note: 0 indicates either expired, or infinite duration. + * + * @return time in seconds + */ uint32_t dcc_duration_seconds(void) { return DCC_Time_Duration_Seconds; } -/* called every second or so. If more than one second, - then seconds should be the number of seconds to tick away */ +/** + * Called every second or so. If more than one second, + * then seconds should be the number of seconds to tick away. + * + * @param seconds Time passed in seconds, since last call. + */ void dcc_timer_seconds(uint32_t seconds) { if (DCC_Time_Duration_Seconds) { @@ -104,6 +131,14 @@ void dcc_timer_seconds(uint32_t seconds) } } +/** + * Set DCC status using duration. + * + * @param status Enable/disable communication + * @param minutes Duration in minutes + * + * @return true/false + */ bool dcc_set_status_duration( BACNET_COMMUNICATION_ENABLE_DISABLE status, uint16_t minutes) { @@ -124,7 +159,17 @@ bool dcc_set_status_duration( } #if BACNET_SVC_DCC_A -/* encode service */ +/** + * Encode service + * + * @param apdu Pointer to the APDU buffer used for encoding. + * @param invoke_id Invoke-Id + * @param timeDuration Optional time duration in minutes. + * @param enable_disable Enable/disable communication + * @param password Pointer to an optional password. + * + * @return Bytes encoded or zero on an error. + */ int dcc_encode_apdu(uint8_t *apdu, uint8_t invoke_id, uint16_t timeDuration, /* 0=optional */ @@ -150,9 +195,10 @@ int dcc_encode_apdu(uint8_t *apdu, apdu_len += len; /* optional password */ if (password) { - /* FIXME: must be at least 1 character, limited to 20 characters */ - len = encode_context_character_string(&apdu[apdu_len], 2, password); - apdu_len += len; + if ((password->length >= 1) && (password->length <= 20)) { + len = encode_context_character_string(&apdu[apdu_len], 2, password); + apdu_len += len; + } } } @@ -160,7 +206,17 @@ int dcc_encode_apdu(uint8_t *apdu, } #endif -/* decode the service request only */ +/** + * Decode the service request only + * + * @param apdu Pointer to the received request. + * @param apdu_len_max Valid count of bytes in the buffer. + * @param timeDuration Pointer to the duration given in minutes [optional] + * @param enable_disable Pointer to the variable takingthe communication enable/disable. + * @param password Pointer to the password [optional] + * + * @return Bytes decoded. + */ int dcc_decode_service_request(uint8_t *apdu, unsigned apdu_len_max, uint16_t *timeDuration, @@ -174,7 +230,7 @@ int dcc_decode_service_request(uint8_t *apdu, BACNET_UNSIGNED_INTEGER decoded_unsigned = 0; uint32_t decoded_enum = 0; - if (apdu_len_max) { + if (apdu && apdu_len_max) { /* Tag 0: timeDuration, in minutes --optional-- */ len = bacnet_unsigned_context_decode( &apdu[apdu_len], apdu_len_max - apdu_len, 0, &decoded_unsigned); diff --git a/src/bacnet/getevent.c b/src/bacnet/getevent.c index 41cfee27..860c6562 100644 --- a/src/bacnet/getevent.c +++ b/src/bacnet/getevent.c @@ -39,7 +39,14 @@ /** @file getevent.c Encode/Decode GetEvent services */ -/* encode service */ +/** Encode service + * + * @param apdu APDU buffer to encode to. + * @param invoke_id Invoke ID + * @param lastReceivedObjectIdentifier Object identifier + * + * @return Bytes encoded. + */ int getevent_encode_apdu(uint8_t *apdu, uint8_t invoke_id, BACNET_OBJECT_ID *lastReceivedObjectIdentifier) @@ -65,7 +72,14 @@ int getevent_encode_apdu(uint8_t *apdu, return apdu_len; } -/* decode the service request only */ +/** Decode the service request only + * + * @param apdu APDU buffer to encode to. + * @param apdu_len Valid bytes in the buffer. + * @param lastReceivedObjectIdentifier Object identifier + * + * @return Bytes encoded. + */ int getevent_decode_service_request(uint8_t *apdu, unsigned apdu_len, BACNET_OBJECT_ID *lastReceivedObjectIdentifier) @@ -78,8 +92,10 @@ int getevent_decode_service_request(uint8_t *apdu, if (!decode_is_context_tag(&apdu[len++], 0)) { return -1; } - len += decode_object_id(&apdu[len], &lastReceivedObjectIdentifier->type, - &lastReceivedObjectIdentifier->instance); + if (len < apdu_len) { + len += decode_object_id(&apdu[len], &lastReceivedObjectIdentifier->type, + &lastReceivedObjectIdentifier->instance); + } } return (int)len; diff --git a/src/bacnet/ihave.c b/src/bacnet/ihave.c index 1a2b5c9f..37dc19d4 100644 --- a/src/bacnet/ihave.c +++ b/src/bacnet/ihave.c @@ -39,6 +39,14 @@ /** @file ihave.c Encode/Decode I-Have service */ +/** + * Encode the I Have request + * + * @param apdu Pointer to the APDU buffer + * @param data Pointer to the I Have data structure. + * + * @return Bytes encoded. + */ int ihave_encode_apdu(uint8_t *apdu, BACNET_I_HAVE_DATA *data) { int len = 0; /* length of each encoding */ @@ -67,7 +75,15 @@ int ihave_encode_apdu(uint8_t *apdu, BACNET_I_HAVE_DATA *data) #if BACNET_SVC_I_HAVE_A -/* decode the service request only */ +/** + * Decode the I Have request only + * + * @param apdu Pointer to the APDU buffer + * @param apdu_len Valid bytes in the buffer + * @param data Pointer to the I Have data structure. + * + * @return Bytes decoded. + */ int ihave_decode_service_request( uint8_t *apdu, unsigned apdu_len, BACNET_I_HAVE_DATA *data) { @@ -76,7 +92,7 @@ int ihave_decode_service_request( uint32_t len_value = 0; BACNET_OBJECT_TYPE decoded_type = OBJECT_NONE; /* for decoding */ - if (apdu_len && data) { + if ((apdu_len >= 2) && data) { /* deviceIdentifier */ len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); if (tag_number == BACNET_APPLICATION_TAG_OBJECT_ID) { @@ -87,6 +103,9 @@ int ihave_decode_service_request( return -1; } /* objectIdentifier */ + if ((unsigned)len >= apdu_len) { + return -1; + } len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); if (tag_number == BACNET_APPLICATION_TAG_OBJECT_ID) { len += decode_object_id( @@ -96,6 +115,9 @@ int ihave_decode_service_request( return -1; } /* objectName */ + if ((unsigned)len >= apdu_len) { + return -1; + } len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); if (tag_number == BACNET_APPLICATION_TAG_CHARACTER_STRING) { len += decode_character_string( @@ -110,12 +132,21 @@ int ihave_decode_service_request( return len; } +/** + * Decode the I Have + * + * @param apdu Pointer to the APDU buffer + * @param apdu_len Valid bytes in the buffer + * @param data Pointer to the I Have data structure. + * + * @return Bytes decoded. + */ int ihave_decode_apdu( uint8_t *apdu, unsigned apdu_len, BACNET_I_HAVE_DATA *data) { int len = 0; - if (!apdu) { + if ((!apdu) || (apdu_len < 2)) { return -1; } /* optional checking - most likely was already done prior to this call */ diff --git a/src/bacnet/rd.c b/src/bacnet/rd.c index 20bc3808..475aab4e 100644 --- a/src/bacnet/rd.c +++ b/src/bacnet/rd.c @@ -32,6 +32,7 @@ ------------------------------------------- ####COPYRIGHTEND####*/ #include + #include "bacnet/bacenum.h" #include "bacnet/bacdcode.h" #include "bacnet/bacdef.h" @@ -39,7 +40,16 @@ /** @file rd.c Encode/Decode Reinitialize Device APDUs */ #if BACNET_SVC_RD_A -/* encode service */ + +/** Encode Reinitialize Device service + * + * @param apdu Pointer to the APDU buffer. + * @param invoke_id Invoke-Id + * @param state Reinitialization state + * @param password Pointer to the pass phrase. + * + * @return Bytes encoded. + */ int rd_encode_apdu(uint8_t *apdu, uint8_t invoke_id, BACNET_REINITIALIZED_STATE state, @@ -58,9 +68,11 @@ int rd_encode_apdu(uint8_t *apdu, apdu_len += len; /* optional password */ if (password) { - /* FIXME: must be at least 1 character, limited to 20 characters */ - len = encode_context_character_string(&apdu[apdu_len], 1, password); - apdu_len += len; + /* Must be at least 1 character, limited to 20 characters */ + if ((password->length >= 1) && (password->length <= 20)) { + len = encode_context_character_string(&apdu[apdu_len], 1, password); + apdu_len += len; + } } } @@ -68,7 +80,15 @@ int rd_encode_apdu(uint8_t *apdu, } #endif -/* decode the service request only */ +/** Decode Reinitialize Device service + * + * @param apdu Pointer to the APDU buffer. + * @param apdu_len Valid bytes in the buffer + * @param state Pointer to the Reinitialization state + * @param password Pointer to the pass phrase. + * + * @return Bytes encoded. + */ int rd_decode_service_request(uint8_t *apdu, unsigned apdu_len, BACNET_REINITIALIZED_STATE *state, @@ -80,7 +100,7 @@ int rd_decode_service_request(uint8_t *apdu, uint32_t value = 0; /* check for value pointers */ - if (apdu_len) { + if ((apdu) && (apdu_len >= 2)) { /* Tag 0: reinitializedStateOfDevice */ if (!decode_is_context_tag(&apdu[len], 0)) { return -1; @@ -98,8 +118,13 @@ int rd_decode_service_request(uint8_t *apdu, } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); - len += - decode_character_string(&apdu[len], len_value_type, password); + if (len < apdu_len) { + if (password) { + len += decode_character_string(&apdu[len], + len_value_type, + password); + } + } } } diff --git a/src/bacnet/readrange.c b/src/bacnet/readrange.c index 808ff259..d2e088d9 100644 --- a/src/bacnet/readrange.c +++ b/src/bacnet/readrange.c @@ -61,10 +61,15 @@ * } */ -/***************************************************************************** - * Build a ReadRange request packet. * - *****************************************************************************/ - +/** + * Build a ReadRange request packet. + * + * @param apdu Pointer to the APDU buffer. + * @param invoke_id Invoke ID + * @param rrdata Pointer to the data used for encoding. + * + * @return Bytes encoded. + */ int rr_encode_apdu( uint8_t *apdu, uint8_t invoke_id, BACNET_READ_RANGE_DATA *rrdata) { @@ -134,10 +139,15 @@ int rr_encode_apdu( return apdu_len; } -/***************************************************************************** - * Decode the received ReadRange request * - *****************************************************************************/ - +/** + * Decode the received ReadRange request + * + * @param apdu Pointer to the APDU buffer. + * @param apdu_len Bytes valid in the APDU buffer. + * @param rrdata Pointer to the data used for encoding. + * + * @return Bytes encoded. + */ int rr_decode_service_request( uint8_t *apdu, unsigned apdu_len, BACNET_READ_RANGE_DATA *rrdata) { @@ -150,7 +160,7 @@ int rr_decode_service_request( BACNET_UNSIGNED_INTEGER unsigned_value; /* check for value pointers */ - if (apdu_len && rrdata) { + if ((apdu_len >= 5) && apdu && rrdata) { /* Tag 0: Object ID */ if (!decode_is_context_tag(&apdu[len++], 0)) { return -1; @@ -158,6 +168,9 @@ int rr_decode_service_request( len += decode_object_id(&apdu[len], &type, &rrdata->object_instance); rrdata->object_type = type; /* Tag 1: Property ID */ + if (len >= apdu_len) { + return(-1); + } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); if (tag_number != 1) { @@ -199,30 +212,60 @@ int rr_decode_service_request( switch (tag_number) { case 3: /* ReadRange by position */ rrdata->RequestType = RR_BY_POSITION; + if (len >= apdu_len) { + break; + } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); + if (len >= apdu_len) { + break; + } len += decode_unsigned( &apdu[len], len_value_type, &unsigned_value); rrdata->Range.RefIndex = (uint32_t)unsigned_value; + if (len >= apdu_len) { + break; + } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); + if (len >= apdu_len) { + break; + } len += decode_signed( &apdu[len], len_value_type, &rrdata->Count); + if (len >= apdu_len) { + break; + } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); break; case 6: /* ReadRange by sequence number */ rrdata->RequestType = RR_BY_SEQUENCE; + if (len >= apdu_len) { + break; + } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); + if (len >= apdu_len) { + break; + } len += decode_unsigned( &apdu[len], len_value_type, &unsigned_value); rrdata->Range.RefSeqNum = (uint32_t)unsigned_value; + if (len >= apdu_len) { + break; + } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); + if (len >= apdu_len) { + break; + } len += decode_signed( &apdu[len], len_value_type, &rrdata->Count); + if (len >= apdu_len) { + break; + } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); /* Allow for this in the response */ @@ -231,17 +274,38 @@ int rr_decode_service_request( case 7: /* ReadRange by time stamp */ rrdata->RequestType = RR_BY_TIME; + if (len >= apdu_len) { + break; + } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); + if (len >= apdu_len) { + break; + } len += decode_date(&apdu[len], &rrdata->Range.RefTime.date); + if (len >= apdu_len) { + break; + } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); + if (len >= apdu_len) { + break; + } len += decode_bacnet_time( &apdu[len], &rrdata->Range.RefTime.time); + if (len >= apdu_len) { + break; + } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); + if (len >= apdu_len) { + break; + } len += decode_signed( &apdu[len], len_value_type, &rrdata->Count); + if (len >= apdu_len) { + break; + } len += decode_tag_number_and_value( &apdu[len], &tag_number, &len_value_type); /* Allow for this in the response */ @@ -253,6 +317,8 @@ int rr_decode_service_request( break; } } + } else { + return(-1); } return (int)len; @@ -272,13 +338,19 @@ int rr_decode_service_request( * } */ -/***************************************************************************** - * Build a ReadRange response packet * - *****************************************************************************/ - +/** + * Build a ReadRange response packet + * + * @param apdu Pointer to the buffer. + * @param invoke_id ID invoked. + * @param rrdata Pointer to the read range data structure used for encoding. + * + * @return The count of encoded bytes. + */ int rr_ack_encode_apdu( uint8_t *apdu, uint8_t invoke_id, BACNET_READ_RANGE_DATA *rrdata) { + int imax = 0; int len = 0; /* length of each encoding */ int apdu_len = 0; /* total length of the apdu, return value */ @@ -309,7 +381,11 @@ int rr_ack_encode_apdu( */ apdu_len += encode_opening_tag(&apdu[apdu_len], 5); if (rrdata->ItemCount != 0) { - for (len = 0; len < rrdata->application_data_len; len++) { + imax = rrdata->application_data_len; + if (imax > (MAX_APDU - apdu_len - 2 /*closing*/)) { + imax = (MAX_APDU - apdu_len - 2); + } + for (len = 0; len < imax; len++) { apdu[apdu_len++] = rrdata->application_data[len]; } } @@ -319,18 +395,25 @@ int rr_ack_encode_apdu( (rrdata->RequestType != RR_BY_POSITION) && (rrdata->RequestType != RR_READ_ALL)) { /* Context 6 Sequence number of first item */ - apdu_len += encode_context_unsigned( - &apdu[apdu_len], 6, rrdata->FirstSequence); + if (apdu_len < (MAX_APDU - 4)) { + apdu_len += encode_context_unsigned( + &apdu[apdu_len], 6, rrdata->FirstSequence); + } } } return apdu_len; } -/***************************************************************************** - * Decode the received ReadRange response * - *****************************************************************************/ - +/** + * Decode the received ReadRange response + * + * @param apdu Pointer to the APDU buffer. + * @param apdu_len Bytes valid in the APDU buffer. + * @param rrdata Pointer to the data filled while decoding. + * + * @return Bytes decoded. + */ int rr_ack_decode_service_request(uint8_t *apdu, int apdu_len, /* total length of the apdu */ BACNET_READ_RANGE_DATA *rrdata) @@ -344,90 +427,111 @@ int rr_ack_decode_service_request(uint8_t *apdu, uint32_t property = 0; /* for decoding */ BACNET_UNSIGNED_INTEGER unsigned_value; - /* FIXME: check apdu_len against the len during decode */ - /* Tag 0: Object ID */ - if (!decode_is_context_tag(&apdu[0], 0)) { - return -1; - } - len = 1; - len += decode_object_id(&apdu[len], &object_type, &rrdata->object_instance); - rrdata->object_type = object_type; - - /* Tag 1: Property ID */ - len += - decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); - if (tag_number != 1) { - return -1; - } - len += decode_enumerated(&apdu[len], len_value_type, &property); - rrdata->object_property = (BACNET_PROPERTY_ID)property; - - /* Tag 2: Optional Array Index */ - tag_len = - decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); - if (tag_number == 2) { - len += tag_len; - len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value); - rrdata->array_index = (BACNET_ARRAY_INDEX)unsigned_value; - } else { - rrdata->array_index = BACNET_ARRAY_ALL; - } - - /* Tag 3: Result Flags */ - len += - decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); - if (tag_number != 3) { - return -1; - } - - len += decode_bitstring(&apdu[len], len_value_type, &rrdata->ResultFlags); - - /* Tag 4: Item count */ - len += - decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); - if (tag_number != 4) { - return -1; - } - - len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value); - rrdata->ItemCount = (uint32_t)unsigned_value; - - if (decode_is_opening_tag_number(&apdu[len], 5)) { - len++; /* a tag number of 5 is not extended so only one octet */ - /* Setup the start position and length of the data returned from the - * request don't decode the application tag number or its data here */ - rrdata->application_data = &apdu[len]; - start_len = len; - while (len < apdu_len) { - if (IS_CONTEXT_SPECIFIC(apdu[len]) && - (decode_is_closing_tag_number(&apdu[len], 5))) { - rrdata->application_data_len = len - start_len; - len++; /* Step over single byte closing tag */ - break; - } else { - /* Don't care about tag number, just skipping over anyway */ - len += decode_tag_number_and_value( - &apdu[len], NULL, &len_value_type); - len += len_value_type; /* Skip over data value as well */ - if (len >= apdu_len) { /* APDU is exhausted so we have failed to - find closing tag */ - return (-1); - } - } - } - } else { - return -1; - } - if (len < apdu_len) { /* Still something left to look at? */ - /* Tag 6: Item count */ - len += decode_tag_number_and_value( - &apdu[len], &tag_number, &len_value_type); - if (tag_number != 6) { + /* Check apdu_len against the len during decode. */ + if (apdu && (apdu_len >= 5 /* minimum */)) { + /* Tag 0: Object ID */ + if (!decode_is_context_tag(&apdu[0], 0)) { return -1; } + len = 1; + len += decode_object_id(&apdu[len], &object_type, &rrdata->object_instance); + rrdata->object_type = object_type; + /* Tag 1: Property ID */ + if (len >= apdu_len) { + return -1; + } + len += + decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); + if (tag_number != 1) { + return -1; + } + len += decode_enumerated(&apdu[len], len_value_type, &property); + rrdata->object_property = (BACNET_PROPERTY_ID)property; + + /* Tag 2: Optional Array Index */ + if (len >= apdu_len) { + return -1; + } + tag_len = + decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); + if (tag_number == 2) { + len += tag_len; + len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value); + rrdata->array_index = (BACNET_ARRAY_INDEX)unsigned_value; + } else { + rrdata->array_index = BACNET_ARRAY_ALL; + } + + /* Tag 3: Result Flags */ + if (len >= apdu_len) { + return -1; + } + len += + decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); + if (tag_number != 3) { + return -1; + } + if (len >= apdu_len) { + return -1; + } + len += decode_bitstring(&apdu[len], len_value_type, &rrdata->ResultFlags); + + /* Tag 4: Item count */ + if (len >= apdu_len) { + return -1; + } + len += + decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); + if (tag_number != 4) { + return -1; + } + if (len >= apdu_len) { + return -1; + } len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value); - rrdata->FirstSequence = (uint32_t)unsigned_value; + rrdata->ItemCount = (uint32_t)unsigned_value; + if (len >= apdu_len) { + return -1; + } + if (decode_is_opening_tag_number(&apdu[len], 5)) { + len++; /* A tag number of 5 is not extended so only one octet + * Setup the start position and length of the data returned from the + * request don't decode the application tag number or its data here. */ + rrdata->application_data = &apdu[len]; + start_len = len; + while (len < apdu_len) { + if (IS_CONTEXT_SPECIFIC(apdu[len]) && + (decode_is_closing_tag_number(&apdu[len], 5))) { + rrdata->application_data_len = len - start_len; + len++; /* Step over single byte closing tag */ + break; + } else { + /* Don't care about tag number, just skipping over anyway */ + len += decode_tag_number_and_value( + &apdu[len], NULL, &len_value_type); + len += len_value_type; /* Skip over data value as well */ + if (len >= apdu_len) { /* APDU is exhausted so we have failed to + * find closing tag */ + return (-1); + } + } + } + } else { + return -1; + } + if (len < apdu_len) { /* Still something left to look at? */ + /* Tag 6: Item count */ + len += decode_tag_number_and_value( + &apdu[len], &tag_number, &len_value_type); + if (tag_number != 6) { + return -1; + } + if (len < apdu_len) { + len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value); + rrdata->FirstSequence = (uint32_t)unsigned_value; + } + } } return len; diff --git a/src/bacnet/reject.c b/src/bacnet/reject.c index 02397b76..1cc0fcfa 100644 --- a/src/bacnet/reject.c +++ b/src/bacnet/reject.c @@ -141,7 +141,14 @@ BACNET_ERROR_CODE reject_convert_to_error_code(BACNET_REJECT_REASON reject_code) return (error_code); } -/* encode service */ +/** Encode service + * + * @param apdu Pointer to the APDU buffer. + * @param invoke_id Invoke-Id. + * @param reject_reason Reason for having rejected. + * + * @return Bytes encoded, typically 3. + */ int reject_encode_apdu(uint8_t *apdu, uint8_t invoke_id, uint8_t reject_reason) { int apdu_len = 0; /* total length of the apdu, return value */ @@ -157,7 +164,17 @@ int reject_encode_apdu(uint8_t *apdu, uint8_t invoke_id, uint8_t reject_reason) } #if !BACNET_SVC_SERVER -/* decode the service request only */ + +/** Decode the service request only. + * + * @param apdu Pointer to the APDU buffer. + * @param apdu_len Bytes valid in the buffer + * @param invoke_id Invoke-Id. + * @param reject_reason Pointer to the variable taking + * the reason for having rejected. + * + * @return Bytes encoded, typically 3. + */ int reject_decode_service_request(uint8_t *apdu, unsigned apdu_len, uint8_t *invoke_id, @@ -170,7 +187,11 @@ int reject_decode_service_request(uint8_t *apdu, *invoke_id = apdu[0]; } if (reject_reason) { - *reject_reason = apdu[1]; + if (apdu_len > 1) { + *reject_reason = apdu[1]; + } else { + *reject_reason = 0; + } } } diff --git a/src/bacnet/rp.c b/src/bacnet/rp.c index a652ebd2..917fb698 100644 --- a/src/bacnet/rp.c +++ b/src/bacnet/rp.c @@ -40,7 +40,15 @@ /** @file rp.c Encode/Decode Read Property and RP ACKs */ #if BACNET_SVC_RP_A -/* encode service */ + +/** Encode the service + * + * @param apdu Pointer to the buffer for encoding. + * @param invoke_id Invoke ID + * @param rpdata Pointer to the property data to be encoded. + * + * @return Bytes encoded or zero on error. + */ int rp_encode_apdu( uint8_t *apdu, uint8_t invoke_id, BACNET_READ_PROPERTY_DATA *rpdata) { @@ -60,6 +68,7 @@ int rp_encode_apdu( rpdata->object_type, rpdata->object_instance); apdu_len += len; } + /* The value should be in the range of 0 to 4194303. */ if (rpdata->object_property <= MAX_BACNET_PROPERTY_ID) { /* check bounds so that we could create malformed messages for testing */ @@ -79,7 +88,14 @@ int rp_encode_apdu( } #endif -/* decode the service request only */ +/** Decode the service request only + * + * @param apdu Pointer to the buffer for encoding. + * @param apdu_len Count of valid bytes inthe buffer. + * @param rpdata Pointer to the property data to be encoded. + * + * @return Bytes decoded or zero on error. + */ int rp_decode_service_request( uint8_t *apdu, unsigned apdu_len, BACNET_READ_PROPERTY_DATA *rpdata) { @@ -143,7 +159,14 @@ int rp_decode_service_request( return (int)len; } -/* alternate method to encode the ack without extra buffer */ +/** Alternate method to encode the ack without extra buffer. + * + * @param apdu Pointer to the buffer for encoding. + * @param invoke_id Invoke Id + * @param rpdata Pointer to the property data to be encoded. + * + * @return Bytes decoded or zero on error. + */ int rp_ack_encode_apdu_init( uint8_t *apdu, uint8_t invoke_id, BACNET_READ_PROPERTY_DATA *rpdata) { @@ -176,7 +199,15 @@ int rp_ack_encode_apdu_init( return apdu_len; } -/* note: encode the application tagged data yourself */ +/** Encode the closing tag for the object property. + * Note: Encode the application tagged data yourself. + * + * @param apdu Pointer to the buffer for encoding. + * @param invoke_id Invoke Id + * @param rpdata Pointer to the property data to be encoded. + * + * @return Bytes encoded or zero on error. + */ int rp_ack_encode_apdu_object_property_end(uint8_t *apdu) { int apdu_len = 0; /* total length of the apdu, return value */ @@ -188,17 +219,31 @@ int rp_ack_encode_apdu_object_property_end(uint8_t *apdu) return apdu_len; } +/** Encode the acknowledge. + * + * @param apdu Pointer to the buffer for encoding. + * @param invoke_id Invoke Id + * @param rpdata Pointer to the property data to be encoded. + * + * @return Bytes encoded or zero on error. + */ int rp_ack_encode_apdu( uint8_t *apdu, uint8_t invoke_id, BACNET_READ_PROPERTY_DATA *rpdata) { + int imax = 0; int len = 0; /* length of each encoding */ int apdu_len = 0; /* total length of the apdu, return value */ if (apdu) { /* Do the initial encoding */ apdu_len = rp_ack_encode_apdu_init(apdu, invoke_id, rpdata); - /* propertyValue */ - for (len = 0; len < rpdata->application_data_len; len++) { + /* propertyValue + * double check maximum possible */ + imax = rpdata->application_data_len; + if (imax > (MAX_APDU - apdu_len - 2)) { + imax = (MAX_APDU - apdu_len - 2); + } + for (len = 0; len < imax; len++) { apdu[apdu_len++] = rpdata->application_data[len]; } apdu_len += encode_closing_tag(&apdu[apdu_len], 3); @@ -231,41 +276,62 @@ int rp_ack_decode_service_request(uint8_t *apdu, uint32_t property = 0; /* for decoding */ BACNET_UNSIGNED_INTEGER unsigned_value = 0; /* for decoding */ - /* FIXME: check apdu_len against the len during decode */ - /* Tag 0: Object ID */ - if (!decode_is_context_tag(&apdu[0], 0)) { - return -1; - } - len = 1; - len += decode_object_id(&apdu[len], &object_type, &rpdata->object_instance); - rpdata->object_type = object_type; - /* Tag 1: Property ID */ - len += - decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); - if (tag_number != 1) { - return -1; - } - len += decode_enumerated(&apdu[len], len_value_type, &property); - rpdata->object_property = (BACNET_PROPERTY_ID)property; - /* Tag 2: Optional Array Index */ - tag_len = - decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); - if (tag_number == 2) { - len += tag_len; - len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value); - rpdata->array_index = (BACNET_ARRAY_INDEX)unsigned_value; - } else { - rpdata->array_index = BACNET_ARRAY_ALL; - } - /* Tag 3: opening context tag */ - if (decode_is_opening_tag_number(&apdu[len], 3)) { - /* a tag number of 3 is not extended so only one octet */ - len++; - /* don't decode the application tag number or its data here */ - rpdata->application_data = &apdu[len]; - rpdata->application_data_len = apdu_len - len - 1 /*closing tag */; - /* len includes the data and the closing tag */ - len = apdu_len; + /* Check basics. */ + if (apdu && (apdu_len >= 8 /*minimum*/)) { + /* Tag 0: Object ID */ + if (!decode_is_context_tag(&apdu[0], 0)) { + return -1; + } + len = 1; + len += decode_object_id(&apdu[len], &object_type, &rpdata->object_instance); + rpdata->object_type = object_type; + /* Tag 1: Property ID */ + if (len >= apdu_len) { + return -1; + } + len += + decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); + if (tag_number != 1) { + return -1; + } + if (len >= apdu_len) { + return -1; + } + len += decode_enumerated(&apdu[len], len_value_type, &property); + rpdata->object_property = (BACNET_PROPERTY_ID)property; + /* Tag 2: Optional Array Index */ + if (len >= apdu_len) { + return -1; + } + tag_len = + decode_tag_number_and_value(&apdu[len], &tag_number, &len_value_type); + if (tag_number == 2) { + len += tag_len; + len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value); + rpdata->array_index = (BACNET_ARRAY_INDEX)unsigned_value; + } else { + rpdata->array_index = BACNET_ARRAY_ALL; + } + /* Tag 3: opening context tag */ + if (len >= apdu_len) { + return -1; + } + if (decode_is_opening_tag_number(&apdu[len], 3)) { + /* a tag number of 3 is not extended so only one octet */ + len++; + /* don't decode the application tag number or its data here */ + rpdata->application_data = &apdu[len]; + /* Just to ensure we do not create a wrapped over value here. */ + if (len < apdu_len) { + rpdata->application_data_len = apdu_len - len - 1 /*closing tag */; + } else { + rpdata->application_data_len = 0; + } + /* len includes the data and the closing tag */ + len = apdu_len; + } else { + return -1; + } } else { return -1; } diff --git a/src/bacnet/rpm.c b/src/bacnet/rpm.c index 0791f5c8..215cb4a8 100644 --- a/src/bacnet/rpm.c +++ b/src/bacnet/rpm.c @@ -43,7 +43,14 @@ /** @file rpm.c Encode/Decode Read Property Multiple and RPM ACKs */ #if BACNET_SVC_RPM_A -/* encode the initial portion of the service */ + +/** Encode the initial portion of the service + * + * @param apdu Pointer to the buffer for encoding. + * @param invoke_id Invoke ID + * + * @return Bytes encoded (usually 4) or zero on error. + */ int rpm_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id) { int apdu_len = 0; /* total length of the apdu, return value */ @@ -59,6 +66,15 @@ int rpm_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id) return apdu_len; } +/** Encode the beginning, including + * Object-id and Read-Access of the service. + * + * @param apdu Pointer to the buffer for encoding. + * @param object_type Object type to encode + * @param object_instance Object instance to encode + * + * @return Bytes encoded or zero on error. + */ int rpm_encode_apdu_object_begin( uint8_t *apdu, BACNET_OBJECT_TYPE object_type, uint32_t object_instance) { @@ -74,6 +90,14 @@ int rpm_encode_apdu_object_begin( return apdu_len; } +/** Encode the object properties of the service. + * + * @param apdu Pointer to the buffer for encoding. + * @param object_property Object property. + * @param array_index Optional array index. + * + * @return Bytes encoded or zero on error. + */ int rpm_encode_apdu_object_property( uint8_t *apdu, BACNET_PROPERTY_ID object_property, BACNET_ARRAY_INDEX array_index) { @@ -91,6 +115,12 @@ int rpm_encode_apdu_object_property( return apdu_len; } +/** Encode the end (closing tag) of the service + * + * @param apdu Pointer to the buffer for encoding. + * + * @return Bytes encoded or zero on error. + */ int rpm_encode_apdu_object_end(uint8_t *apdu) { int apdu_len = 0; /* total length of the apdu, return value */ @@ -130,6 +160,8 @@ int rpm_encode_apdu(uint8_t *apdu, apdu_len += len; rpm_object = read_access_data; while (rpm_object) { + /* The encode function will return a length not more than 12. So the temp buffer + * being 16 bytes is fine enought. */ len = encode_context_object_id(&apdu_temp[0], 0, rpm_object->object_type, rpm_object->object_instance); len = (int)memcopy(&apdu[0], &apdu_temp[0], (size_t)apdu_len, @@ -148,7 +180,9 @@ int rpm_encode_apdu(uint8_t *apdu, apdu_len += len; rpm_property = rpm_object->listOfProperties; while (rpm_property) { - /* stuff as many properties into it as APDU length will allow */ + /* The encode function will return a length not more than 12. So the temp buffer + * being 16 bytes is fine enought. + * Stuff as many properties into it as APDU length will allow. */ len = encode_context_enumerated( &apdu_temp[0], 0, rpm_property->propertyIdentifier); len = (int)memcopy(&apdu[0], &apdu_temp[0], (size_t)apdu_len, @@ -169,6 +203,10 @@ int rpm_encode_apdu(uint8_t *apdu, apdu_len += len; } rpm_property = rpm_property->next; + /* Full? */ + if ((unsigned)apdu_len >= max_apdu) { + return 0; + } } len = encode_closing_tag(&apdu_temp[0], 1); len = (int)memcopy(&apdu[0], &apdu_temp[0], (size_t)apdu_len, @@ -185,8 +223,14 @@ int rpm_encode_apdu(uint8_t *apdu, #endif -/* decode the object portion of the service request only. Bails out if - * tags are wrong or missing/incomplete +/** Decode the object portion of the service request only. Bails out if + * tags are wrong or missing/incomplete. + * + * @param apdu [in] Buffer of bytes received. + * @param apdu_len [in] Count of valid bytes in the buffer. + * @param rpmdata [in] The data structure to be filled. + * + * @return Length of decoded bytes, or 0 on failure. */ int rpm_decode_object_id( uint8_t *apdu, unsigned apdu_len, BACNET_RPM_DATA *rpmdata) @@ -218,6 +262,13 @@ int rpm_decode_object_id( return len; } +/** Decode the end portion of the service request only. + * + * @param apdu [in] Buffer of bytes received. + * @param apdu_len [in] Count of valid bytes in the buffer. + * + * @return Length of decoded bytes (usually 1), or 0 on failure. + */ int rpm_decode_object_end(uint8_t *apdu, unsigned apdu_len) { int len = 0; /* total length of the apdu, return value */ @@ -231,14 +282,20 @@ int rpm_decode_object_end(uint8_t *apdu, unsigned apdu_len) return len; } -/* decode the object property portion of the service request only */ -/* BACnetPropertyReference ::= SEQUENCE { - propertyIdentifier [0] BACnetPropertyIdentifier, - propertyArrayIndex [1] Unsigned OPTIONAL - --used only with array datatype - -- if omitted with an array the entire array is referenced - } -*/ +/** Decode the object property portion of the service request only + * BACnetPropertyReference ::= SEQUENCE { + * propertyIdentifier [0] BACnetPropertyIdentifier, + * propertyArrayIndex [1] Unsigned OPTIONAL + * --used only with array datatype + * -- if omitted with an array the entire array is referenced + * } + * + * @param apdu Pointer to received bytes. + * @param apdu_len Count of received bytes. + * @param rpmdata Pointer to the data structure to be filled. + * + * @return Bytes decoded or zero on failure. + */ int rpm_decode_object_property( uint8_t *apdu, unsigned apdu_len, BACNET_RPM_DATA *rpmdata) { @@ -294,6 +351,13 @@ int rpm_decode_object_property( return len; } +/** Encode the acknowledge for a RPM. + * + * @param apdu [in] Buffer of bytes to transmit. + * @param invoke_id [in] Invoke Id. + * + * @return Length of encoded bytes (usually 3) or 0 on failure. + */ int rpm_ack_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id) { int apdu_len = 0; /* total length of the apdu, return value */ @@ -308,6 +372,13 @@ int rpm_ack_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id) return apdu_len; } +/** Encode the object type for an acknowledge of a RPM. + * + * @param apdu [in] Buffer of bytes to transmit. + * @param rpmdata [in] Pointer to the data used to fill in the APDU. + * + * @return Length of encoded bytes or 0 on failure. + */ int rpm_ack_encode_apdu_object_begin(uint8_t *apdu, BACNET_RPM_DATA *rpmdata) { int apdu_len = 0; /* total length of the apdu, return value */ @@ -323,6 +394,14 @@ int rpm_ack_encode_apdu_object_begin(uint8_t *apdu, BACNET_RPM_DATA *rpmdata) return apdu_len; } +/** Encode the object property for an acknowledge of a RPM. + * + * @param apdu [in] Buffer of bytes to transmit. + * @param object_property [in] Object property ID. + * @param array_index Optional array index + * + * @return Length of encoded bytes or 0 on failure. + */ int rpm_ack_encode_apdu_object_property( uint8_t *apdu, BACNET_PROPERTY_ID object_property, BACNET_ARRAY_INDEX array_index) { @@ -333,14 +412,21 @@ int rpm_ack_encode_apdu_object_property( apdu_len = encode_context_enumerated(&apdu[0], 2, object_property); /* Tag 3: optional propertyArrayIndex */ if (array_index != BACNET_ARRAY_ALL) { - apdu_len += - encode_context_unsigned(&apdu[apdu_len], 3, array_index); + apdu_len += encode_context_unsigned(&apdu[apdu_len], 3, array_index); } } return apdu_len; } +/** Encode the object property value for an acknowledge of a RPM. + * + * @param apdu [in] Buffer of bytes to transmit. + * @param application_data [in] Pointer to the application data used to fill in the APDU. + * @param application_data_len [in] Length of the application data. + * + * @return Length of encoded bytes or 0 on failure. + */ int rpm_ack_encode_apdu_object_property_value( uint8_t *apdu, uint8_t *application_data, unsigned application_data_len) { @@ -364,6 +450,14 @@ int rpm_ack_encode_apdu_object_property_value( return apdu_len; } +/** Encode the object property error for an acknowledge of a RPM. + * + * @param apdu [in] Buffer of bytes to transmit. + * @param error_class [in] Error Class + * @param error_code [in] Error Code + * + * @return Length of encoded bytes or 0 on failure. + */ int rpm_ack_encode_apdu_object_property_error( uint8_t *apdu, BACNET_ERROR_CLASS error_class, BACNET_ERROR_CODE error_code) { @@ -380,6 +474,12 @@ int rpm_ack_encode_apdu_object_property_error( return apdu_len; } +/** Encode the end tag for an acknowledge of a RPM. + * + * @param apdu [in] Buffer of bytes to transmit. + * + * @return Length of encoded bytes or 0 on failure. + */ int rpm_ack_encode_apdu_object_end(uint8_t *apdu) { int apdu_len = 0; /* total length of the apdu, return value */ diff --git a/src/bacnet/wp.c b/src/bacnet/wp.c index 46b3531f..ec437604 100644 --- a/src/bacnet/wp.c +++ b/src/bacnet/wp.c @@ -38,13 +38,22 @@ #include "bacnet/wp.h" /** @file wp.c Encode/Decode BACnet Write Property APDUs */ + #if BACNET_SVC_WP_A -/* encode service */ +/** Initialize the APDU for encode service. + * + * @param apdu Pointer to the buffer. + * @param invoke_id ID of service invoked. + * @param wpdata Pointer to the write property data. + * + * @return Bytes encoded + */ int wp_encode_apdu( uint8_t *apdu, uint8_t invoke_id, BACNET_WRITE_PROPERTY_DATA *wpdata) { int apdu_len = 0; /* total length of the apdu, return value */ int len = 0; /* total length of the apdu, return value */ + int imax = 0; /* maximum application data length */ if (apdu) { apdu[0] = PDU_TYPE_CONFIRMED_SERVICE_REQUEST; @@ -67,10 +76,14 @@ int wp_encode_apdu( /* propertyValue */ len = encode_opening_tag(&apdu[apdu_len], 3); apdu_len += len; - for (len = 0; len < wpdata->application_data_len; len++) { + imax = wpdata->application_data_len; + if (imax > (MAX_APDU - 2 /*closing*/ - apdu_len)) { + imax = MAX_APDU - 2 - apdu_len; + } + for (len = 0; len < imax; len++) { apdu[apdu_len + len] = wpdata->application_data[len]; } - apdu_len += wpdata->application_data_len; + apdu_len += imax; len = encode_closing_tag(&apdu[apdu_len], 3); apdu_len += len; /* optional priority - 0 if not set, 1..16 if set */ @@ -84,9 +97,17 @@ int wp_encode_apdu( } #endif -/* decode the service request only */ -/* FIXME: there could be various error messages returned - using unique values less than zero */ +/** Decode the service request only + * + * FIXME: there could be various error messages returned + * using unique values less than zero. + * + * @param apdu Pointer to the buffer. + * @param apdu_len Valid bytes in the buffer + * @param wpdata Pointer to the write property data. + * + * @return Bytes encoded or a negative value as error. + */ int wp_decode_service_request( uint8_t *apdu, unsigned apdu_len, BACNET_WRITE_PROPERTY_DATA *wpdata) { @@ -98,6 +119,7 @@ int wp_decode_service_request( uint32_t property = 0; /* for decoding */ BACNET_UNSIGNED_INTEGER unsigned_value = 0; int i = 0; /* loop counter */ + int imax = 0; /* max application data length */ /* check for value pointers */ if (apdu_len && wpdata) { @@ -132,16 +154,23 @@ int wp_decode_service_request( return -1; } /* determine the length of the data blob */ - wpdata->application_data_len = bacapp_data_len( - &apdu[len], apdu_len - len, (BACNET_PROPERTY_ID)property); + imax = bacapp_data_len( \ + &apdu[len], apdu_len - len, (BACNET_PROPERTY_ID)property); + if (imax == BACNET_STATUS_ERROR) { + return -2; + } /* a tag number of 3 is not extended so only one octet */ len++; /* copy the data from the APDU */ - for (i = 0; i < wpdata->application_data_len; i++) { + if (imax > (MAX_APDU - len - 1 /*closing*/)) { + imax = (MAX_APDU - len - 1); + } + for (i = 0; i < imax; i++) { wpdata->application_data[i] = apdu[len + i]; } + wpdata->application_data_len = imax; /* add on the data length */ - len += wpdata->application_data_len; + len += imax; if (!decode_is_closing_tag_number(&apdu[len], 3)) { return -2; } diff --git a/src/bacnet/wpm.c b/src/bacnet/wpm.c index 726fda95..d260c9e8 100644 --- a/src/bacnet/wpm.c +++ b/src/bacnet/wpm.c @@ -46,8 +46,10 @@ * * @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 + * @param wp_data [out] The BACNET_WRITE_PROPERTY_DATA structure * which will contain the reponse values or error. + * + * @return Count of decoded bytes. */ int wpm_decode_object_id( uint8_t *apdu, uint16_t apdu_len, BACNET_WRITE_PROPERTY_DATA *wp_data) @@ -102,6 +104,14 @@ int wpm_decode_object_id( return (int)len; } +/** Decoding for an object property. + * + * @param apdu [in] The contents of the APDU buffer. + * @param apdu_len [in] The length of the APDU buffer. + * @param wp_data [out] The BACNET_WRITE_PROPERTY_DATA structure. + * + * @return Bytes decoded + */ int wpm_decode_object_property( uint8_t *apdu, uint16_t apdu_len, BACNET_WRITE_PROPERTY_DATA *wp_data) { @@ -109,7 +119,7 @@ int wpm_decode_object_property( uint32_t len_value = 0; uint32_t enum_value = 0; BACNET_UNSIGNED_INTEGER unsigned_value = 0; - int len = 0, i = 0; + int len = 0, i = 0, imax = 0; if ((apdu) && (apdu_len) && (wp_data)) { wp_data->array_index = BACNET_ARRAY_ALL; @@ -137,19 +147,28 @@ int wpm_decode_object_property( /* tag 2 - Property Value */ if ((tag_number == 2) && (decode_is_opening_tag(&apdu[len - 1]))) { len--; - wp_data->application_data_len = bacapp_data_len(&apdu[len], + imax = bacapp_data_len(&apdu[len], (unsigned)(apdu_len - len), wp_data->object_property); len++; - /* copy application data */ - for (i = 0; i < wp_data->application_data_len; i++) { + /* copy application data, check max lengh */ + if (imax > (apdu_len - len)) { + imax = (apdu_len - len); + } + for (i = 0; i < imax; 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]))) { + wp_data->application_data_len = imax; + len += imax; + if (len < apdu_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]))) { + 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; } @@ -159,12 +178,14 @@ int wpm_decode_object_property( } /* tag 3 - Priority - optional */ - len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); - if (tag_number == 3) { - len += decode_unsigned(&apdu[len], len_value, &unsigned_value); - wp_data->priority = (uint8_t)unsigned_value; - } else { - len--; + if (len < apdu_len) { + len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); + if (tag_number == 3) { + len += decode_unsigned(&apdu[len], len_value, &unsigned_value); + wp_data->priority = (uint8_t)unsigned_value; + } else { + len--; + } } } else { if (wp_data) { @@ -176,7 +197,13 @@ int wpm_decode_object_property( return len; } -/* encode functions */ +/** Init the APDU for encoding. + * + * @param apdu [in] The APDU buffer. + * @param invoke_id [in] The ID of the saervice invoked. + * + * @return Bytes encoded, usually 4 on success. + */ int wpm_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id) { int apdu_len = 0; /* total length of the apdu, return value */ @@ -192,6 +219,14 @@ int wpm_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id) return apdu_len; } +/** Decode the very begin of an object in the APDU. + * + * @param apdu [in] The APDU buffer. + * @param object_type [in] The object type to decode. + * @param object_instance [in] The object instance. + * + * @return Bytes encoded. + */ int wpm_encode_apdu_object_begin( uint8_t *apdu, BACNET_OBJECT_TYPE object_type, uint32_t object_instance) { @@ -207,6 +242,12 @@ int wpm_encode_apdu_object_begin( return apdu_len; } +/** Decode the very end of an object in the APDU. + * + * @param apdu [in] The APDU buffer. + * + * @return Bytes encoded. + */ int wpm_encode_apdu_object_end(uint8_t *apdu) { int apdu_len = 0; /* total length of the apdu, return value */ @@ -218,11 +259,19 @@ int wpm_encode_apdu_object_end(uint8_t *apdu) return apdu_len; } +/** Encode the object property into the APDU. + * + * @param apdu [in] The APDU buffer. + * @param wpdata [in] Pointer to the property data. + * + * @return Bytes encoded. + */ int wpm_encode_apdu_object_property( uint8_t *apdu, BACNET_WRITE_PROPERTY_DATA *wpdata) { int apdu_len = 0; /* total length of the apdu, return value */ int len = 0; + int imax; if (apdu) { apdu_len = @@ -233,20 +282,35 @@ int wpm_encode_apdu_object_property( &apdu[apdu_len], 1, wpdata->array_index); } apdu_len += encode_opening_tag(&apdu[apdu_len], 2); - for (len = 0; len < wpdata->application_data_len; len++) { + imax = wpdata->application_data_len; + if (imax > (MAX_APDU - apdu_len - 2)) { + imax = (MAX_APDU - apdu_len - 2); + } + for (len = 0; len < imax; len++) { apdu[apdu_len] = wpdata->application_data[len]; apdu_len++; } apdu_len += encode_closing_tag(&apdu[apdu_len], 2); if (wpdata->priority != BACNET_NO_PRIORITY) { - apdu_len += - encode_context_unsigned(&apdu[apdu_len], 3, wpdata->priority); + if (apdu_len < MAX_APDU) { + apdu_len += + encode_context_unsigned(&apdu[apdu_len], 3, wpdata->priority); + } } } return apdu_len; } +/** Encode the request into the APDU. + * + * @param apdu [in] The APDU buffer. + * @param max_apdu [in] Maximum space in the buffer. + * @param invoke_id [in] Invoked service ID. + * @param write_access_data [in] Access data. + * + * @return Bytes encoded. + */ int wpm_encode_apdu(uint8_t *apdu, size_t max_apdu, uint8_t invoke_id, @@ -254,6 +318,7 @@ int wpm_encode_apdu(uint8_t *apdu, { int apdu_len = 0; int len = 0; + size_t usize; BACNET_WRITE_ACCESS_DATA *wpm_object; /* current object */ uint8_t apdu_temp[MAX_APDU]; /* temp for data before copy */ BACNET_PROPERTY_VALUE *wpm_property; /* current property */ @@ -271,6 +336,9 @@ int wpm_encode_apdu(uint8_t *apdu, len = wpm_encode_apdu_object_begin(&apdu[apdu_len], wpm_object->object_type, wpm_object->object_instance); apdu_len += len; + if (apdu_len >= (int)max_apdu) { + break; + } wpm_property = wpm_object->listOfProperties; @@ -278,12 +346,18 @@ int wpm_encode_apdu(uint8_t *apdu, wpdata.object_property = wpm_property->propertyIdentifier; wpdata.array_index = wpm_property->propertyArrayIndex; wpdata.priority = wpm_property->priority; - wpdata.application_data_len = - bacapp_encode_data(&apdu_temp[0], &wpm_property->value); - memcpy(&wpdata.application_data[0], &apdu_temp[0], - (size_t)wpdata.application_data_len); + usize = (size_t)bacapp_encode_data(&apdu_temp[0], &wpm_property->value); + if (usize > sizeof(wpdata.application_data)) + { + usize = sizeof(wpdata.application_data); + } + wpdata.application_data_len = (int)usize; + memcpy(&wpdata.application_data[0], &apdu_temp[0], usize); len = wpm_encode_apdu_object_property(&apdu[apdu_len], &wpdata); apdu_len += len; + if (apdu_len >= (int)max_apdu) { + break; + } wpm_property = wpm_property->next; } @@ -298,6 +372,13 @@ int wpm_encode_apdu(uint8_t *apdu, return apdu_len; } +/** Init the APDU for encoding the confiremd write property multiple service. + * + * @param apdu [in] The APDU buffer. + * @param invoke_id [in] Invoked service ID. + * + * @return Bytes encoded, usually 3. + */ int wpm_ack_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id) { int len = 0; @@ -311,6 +392,14 @@ int wpm_ack_encode_apdu_init(uint8_t *apdu, uint8_t invoke_id) return len; } +/** Encode an Error acknowledge in the APDU. + * + * @param apdu [in] The APDU buffer. + * @param invoke_id [in] Invoked service ID. + * @param wp_data [in] Data of the invoked property. + * + * @return Bytes encoded. + */ int wpm_error_ack_encode_apdu( uint8_t *apdu, uint8_t invoke_id, BACNET_WRITE_PROPERTY_DATA *wp_data) {