diff --git a/.travis.yml b/.travis.yml index 9d3603b5..bd4a21f2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,8 +30,7 @@ jobs: - stage: lint os: linux compiler: clang - # script: scan-build --status-bugs -analyze-headers -v make -j2 clean server - script: skip + script: make clean lint - stage: ports-arm os: linux before_script: diff --git a/Makefile b/Makefile index 92281747..01a612b8 100644 --- a/Makefile +++ b/Makefile @@ -115,6 +115,10 @@ tidy: clang-tidy {} -fix-errors -checks="readability-braces-around-statements" \ -- -Isrc -Iports/linux \; +.PHONY : lint +lint: + scan-build --status-bugs -analyze-headers make -j2 clean server + .PHONY: clean clean: $(MAKE) -s -C src clean diff --git a/src/bacnet/basic/binding/address.c b/src/bacnet/basic/binding/address.c index 9f7ad2fd..b338bc95 100644 --- a/src/bacnet/basic/binding/address.c +++ b/src/bacnet/basic/binding/address.c @@ -234,13 +234,15 @@ void address_mac_init(BACNET_MAC_ADDRESS *mac, uint8_t *adr, uint8_t len) { uint8_t i = 0; - if (mac && adr && (len <= sizeof(mac->adr))) { - for (i = 0; i < len; i++) { - mac->adr[i] = adr[i]; + if (mac) { + if (adr && (len <= sizeof(mac->adr))) { + for (i = 0; i < len; i++) { + mac->adr[i] = adr[i]; + } + mac->len = len; + } else { + mac->len = 0; } - mac->len = len; - } else { - mac->len = 0; } } diff --git a/src/bacnet/basic/object/bacfile.c b/src/bacnet/basic/object/bacfile.c index 647a9ff5..29e78d74 100644 --- a/src/bacnet/basic/object/bacfile.c +++ b/src/bacnet/basic/object/bacfile.c @@ -394,7 +394,8 @@ uint32_t bacfile_instance_from_tsm(uint8_t invokeID) len = apdu_decode_confirmed_service_request(&apdu[0], apdu_len, &service_data, &service_choice, &service_request, &service_request_len); - if (service_choice == SERVICE_CONFIRMED_ATOMIC_READ_FILE) { + if ((len > 0) && + (service_choice == SERVICE_CONFIRMED_ATOMIC_READ_FILE)) { len = arf_decode_service_request( service_request, service_request_len, &data); if (len > 0) { diff --git a/src/bacnet/basic/object/channel.c b/src/bacnet/basic/object/channel.c index 8064d1e5..dcffe080 100644 --- a/src/bacnet/basic/object/channel.c +++ b/src/bacnet/basic/object/channel.c @@ -1213,8 +1213,10 @@ bool Channel_Present_Value_Set( if (wp_data->priority != 6 /* reserved */) { status = Channel_Value_Copy(&Channel[index].Present_Value, value); + (void)status; status = Channel_Write_Members( &Channel[index], value, wp_data->priority); + (void)status; status = true; } else { /* Command priority 6 is reserved for use by Minimum On/Off diff --git a/src/bacnet/basic/object/device.c b/src/bacnet/basic/object/device.c index c952927f..afb49790 100644 --- a/src/bacnet/basic/object/device.c +++ b/src/bacnet/basic/object/device.c @@ -1757,11 +1757,11 @@ void Device_COV_Clear(BACNET_OBJECT_TYPE object_type, uint32_t object_instance) #if defined(INTRINSIC_REPORTING) void Device_local_reporting(void) { - struct object_functions *pObject; - uint32_t objects_count; - uint32_t object_instance; - BACNET_OBJECT_TYPE object_type; - uint32_t idx; + struct object_functions *pObject = NULL; + uint32_t objects_count = 0; + uint32_t object_instance = 0; + BACNET_OBJECT_TYPE object_type = OBJECT_NONE; + uint32_t idx = 0; objects_count = Device_Object_List_Count(); diff --git a/src/bacnet/basic/object/lc.c b/src/bacnet/basic/object/lc.c index 1bc4ba6a..17f1e0f2 100644 --- a/src/bacnet/basic/object/lc.c +++ b/src/bacnet/basic/object/lc.c @@ -901,7 +901,11 @@ bool Load_Control_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) len = bacapp_decode_context_data(wp_data->application_data, wp_data->application_data_len, &value, PROP_REQUESTED_SHED_LEVEL); - if (value.context_tag == 0) { + if (len == BACNET_STATUS_ERROR) { + /* error! */ + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_INVALID_DATA_TYPE; + } else if (value.context_tag == 0) { /* percent - Unsigned */ Requested_Shed_Level[object_index].type = BACNET_SHED_TYPE_PERCENT; diff --git a/src/bacnet/basic/object/lo.c b/src/bacnet/basic/object/lo.c index c29e9082..989bb0a8 100644 --- a/src/bacnet/basic/object/lo.c +++ b/src/bacnet/basic/object/lo.c @@ -1034,6 +1034,15 @@ int Lighting_Output_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) } else { len = encode_application_null(&apdu[apdu_len]); } + /* add it if we have room */ + if ((apdu_len + len) < MAX_APDU) { + apdu_len += len; + } else { + rpdata->error_code = + ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED; + apdu_len = BACNET_STATUS_ABORT; + break; + } } else { rpdata->error_class = ERROR_CLASS_PROPERTY; rpdata->error_code = ERROR_CODE_INVALID_ARRAY_INDEX; diff --git a/src/bacnet/basic/object/netport.c b/src/bacnet/basic/object/netport.c index c4c28d0d..7108b708 100644 --- a/src/bacnet/basic/object/netport.c +++ b/src/bacnet/basic/object/netport.c @@ -930,9 +930,12 @@ bool Network_Port_IP_Subnet( if (index < BACNET_NETWORK_PORTS_MAX) { if (Object_List[index].Network_Type == PORT_TYPE_BIP) { prefix = Object_List[index].Network.IPv4.IP_Subnet_Prefix; - mask = (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF; - encode_unsigned32(ip_mask, mask); - status = octetstring_init(subnet_mask, ip_mask, sizeof(ip_mask)); + if ((prefix > 0) && (prefix <= 32)) { + mask = (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF; + encode_unsigned32(ip_mask, mask); + status = octetstring_init(subnet_mask, ip_mask, + sizeof(ip_mask)); + } } } @@ -979,7 +982,7 @@ bool Network_Port_IP_Subnet_Prefix_Set(uint32_t object_instance, uint8_t value) index = Network_Port_Instance_To_Index(object_instance); if (index < BACNET_NETWORK_PORTS_MAX) { if (Object_List[index].Network_Type == PORT_TYPE_BIP) { - if (value <= 32) { + if ((value > 0) && (value <= 32)) { if (Object_List[index].Network.IPv4.IP_Subnet_Prefix != value) { Object_List[index].Changes_Pending = true; } @@ -1423,7 +1426,7 @@ bool Network_Port_IPv6_Subnet_Prefix_Set( index = Network_Port_Instance_To_Index(object_instance); if (index < BACNET_NETWORK_PORTS_MAX) { if (Object_List[index].Network_Type == PORT_TYPE_BIP6) { - if (value <= 128) { + if ((value > 0) && (value <= 128)) { if (Object_List[index].Network.IPv6.IP_Subnet_Prefix != value) { Object_List[index].Changes_Pending = true; } diff --git a/src/bacnet/basic/service/h_apdu.c b/src/bacnet/basic/service/h_apdu.c index 0027bf2b..25d23dcd 100644 --- a/src/bacnet/basic/service/h_apdu.c +++ b/src/bacnet/basic/service/h_apdu.c @@ -314,21 +314,31 @@ uint16_t apdu_decode_confirmed_service_request(uint8_t *apdu, /* APDU data */ { uint16_t len = 0; /* counts where we are in PDU */ - service_data->segmented_message = (apdu[0] & BIT(3)) ? true : false; - service_data->more_follows = (apdu[0] & BIT(2)) ? true : false; - service_data->segmented_response_accepted = - (apdu[0] & BIT(1)) ? true : false; - service_data->max_segs = decode_max_segs(apdu[1]); - service_data->max_resp = decode_max_apdu(apdu[1]); - service_data->invoke_id = apdu[2]; - len = 3; - if (service_data->segmented_message) { - service_data->sequence_number = apdu[len++]; - service_data->proposed_window_number = apdu[len++]; + if (apdu_len >= 3) { + service_data->segmented_message = (apdu[0] & BIT(3)) ? true : false; + service_data->more_follows = (apdu[0] & BIT(2)) ? true : false; + service_data->segmented_response_accepted = + (apdu[0] & BIT(1)) ? true : false; + service_data->max_segs = decode_max_segs(apdu[1]); + service_data->max_resp = decode_max_apdu(apdu[1]); + service_data->invoke_id = apdu[2]; + len = 3; + if (service_data->segmented_message) { + if (apdu_len >= (len+2)) { + service_data->sequence_number = apdu[len++]; + service_data->proposed_window_number = apdu[len++]; + } else { + return 0; + } + } + if (apdu_len >= (len+2)) { + *service_choice = apdu[len++]; + *service_request = &apdu[len]; + *service_request_len = apdu_len - len; + } else { + return 0; + } } - *service_choice = apdu[len++]; - *service_request = &apdu[len]; - *service_request_len = apdu_len - len; return len; } diff --git a/src/bacnet/basic/service/h_rp_a.c b/src/bacnet/basic/service/h_rp_a.c index a0ece448..0682a75d 100644 --- a/src/bacnet/basic/service/h_rp_a.c +++ b/src/bacnet/basic/service/h_rp_a.c @@ -184,7 +184,6 @@ int rp_ack_fully_decode_service_request( vlen = rp1data.application_data_len; value = calloc(1, sizeof(BACNET_APPLICATION_DATA_VALUE)); rp1_property->value = value; - old_value = value; while (value && vdata && (vlen > 0)) { if (IS_CONTEXT_SPECIFIC(*vdata)) { len = bacapp_decode_context_data( diff --git a/src/bacnet/basic/service/h_rpm.c b/src/bacnet/basic/service/h_rpm.c index 15f31e40..c76ad7e9 100644 --- a/src/bacnet/basic/service/h_rpm.c +++ b/src/bacnet/basic/service/h_rpm.c @@ -119,7 +119,6 @@ static int RPM_Encode_Property( return BACNET_STATUS_ABORT; } apdu_len += len; - len = 0; rpdata.error_class = ERROR_CLASS_OBJECT; rpdata.error_code = ERROR_CODE_UNKNOWN_OBJECT; rpdata.object_type = rpmdata->object_type; diff --git a/src/bacnet/basic/service/h_rpm_a.c b/src/bacnet/basic/service/h_rpm_a.c index 3d7ce6e4..a246b1de 100644 --- a/src/bacnet/basic/service/h_rpm_a.c +++ b/src/bacnet/basic/service/h_rpm_a.c @@ -25,7 +25,6 @@ #include #include #include -#include #include "bacnet/config.h" #include "bacnet/bacdef.h" #include "bacnet/bacdcode.h" @@ -66,7 +65,9 @@ int rpm_ack_decode_service_request( BACNET_APPLICATION_DATA_VALUE *value; BACNET_APPLICATION_DATA_VALUE *old_value; - assert(read_access_data != NULL); + if (!read_access_data) { + return 0; + } rpm_object = read_access_data; old_rpm_object = rpm_object; while (rpm_object && apdu_len) { @@ -74,7 +75,11 @@ int rpm_ack_decode_service_request( &rpm_object->object_instance); if (len <= 0) { old_rpm_object->next = NULL; - free(rpm_object); + if (rpm_object != read_access_data) { + /* don't free original */ + free(rpm_object); + rpm_object = NULL; + } break; } decoded_len += len; @@ -94,6 +99,7 @@ int rpm_ack_decode_service_request( rpm_object->listOfProperties = NULL; } free(rpm_property); + rpm_property = NULL; break; } decoded_len += len; @@ -108,7 +114,6 @@ int rpm_ack_decode_service_request( more than one element to decode */ value = calloc(1, sizeof(BACNET_APPLICATION_DATA_VALUE)); rpm_property->value = value; - old_value = value; while (value && (apdu_len > 0)) { if (IS_CONTEXT_SPECIFIC(*apdu)) { len = bacapp_decode_context_data(apdu, apdu_len, value, @@ -199,8 +204,8 @@ void rpm_ack_print_data(BACNET_READ_ACCESS_DATA *rpm_data) #ifdef BACAPP_PRINT_ENABLED BACNET_OBJECT_PROPERTY_VALUE object_value; /* for bacapp printing */ #endif - BACNET_PROPERTY_REFERENCE *listOfProperties; - BACNET_APPLICATION_DATA_VALUE *value; + BACNET_PROPERTY_REFERENCE *listOfProperties = NULL; + BACNET_APPLICATION_DATA_VALUE *value = NULL; #if PRINT_ENABLED bool array_value = false; #endif @@ -283,6 +288,44 @@ void rpm_ack_print_data(BACNET_READ_ACCESS_DATA *rpm_data) } } +/** + * Free the allocated memory from a ReadPropertyMultiple ACK. + * @param rpm_data - #BACNET_READ_ACCESS_DATA + * @return RPM data from the next element in the linked list + */ +static BACNET_READ_ACCESS_DATA *rpm_data_free( + BACNET_READ_ACCESS_DATA *rpm_data) +{ + BACNET_READ_ACCESS_DATA *old_rpm_data = NULL; + BACNET_PROPERTY_REFERENCE *rpm_property = NULL; + BACNET_PROPERTY_REFERENCE *old_rpm_property = NULL; + BACNET_APPLICATION_DATA_VALUE *value = NULL; + BACNET_APPLICATION_DATA_VALUE *old_value = NULL; + + if (rpm_data) { + rpm_property = rpm_data->listOfProperties; + while (rpm_property) { + value = rpm_property->value; + while (value) { + old_value = value; + value = value->next; + free(old_value); + old_value = NULL; + } + old_rpm_property = rpm_property; + rpm_property = rpm_property->next; + free(old_rpm_property); + old_rpm_property = NULL; + } + old_rpm_data = rpm_data; + rpm_data = rpm_data->next; + free(old_rpm_data); + old_rpm_data = NULL; + } + + return rpm_data; +} + /** Handler for a ReadPropertyMultiple ACK. * @ingroup DSRPM * For each read property, print out the ACK'd data for debugging, @@ -300,12 +343,7 @@ void handler_read_property_multiple_ack(uint8_t *service_request, BACNET_CONFIRMED_SERVICE_ACK_DATA *service_data) { int len = 0; - BACNET_READ_ACCESS_DATA *rpm_data; - BACNET_READ_ACCESS_DATA *old_rpm_data; - BACNET_PROPERTY_REFERENCE *rpm_property; - BACNET_PROPERTY_REFERENCE *old_rpm_property; - BACNET_APPLICATION_DATA_VALUE *value; - BACNET_APPLICATION_DATA_VALUE *old_value; + BACNET_READ_ACCESS_DATA * rpm_data; (void)src; (void)service_data; /* we could use these... */ @@ -314,49 +352,18 @@ void handler_read_property_multiple_ack(uint8_t *service_request, if (rpm_data) { len = rpm_ack_decode_service_request( service_request, service_len, rpm_data); - } -#if 1 - fprintf(stderr, "Received Read-Property-Multiple Ack!\n"); -#endif - if (len > 0) { - while (rpm_data) { - rpm_ack_print_data(rpm_data); - rpm_property = rpm_data->listOfProperties; - while (rpm_property) { - value = rpm_property->value; - while (value) { - old_value = value; - value = value->next; - free(old_value); - } - old_rpm_property = rpm_property; - rpm_property = rpm_property->next; - free(old_rpm_property); + if (len > 0) { + while (rpm_data) { + rpm_ack_print_data(rpm_data); + rpm_data = rpm_data_free(rpm_data); } - old_rpm_data = rpm_data; - rpm_data = rpm_data->next; - free(old_rpm_data); - } - } else { -#if 1 - fprintf(stderr, "RPM Ack Malformed! Freeing memory...\n"); -#endif - while (rpm_data) { - rpm_property = rpm_data->listOfProperties; - while (rpm_property) { - value = rpm_property->value; - while (value) { - old_value = value; - value = value->next; - free(old_value); - } - old_rpm_property = rpm_property; - rpm_property = rpm_property->next; - free(old_rpm_property); + } else { + #if 1 + fprintf(stderr, "RPM Ack Malformed! Freeing memory...\n"); + #endif + while (rpm_data) { + rpm_data = rpm_data_free(rpm_data); } - old_rpm_data = rpm_data; - rpm_data = rpm_data->next; - free(old_rpm_data); } } } diff --git a/src/bacnet/basic/sys/keylist.c b/src/bacnet/basic/sys/keylist.c index b6dc79c9..7478d6ba 100644 --- a/src/bacnet/basic/sys/keylist.c +++ b/src/bacnet/basic/sys/keylist.c @@ -75,7 +75,7 @@ static int CheckArraySize(OS_Keylist list) { int new_size = 0; /* set it up so that no size change is the default */ const int chunk = 8; /* minimum number of nodes to allocate memory for */ - struct Keylist_Node **new_array; /* new array of nodes, if needed */ + struct Keylist_Node **new_array = NULL; /* new array of nodes, if needed */ int i; /* counter */ if (!list) { return FALSE; @@ -91,7 +91,8 @@ static int CheckArraySize(OS_Keylist list) } if (new_size) { /* Allocate more room for node pointer array */ - new_array = calloc((size_t)new_size, sizeof(new_array)); + new_array = calloc((size_t)new_size, + sizeof(struct Keylist_Node *)); /* See if we got the memory we wanted */ if (!new_array) { diff --git a/src/bacnet/datalink/dlenv.c b/src/bacnet/datalink/dlenv.c index 851723ab..67c920cc 100644 --- a/src/bacnet/datalink/dlenv.c +++ b/src/bacnet/datalink/dlenv.c @@ -220,7 +220,7 @@ static void dlenv_network_port_init(void) uint32_t address = 0; uint32_t broadcast = 0; uint32_t test_broadcast = 0; - uint32_t mask = 0; + uint32_t mask = 0xFFFFFFFE; uint16_t port = 0; uint8_t mac[4 + 2] = { 0 }; uint8_t prefix = 0; @@ -235,12 +235,13 @@ static void dlenv_network_port_init(void) memcpy(&mac[4], &port, 2); Network_Port_MAC_Address_Set(instance, &mac[0], 6); broadcast = bip_get_broadcast_addr(); - for (prefix = 0; prefix < 32; prefix++) { - mask = htonl((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF); + /* calculate the subnet prefix from the broadcast address */ + for (prefix = 1; prefix <= 32; prefix++) { test_broadcast = (address & mask) | (~mask); if (test_broadcast == broadcast) { break; } + mask = mask<<1; } Network_Port_IP_Subnet_Prefix_Set(instance, prefix); Network_Port_Link_Speed_Set(instance, 0.0); diff --git a/src/bacnet/readrange.c b/src/bacnet/readrange.c index f58274cf..47c1107d 100644 --- a/src/bacnet/readrange.c +++ b/src/bacnet/readrange.c @@ -428,7 +428,6 @@ int rr_ack_decode_service_request(uint8_t *apdu, decode_unsigned(&apdu[len], len_value_type, &rrdata->FirstSequence); } - len = apdu_len; /* There should be nothing left to see here */ return len; } diff --git a/src/bacnet/timestamp.c b/src/bacnet/timestamp.c index 8fb537ed..70c58e23 100644 --- a/src/bacnet/timestamp.c +++ b/src/bacnet/timestamp.c @@ -104,7 +104,7 @@ int bacapp_encode_timestamp(uint8_t *apdu, BACNET_TIMESTAMP *value) default: len = 0; - assert(0); + assert(len); break; } } diff --git a/src/bacnet/wpm.c b/src/bacnet/wpm.c index 5dbfb792..d85fc839 100644 --- a/src/bacnet/wpm.c +++ b/src/bacnet/wpm.c @@ -58,7 +58,7 @@ int wpm_decode_object_id( BACNET_OBJECT_TYPE object_type = OBJECT_NONE; uint16_t len = 0; - if (apdu && (apdu_len > 5) && wp_data) { + if (apdu && (apdu_len > 5)) { /* Context tag 0 - Object ID */ len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); if ((tag_number == 0) && (apdu_len > len)) { @@ -66,26 +66,36 @@ int wpm_decode_object_id( if (apdu_len >= 4) { len += decode_object_id( &apdu[len], &object_type, &object_instance); - wp_data->object_type = object_type; - wp_data->object_instance = object_instance; + if (wp_data) { + wp_data->object_type = object_type; + wp_data->object_instance = object_instance; + } apdu_len -= len; } else { - wp_data->error_code = - ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER; + if (wp_data) { + wp_data->error_code = + ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER; + } return BACNET_STATUS_REJECT; } } else { - wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG; + if (wp_data) { + wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG; + } return BACNET_STATUS_REJECT; } /* just test for the next tag - no need to decode it here */ /* Context tag 1: sequence of BACnetPropertyValue */ if (apdu_len && !decode_is_opening_tag_number(&apdu[len], 1)) { - wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG; + if (wp_data) { + wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG; + } return BACNET_STATUS_REJECT; } } else { - wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER; + if (wp_data) { + wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER; + } return BACNET_STATUS_REJECT; } @@ -156,7 +166,9 @@ int wpm_decode_object_property( len--; } } else { - wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER; + if (wp_data) { + wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER; + } return BACNET_STATUS_REJECT; }