Secured the BACnet Who-Request decoder by changing deprecated decode functions. (#891)

src/bacnet/whois.c

@@ -12,7 +12,43 @@
 #include "bacnet/bacdcode.h"
 #include "bacnet/whois.h"
 
-/* encode I-Am service  - use -1 for limit if you want unlimited */
+/**
+ * @brief Encode a Who-Is-Request APDU
+ * @ingroup BIBB-DM-DDB
+ * @param apdu [out] Buffer in which the APDU contents are built, or NULL for
+ *  length determination
+ * @param low_limit [in] The low limit of the range of device instance numbers
+ * @param high_limit [in] The high limit of the range of device instance numbers
+ * @return The length of the apdu encoded (0 is a valid length)
+ */
+int whois_request_encode(uint8_t *apdu, int32_t low_limit, int32_t high_limit)
+{
+    int len = 0;
+    int apdu_len = 0;
+
+    if ((low_limit >= 0) && (low_limit <= BACNET_MAX_INSTANCE) &&
+        (high_limit >= 0) && (high_limit <= BACNET_MAX_INSTANCE)) {
+        len = encode_context_unsigned(apdu, 0, low_limit);
+        apdu_len += len;
+        if (apdu) {
+            apdu += len;
+        }
+        len = encode_context_unsigned(apdu, 1, high_limit);
+        apdu_len += len;
+    }
+
+    return apdu_len;
+}
+
+/**
+ * @brief Encode a Who-Is-Request unconfirmed service APDU
+ * @ingroup BIBB-DM-DDB
+ * @param apdu [out] Buffer in which the APDU contents are built, or NULL for
+ *  length determination
+ * @param low_limit [in] The low limit of the range of device instance numbers
+ * @param high_limit [in] The high limit of the range of device instance numbers
+ * @return The length of the apdu encoded (0 is a valid length)
+ */
 int whois_encode_apdu(uint8_t *apdu, int32_t low_limit, int32_t high_limit)
 {
     int len = 0; /* length of each encoding */
@@ -21,69 +57,67 @@ int whois_encode_apdu(uint8_t *apdu, int32_t low_limit, int32_t high_limit)
     if (apdu) {
         apdu[0] = PDU_TYPE_UNCONFIRMED_SERVICE_REQUEST;
         apdu[1] = SERVICE_UNCONFIRMED_WHO_IS; /* service choice */
-        apdu_len = 2;
-        /* optional limits - must be used as a pair */
-        if ((low_limit >= 0) && (low_limit <= BACNET_MAX_INSTANCE) &&
-            (high_limit >= 0) && (high_limit <= BACNET_MAX_INSTANCE)) {
-            len = encode_context_unsigned(&apdu[apdu_len], 0, low_limit);
-            apdu_len += len;
-            len = encode_context_unsigned(&apdu[apdu_len], 1, high_limit);
-            apdu_len += len;
-        }
     }
+    len = 2;
+    apdu_len += len;
+    if (apdu) {
+        apdu += len;
+    }
+    len = whois_request_encode(apdu, low_limit, high_limit);
+    apdu_len += len;
 
     return apdu_len;
 }
 
-/* decode the service request only */
+/**
+ * @brief Decode a Who-Is-Request APDU
+ * @ingroup BIBB-DM-DDB
+ * @param apdu [in] Buffer containing the APDU
+ * @param apdu_size [in] The length of the APDU
+ * @param pLow_limit [out] The low limit of the range of device instance numbers
+ * @param pHigh_limit [out] The high limit of the range of device instance
+ * numbers
+ * @return The number of bytes decoded , or #BACNET_STATUS_ERROR on error
+ */
 int whois_decode_service_request(
     const uint8_t *apdu,
-    unsigned apdu_len,
+    unsigned apdu_size,
     int32_t *pLow_limit,
     int32_t *pHigh_limit)
 {
-    unsigned int len = 0;
+    int len = 0, apdu_len = 0;
-    uint8_t tag_number = 0;
-    uint32_t len_value = 0;
     BACNET_UNSIGNED_INTEGER unsigned_value = 0;
 
-    /* optional limits - must be used as a pair */
+    /* optional limits - used as a pair */
-    if (apdu_len) {
+    if (apdu && (apdu_size > 0)) {
-        len += decode_tag_number_and_value(&apdu[len], &tag_number, &len_value);
+        len = bacnet_unsigned_context_decode(
-        if (tag_number != 0) {
+            &apdu[apdu_len], apdu_size - apdu_len, 0, &unsigned_value);
-            return BACNET_STATUS_ERROR;
+        if (len > 0) {
-        }
-        if (apdu_len > (unsigned)len) {
-            len += decode_unsigned(&apdu[len], len_value, &unsigned_value);
             if (unsigned_value <= BACNET_MAX_INSTANCE) {
                 if (pLow_limit) {
                     *pLow_limit = (int32_t)unsigned_value;
                 }
-            }
-            if (apdu_len > (unsigned)len) {
-                len += decode_tag_number_and_value(
-                    &apdu[len], &tag_number, &len_value);
-                if (tag_number != 1) {
-                    return BACNET_STATUS_ERROR;
-                }
-                if (apdu_len > (unsigned)len) {
-                    len +=
-                        decode_unsigned(&apdu[len], len_value, &unsigned_value);
-                    if (unsigned_value <= BACNET_MAX_INSTANCE) {
-                        if (pHigh_limit) {
-                            *pHigh_limit = (int32_t)unsigned_value;
-                            ;
-                        }
-                    }
-                } else {
-                    return BACNET_STATUS_ERROR;
-                }
             } else {
                 return BACNET_STATUS_ERROR;
             }
         } else {
             return BACNET_STATUS_ERROR;
         }
+        apdu_len += len;
+        len = bacnet_unsigned_context_decode(
+            &apdu[apdu_len], apdu_size - apdu_len, 1, &unsigned_value);
+        if (len > 0) {
+            if (unsigned_value <= BACNET_MAX_INSTANCE) {
+                if (pHigh_limit) {
+                    *pHigh_limit = (int32_t)unsigned_value;
+                }
+            } else {
+                return BACNET_STATUS_ERROR;
+            }
+        } else {
+            return BACNET_STATUS_ERROR;
+        }
+        apdu_len += len;
     } else {
         if (pLow_limit) {
             *pLow_limit = -1;
@@ -91,8 +125,7 @@ int whois_decode_service_request(
         if (pHigh_limit) {
             *pHigh_limit = -1;
         }
-        len = 0;
     }
 
-    return (int)len;
+    return apdu_len;
 }