From 86dbaf409b8ca3d78eb195fd24261ec8bbe080fa Mon Sep 17 00:00:00 2001 From: skarg Date: Tue, 12 Oct 2010 03:02:05 +0000 Subject: [PATCH] Added handling for bacapp decode value returning error codes. --- bacnet-stack/demo/handler/h_rp_a.c | 40 ++++++++++++++++++++++------- bacnet-stack/demo/handler/h_rpm_a.c | 9 ++++--- bacnet-stack/demo/handler/h_rr_a.c | 8 +++--- bacnet-stack/src/bacapp.c | 10 ++++++-- 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/bacnet-stack/demo/handler/h_rp_a.c b/bacnet-stack/demo/handler/h_rp_a.c index 01f51a0d..fa62b01a 100644 --- a/bacnet-stack/demo/handler/h_rp_a.c +++ b/bacnet-stack/demo/handler/h_rp_a.c @@ -79,7 +79,7 @@ void rp_ack_print_data( object_value.array_index = data->array_index; object_value.value = &value; bacapp_print_value(stdout, &object_value); - if (len) { + if (len > 0) { if (len < application_data_len) { application_data += len; application_data_len -= len; @@ -87,10 +87,12 @@ void rp_ack_print_data( #if PRINT_ENABLED fprintf(stdout, ","); #endif - } else + } else { break; - } else + } + } else { break; + } } #if PRINT_ENABLED if (print_brace) @@ -166,8 +168,10 @@ int rp_ack_fully_decode_service_request( read_access_data->object_instance = rp1data.object_instance; rp1_property = calloc(1, sizeof(BACNET_PROPERTY_REFERENCE)); read_access_data->listOfProperties = rp1_property; - if (rp1_property == NULL) - return -1; /* can't proceed if calloc failed. */ + if (rp1_property == NULL) { + /* can't proceed if calloc failed. */ + return BACNET_STATUS_ERROR; + } rp1_property->propertyIdentifier = rp1data.object_property; rp1_property->propertyArrayIndex = rp1data.array_index; /* Is there no Error case possible here, as there is when decoding RPM? */ @@ -189,6 +193,18 @@ int rp_ack_fully_decode_service_request( } else { len = bacapp_decode_application_data(vdata, vlen, value); } + if (len < 0) { + /* unable to decode the data */ + while (value) { + /* free the linked list of values */ + old_value = value; + value = value->next; + free(old_value); + } + free(rp1_property); + read_access_data->listOfProperties = NULL; + return len; + } decoded_len += len; vlen -= len; vdata += len; @@ -199,14 +215,20 @@ int rp_ack_fully_decode_service_request( vdata++; break; } else { - /* nothing decoded and no closing tag, so malformed */ if (len == 0) { - free(value); + /* nothing decoded and no closing tag, so malformed */ + while (value) { + /* free the linked list of values */ + old_value = value; + value = value->next; + free(old_value); + } free(rp1_property); read_access_data->listOfProperties = NULL; - return -1; + return BACNET_STATUS_ERROR; } - if (vlen > 0) { /* If more values */ + if (vlen > 0) { + /* If more values */ old_value = value; value = calloc(1, sizeof(BACNET_APPLICATION_DATA_VALUE)); old_value->next = value; diff --git a/bacnet-stack/demo/handler/h_rpm_a.c b/bacnet-stack/demo/handler/h_rpm_a.c index b36686f6..aafe84c3 100644 --- a/bacnet-stack/demo/handler/h_rpm_a.c +++ b/bacnet-stack/demo/handler/h_rpm_a.c @@ -125,6 +125,11 @@ int rpm_ack_decode_service_request( bacapp_decode_application_data(apdu, apdu_len, value); } + if (len <= 0) { + /* problem decoding */ + /* calling function will free the memory */ + return BACNET_STATUS_ERROR; + } decoded_len += len; apdu_len -= len; apdu += len; @@ -134,10 +139,6 @@ int rpm_ack_decode_service_request( apdu++; break; } else { - /* nothing decoded and no closing tag, so malformed */ - if (len == 0) { - return -1; - } old_value = value; value = calloc(1, sizeof(BACNET_APPLICATION_DATA_VALUE)); diff --git a/bacnet-stack/demo/handler/h_rr_a.c b/bacnet-stack/demo/handler/h_rr_a.c index 4f5dbd92..17ab4f84 100644 --- a/bacnet-stack/demo/handler/h_rr_a.c +++ b/bacnet-stack/demo/handler/h_rr_a.c @@ -76,7 +76,7 @@ static void PrintReadRangeData( object_value.array_index = data->array_index; object_value.value = &value; bacapp_print_value(stdout, &object_value); - if (len) { + if (len > 0) { if (len < application_data_len) { application_data += len; application_data_len -= len; @@ -84,10 +84,12 @@ static void PrintReadRangeData( #if PRINT_ENABLED fprintf(stdout, ","); #endif - } else + } else { break; - } else + } + } else { break; + } } #if PRINT_ENABLED if (print_brace) diff --git a/bacnet-stack/src/bacapp.c b/bacnet-stack/src/bacapp.c index 7e32c0c1..0bfa5e80 100644 --- a/bacnet-stack/src/bacapp.c +++ b/bacnet-stack/src/bacapp.c @@ -801,19 +801,25 @@ int bacapp_data_len( bacapp_decode_application_data(&apdu[apdu_len], max_apdu_len - apdu_len, &application_value); } + if (len < 0) { + /* error: len indicates an error */ + total_len = len; + break; + } + apdu_len += len; if (opening_tag_number_counter) { if (len > 0) { total_len += len; } else { /* error: len is not incrementing */ - total_len = -1; + total_len = BACNET_STATUS_ERROR; break; } } if ((unsigned) apdu_len > max_apdu_len) { /* error: exceeding our buffer limit */ - total_len = -1; + total_len = BACNET_STATUS_ERROR; break; } }