diff --git a/bacnet-stack/demo/handler/h_whois.c b/bacnet-stack/demo/handler/h_whois.c index 1da8cf9e..f20df658 100644 --- a/bacnet-stack/demo/handler/h_whois.c +++ b/bacnet-stack/demo/handler/h_whois.c @@ -60,17 +60,19 @@ void handler_who_is( len = whois_decode_service_request(service_request, service_len, &low_limit, &high_limit); - if (len == 0) + if (len == 0) { Send_I_Am(&Handler_Transmit_Buffer[0]); - else if (len != -1) { + } else if (len != BACNET_STATUS_ERROR) { /* is my device id within the limits? */ + /* or */ + /* BACnet wildcard is the max instance number - everyone responds */ if (((Device_Object_Instance_Number() >= (uint32_t) low_limit) && (Device_Object_Instance_Number() <= (uint32_t) high_limit)) || - /* BACnet wildcard is the max instance number - everyone responds */ ((BACNET_MAX_INSTANCE >= (uint32_t) low_limit) && - (BACNET_MAX_INSTANCE <= (uint32_t) high_limit))) + (BACNET_MAX_INSTANCE <= (uint32_t) high_limit))) { Send_I_Am(&Handler_Transmit_Buffer[0]); + } } return; @@ -114,19 +116,19 @@ void handler_who_is_unicast( #ifdef BAC_ROUTING /* was for BAC_ROUTING - delete in 2/2012 if still unused */ - /* EKH: I restored this to BAC_ROUTING (from DEPRECATED) because I found that the server demo with the built-in + /* EKH: I restored this to BAC_ROUTING (from DEPRECATED) because I found that the server demo with the built-in virtual Router did not insert the SADRs of the virtual devices on the virtual network without it */ /** Local function to check Who-Is requests against our Device IDs. * Will check the gateway (root Device) and all virtual routed * Devices against the range and respond for each that matches. - * + * * @param service_request [in] The received message to be handled. * @param service_len [in] Length of the service_request message. * @param src [in] The BACNET_ADDRESS of the message's source. * @param is_unicast [in] True if should send unicast response(s) - * back to the src, else False if should broadcast response(s). + * back to the src, else False if should broadcast response(s). */ static void check_who_is_for_routing( uint8_t * service_request, @@ -174,12 +176,12 @@ static void check_who_is_for_routing( } -/** Handler for Who-Is requests in the virtual routing setup, +/** Handler for Who-Is requests in the virtual routing setup, * with broadcast I-Am response(s). * @ingroup DMDDB * Will check the gateway (root Device) and all virtual routed * Devices against the range and respond for each that matches. - * + * * @ingroup DMDDB * @param service_request [in] The received message to be handled. * @param service_len [in] Length of the service_request message. @@ -194,11 +196,11 @@ void handler_who_is_bcast_for_routing( } -/** Handler for Who-Is requests in the virtual routing setup, +/** Handler for Who-Is requests in the virtual routing setup, * with unicast I-Am response(s) returned to the src. * Will check the gateway (root Device) and all virtual routed * Devices against the range and respond for each that matches. - * + * * @ingroup DMDDB * @param service_request [in] The received message to be handled. * @param service_len [in] Length of the service_request message. diff --git a/bacnet-stack/src/whois.c b/bacnet-stack/src/whois.c index 12ad6aca..44213a69 100644 --- a/bacnet-stack/src/whois.c +++ b/bacnet-stack/src/whois.c @@ -81,22 +81,46 @@ int whois_decode_service_request( if (apdu_len) { len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); - if (tag_number != 0) - return -1; - len += decode_unsigned(&apdu[len], len_value, &decoded_value); - if (decoded_value <= BACNET_MAX_INSTANCE) { - if (pLow_limit) - *pLow_limit = decoded_value; + if (tag_number != 0) { + return BACNET_STATUS_ERROR; } - len += - decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); - if (tag_number != 1) - return -1; - len += decode_unsigned(&apdu[len], len_value, &decoded_value); - if (decoded_value <= BACNET_MAX_INSTANCE) { - if (pHigh_limit) - *pHigh_limit = decoded_value; + if (apdu_len > len) { + len += decode_unsigned(&apdu[len], len_value, &decoded_value); + if (decoded_value <= BACNET_MAX_INSTANCE) { + if (pLow_limit) { + *pLow_limit = decoded_value; + } + } + if (apdu_len > len) { + len += + decode_tag_number_and_value(&apdu[len], &tag_number, &len_value); + if (tag_number != 1) { + return BACNET_STATUS_ERROR; + } + if (apdu_len > len) { + len += decode_unsigned(&apdu[len], len_value, &decoded_value); + if (decoded_value <= BACNET_MAX_INSTANCE) { + if (pHigh_limit) { + *pHigh_limit = decoded_value; + } + } + } else { + return BACNET_STATUS_ERROR; + } + } else { + return BACNET_STATUS_ERROR; + } + } else { + return BACNET_STATUS_ERROR; } + } else { + if (pLow_limit) { + *pLow_limit = -1; + } + if (pHigh_limit) { + *pHigh_limit = -1; + } + len = 0; } return len; @@ -115,15 +139,18 @@ int whois_decode_apdu( { int len = 0; - if (!apdu) - return -1; - /* optional checking - most likely was already done prior to this call */ - if (apdu[0] != PDU_TYPE_UNCONFIRMED_SERVICE_REQUEST) - return -1; - if (apdu[1] != SERVICE_UNCONFIRMED_WHO_IS) - return -1; + if (!apdu) { + return BACNET_STATUS_ERROR; + } /* optional limits - must be used as a pair */ - if (apdu_len > 2) { + if (apdu_len >= 2) { + /* optional checking - most likely was already done prior to this call */ + if (apdu[0] != PDU_TYPE_UNCONFIRMED_SERVICE_REQUEST) { + return BACNET_STATUS_ERROR; + } + if (apdu[1] != SERVICE_UNCONFIRMED_WHO_IS) { + return BACNET_STATUS_ERROR; + } len = whois_decode_service_request(&apdu[2], apdu_len - 2, pLow_limit, pHigh_limit); @@ -140,35 +167,55 @@ void testWhoIs( int apdu_len = 0; int32_t low_limit = -1; int32_t high_limit = -1; - int32_t test_low_limit = -1; - int32_t test_high_limit = -1; + int32_t test_low_limit = 0; + int32_t test_high_limit = 0; + /* normal who-is without limits */ len = whois_encode_apdu(&apdu[0], low_limit, high_limit); - ct_test(pTest, len != 0); + ct_test(pTest, len > 0); apdu_len = len; len = whois_decode_apdu(&apdu[0], apdu_len, &test_low_limit, &test_high_limit); - ct_test(pTest, len != -1); + ct_test(pTest, len != BACNET_STATUS_ERROR); ct_test(pTest, test_low_limit == low_limit); ct_test(pTest, test_high_limit == high_limit); + /* normal who-is with limits - complete range */ for (low_limit = 0; low_limit <= BACNET_MAX_INSTANCE; low_limit += (BACNET_MAX_INSTANCE / 4)) { for (high_limit = 0; high_limit <= BACNET_MAX_INSTANCE; high_limit += (BACNET_MAX_INSTANCE / 4)) { len = whois_encode_apdu(&apdu[0], low_limit, high_limit); apdu_len = len; - ct_test(pTest, len != 0); + ct_test(pTest, len > 0); len = whois_decode_apdu(&apdu[0], apdu_len, &test_low_limit, &test_high_limit); - ct_test(pTest, len != -1); + ct_test(pTest, len != BACNET_STATUS_ERROR); ct_test(pTest, test_low_limit == low_limit); ct_test(pTest, test_high_limit == high_limit); } } + /* abnormal case: + who-is with no limits, but with APDU containing 2 limits */ + low_limit = 0; + high_limit = 0; + len = whois_encode_apdu(&apdu[0], low_limit, high_limit); + ct_test(pTest, len > 0); + apdu_len = len; + low_limit = -1; + high_limit = -1; + len = whois_encode_apdu(&apdu[0], low_limit, high_limit); + ct_test(pTest, len > 0); + apdu_len = len; + len = + whois_decode_apdu(&apdu[0], apdu_len, &test_low_limit, + &test_high_limit); + ct_test(pTest, len != BACNET_STATUS_ERROR); + ct_test(pTest, test_low_limit == low_limit); + ct_test(pTest, test_high_limit == high_limit); } #ifdef TEST_WHOIS