Secured SubscribeCOVProperty decoder. (#892)

* Secured SubscribeCOVProperty decoder.

* Changed datatype of monitoredProperty in struct BACnet_Subscribe_COV_Data

src/bacnet/bacdevobjpropref.c

@@ -919,6 +919,21 @@ bool bacnet_property_reference_same(
     return status;
 }
 
+/**
+ * @brief Copy the complex data of src to dest
+ * @param dest - destination structure
+ * @param src - source structure
+ */
+void bacnet_property_reference_copy(
+    struct BACnetPropertyReference *dest,
+    const struct BACnetPropertyReference *src)
+{
+    if (dest && src) {
+        dest->property_identifier = src->property_identifier;
+        dest->property_array_index = src->property_array_index;
+    }
+}
+
 /**
  * @brief Decode a BACnetPropertyReference from a buffer
  * @param apdu - the APDU buffer

src/bacnet/bacdevobjpropref.h

@@ -187,6 +187,10 @@ bool bacnet_property_reference_same(
     const struct BACnetPropertyReference *value1,
     const struct BACnetPropertyReference *value2);
 BACNET_STACK_EXPORT
+void bacnet_property_reference_copy(
+    struct BACnetPropertyReference *dest,
+    const struct BACnetPropertyReference *src);
+BACNET_STACK_EXPORT
 int bacnet_property_reference_decode(
     const uint8_t *apdu,
     uint32_t apdu_size,

src/bacnet/cov.c

@@ -335,6 +335,9 @@ int cov_subscribe_apdu_encode(
     int len = 0; /* length of each encoding */
     int apdu_len = 0; /* total length of the apdu, return value */
 
+    if (!data) {
+        return 0;
+    }
     /* tag 0 - subscriberProcessIdentifier */
     len = encode_context_unsigned(apdu, 0, data->subscriberProcessIdentifier);
     apdu_len += len;
@@ -448,7 +451,7 @@ int cov_subscribe_encode_apdu(
  * @param apdu_size  number 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.
+ * @return Bytes decoded or BACNET_STATUS_ERROR on error.
  */
 int cov_subscribe_decode_service_request(
     const uint8_t *apdu, unsigned apdu_size, BACNET_SUBSCRIBE_COV_DATA *data)
@@ -460,6 +463,12 @@ int cov_subscribe_decode_service_request(
     uint32_t decoded_instance = 0;
     bool decoded_boolean = false;
 
+    if (!apdu) {
+        return BACNET_STATUS_REJECT;
+    }
+    if (apdu_size == 0) {
+        return BACNET_STATUS_REJECT;
+    }
     /* subscriberProcessIdentifier [0] Unsigned32 */
     value_len = bacnet_unsigned_context_decode(
         &apdu[len], apdu_size - len, 0, &decoded_value);
@@ -609,26 +618,8 @@ int cov_subscribe_property_apdu_encode(
         }
     }
     /* tag 4 - monitoredPropertyIdentifier */
-    len = encode_opening_tag(apdu, 4);
+    len = bacnet_property_reference_context_encode(
-    apdu_len += len;
+        apdu, 4, &data->monitoredProperty);
-    if (apdu) {
-        apdu += len;
-    }
-    len = encode_context_enumerated(
-        apdu, 0, data->monitoredProperty.propertyIdentifier);
-    apdu_len += len;
-    if (apdu) {
-        apdu += len;
-    }
-    if (data->monitoredProperty.propertyArrayIndex != BACNET_ARRAY_ALL) {
-        len = encode_context_unsigned(
-            apdu, 1, data->monitoredProperty.propertyArrayIndex);
-        apdu_len += len;
-        if (apdu) {
-            apdu += len;
-        }
-    }
-    len = encode_closing_tag(apdu, 4);
     apdu_len += len;
     if (apdu) {
         apdu += len;
@@ -709,133 +700,140 @@ int cov_subscribe_property_encode_apdu(
 /**
  * Decode the COV-service property request only.
  *
+ * SubscribeCOVProperty-Request ::= SEQUENCE {
+ *       subscriberProcessIdentifier  [0] Unsigned32,
+ *       monitoredObjectIdentifier    [1] BACnetObjectIdentifier,
+ *       issueConfirmedNotifications  [2] BOOLEAN OPTIONAL,
+ *       lifetime                     [3] Unsigned OPTIONAL,
+ *       monitoredPropertyIdentifier  [4] BACnetPropertyReference,
+ *       covIncrement                 [5] REAL OPTIONAL
+ *       }
+ *
+ * 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 the buffer.
- * @param apdu_len  Count of valid bytes in the buffer.
+ * @param apdu_size  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.
+ * @return Bytes decoded or BACNET_STATUS_REJECT on error.
  */
 int cov_subscribe_property_decode_service_request(
-    const uint8_t *apdu, unsigned apdu_len, BACNET_SUBSCRIBE_COV_DATA *data)
+    const uint8_t *apdu, unsigned apdu_size, BACNET_SUBSCRIBE_COV_DATA *data)
 {
-    int len = 0; /* return value */
+    int len = 0, apdu_len = 0;
-    uint8_t tag_number = 0;
+    BACNET_UNSIGNED_INTEGER decoded_unsigned = 0; /* for decoding */
-    uint32_t len_value = 0;
-    BACNET_UNSIGNED_INTEGER decoded_value = 0; /* for decoding */
     BACNET_OBJECT_TYPE decoded_type = OBJECT_NONE; /* for decoding */
-    uint32_t property = 0; /* for decoding */
+    uint32_t decoded_instance = 0; /* for decoding */
+    bool decoded_boolean = false; /* for decoding */
+    struct BACnetPropertyReference decoded_reference = { 0 };
+    float decoded_real = 0.0f; /* for decoding */
 
-    if ((apdu_len > 2) && data) {
+    if (!apdu) {
-        /* tag 0 - subscriberProcessIdentifier */
+        return BACNET_STATUS_REJECT;
-        if (decode_is_context_tag(&apdu[len], 0)) {
+    }
-            len += decode_tag_number_and_value(
+    if (apdu_size == 0) {
-                &apdu[len], &tag_number, &len_value);
+        return BACNET_STATUS_REJECT;
-            len += decode_unsigned(&apdu[len], len_value, &decoded_value);
+    }
-            data->subscriberProcessIdentifier = decoded_value;
+    /* subscriberProcessIdentifier [0] Unsigned32 */
-        } else {
+    len = bacnet_unsigned_context_decode(
+        &apdu[apdu_len], apdu_size - apdu_len, 0, &decoded_unsigned);
+    if (len > 0) {
+        if (decoded_unsigned > UINT32_MAX) {
             data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
             return BACNET_STATUS_REJECT;
         }
-        /* tag 1 - monitoredObjectIdentifier */
+        if (data) {
-        if (len >= (int)apdu_len) {
+            data->subscriberProcessIdentifier = decoded_unsigned;
-            return BACNET_STATUS_REJECT;
         }
-        if (decode_is_context_tag(&apdu[len], 1)) {
+        apdu_len += len;
-            len += decode_tag_number_and_value(
+    } else {
-                &apdu[len], &tag_number, &len_value);
+        data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
-            len += decode_object_id(
+        return BACNET_STATUS_REJECT;
-                &apdu[len], &decoded_type,
+    }
-                &data->monitoredObjectIdentifier.instance);
+    /* monitoredObjectIdentifier [1] BACnetObjectIdentifier */
+    len = bacnet_object_id_context_decode(
+        &apdu[apdu_len], apdu_size - apdu_len, 1, &decoded_type,
+        &decoded_instance);
+    if (len > 0) {
+        if (data) {
             data->monitoredObjectIdentifier.type = decoded_type;
-        } else {
+            data->monitoredObjectIdentifier.instance = decoded_instance;
+        }
+        apdu_len += len;
+    } else {
+        if (data) {
             data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
-            return BACNET_STATUS_REJECT;
         }
-        /* tag 2 - issueConfirmedNotifications - optional */
+        return BACNET_STATUS_REJECT;
-        if (len >= (int)apdu_len) {
+    }
-            return BACNET_STATUS_REJECT;
+    /* issueConfirmedNotifications  [2] BOOLEAN OPTIONAL */
-        }
+    len = bacnet_boolean_context_decode(
-        if (decode_is_context_tag(&apdu[len], 2)) {
+        &apdu[apdu_len], apdu_size - apdu_len, 2, &decoded_boolean);
+    if (len > 0) {
+        if (data) {
             data->cancellationRequest = false;
-            len += decode_tag_number_and_value(
+            data->issueConfirmedNotifications = decoded_boolean;
-                &apdu[len], &tag_number, &len_value);
+        }
-            data->issueConfirmedNotifications =
+        apdu_len += len;
-                decode_context_boolean(&apdu[len]);
+    } else {
-            len++;
+        /* skip - optional */
-        } else {
+        if (data) {
+            data->issueConfirmedNotifications = false;
             data->cancellationRequest = true;
         }
-        /* tag 3 - lifetime - optional */
+    }
-        if (len >= (int)apdu_len) {
+    /* lifetime [3] Unsigned OPTIONAL */
-            return BACNET_STATUS_REJECT;
+    len = bacnet_unsigned_context_decode(
+        &apdu[apdu_len], apdu_size - apdu_len, 3, &decoded_unsigned);
+    if (len > 0) {
+        if (data) {
+            data->lifetime = decoded_unsigned;
         }
-        if (decode_is_context_tag(&apdu[len], 3)) {
+        apdu_len += len;
-            len += decode_tag_number_and_value(
+    } else {
-                &apdu[len], &tag_number, &len_value);
+        /* skip - optional */
-            len += decode_unsigned(&apdu[len], len_value, &decoded_value);
+        if (data) {
-            data->lifetime = decoded_value;
-        } else {
             data->lifetime = 0;
         }
-        /* tag 4 - monitoredPropertyIdentifier */
+    }
-        if (len >= (int)apdu_len) {
+    /* monitoredPropertyIdentifier [4] BACnetPropertyReference */
-            return BACNET_STATUS_REJECT;
+    len = bacnet_property_reference_context_decode(
+        &apdu[apdu_len], apdu_size - apdu_len, 4, &decoded_reference);
+    if (len > 0) {
+        if (data) {
+            bacnet_property_reference_copy(
+                &data->monitoredProperty, &decoded_reference);
         }
-        if (!decode_is_opening_tag_number(&apdu[len], 4)) {
+        apdu_len += len;
+    } else {
+        if (data) {
             data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
-            return BACNET_STATUS_REJECT;
         }
-        /* a tag number of 4 is not extended so only one octet */
+        return BACNET_STATUS_REJECT;
-        len++;
+    }
-        /* the propertyIdentifier is tag 0 */
+    /* covIncrement [5] REAL OPTIONAL */
-        if (len >= (int)apdu_len) {
+    len = bacnet_real_context_decode(
-            return BACNET_STATUS_REJECT;
+        &apdu[apdu_len], apdu_size - apdu_len, 5, &decoded_real);
+    if (len > 0) {
+        if (data) {
+            data->covIncrement = decoded_real;
+            data->covIncrementPresent = true;
         }
-        if (decode_is_context_tag(&apdu[len], 0)) {
+        apdu_len += len;
-            len += decode_tag_number_and_value(
+    } else {
-                &apdu[len], &tag_number, &len_value);
+        /* skip - optional */
-            len += decode_enumerated(&apdu[len], len_value, &property);
+        if (data) {
-            data->monitoredProperty.propertyIdentifier =
+            data->covIncrement = 0.0f;
-                (BACNET_PROPERTY_ID)property;
-        } else {
-            data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
-            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);
-            len += decode_unsigned(&apdu[len], len_value, &decoded_value);
-            data->monitoredProperty.propertyArrayIndex = decoded_value;
-        } else {
-            data->monitoredProperty.propertyArrayIndex = BACNET_ARRAY_ALL;
-        }
-
-        if (!decode_is_closing_tag_number(&apdu[len], 4)) {
-            data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
-            return BACNET_STATUS_REJECT;
-        }
-        /* a tag number of 4 is not extended so only one octet */
-        len++;
-        /* tag 5 - covIncrement - optional */
-        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;
         }
     }
 
-    return len;
+    return apdu_len;
 }
 
 /**

src/bacnet/cov.h

@@ -32,7 +32,7 @@ typedef struct BACnet_Subscribe_COV_Data {
     bool issueConfirmedNotifications; /* optional */
     uint32_t lifetime; /* seconds, optional */
     bool covSubscribeToProperty; /* true to use per-property subscription */
-    BACNET_PROPERTY_REFERENCE monitoredProperty;
+    struct BACnetPropertyReference monitoredProperty;
     bool covIncrementPresent; /* true if present */
     float covIncrement; /* optional */
     BACNET_ERROR_CLASS error_class;

test/bacnet/cov/src/main.c

@@ -22,124 +22,151 @@
  */
 int ccov_notify_decode_apdu(
     const uint8_t *apdu,
-    unsigned apdu_len,
+    unsigned apdu_size,
     uint8_t *invoke_id,
     BACNET_COV_DATA *data)
 {
     int len = 0;
-    unsigned offset = 0;
+    unsigned apdu_len = 0;
 
-    if (!apdu) {
+    if (apdu && (apdu_size >= 4)) {
-        return -1;
+        if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) {
+            return -2;
+        }
+        /*  apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
+        *invoke_id = apdu[2]; /* invoke id - filled in by net layer */
+        if (apdu[3] != SERVICE_CONFIRMED_COV_NOTIFICATION) {
+            return -3;
+        }
     }
-    /* optional checking - most likely was already done prior to this call */
+    apdu_len = 4;
-    if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) {
+    /* optional limits - must be used as a pair - 3 test cases */
-        return -2;
+    if (apdu) {
-    }
+        if (apdu_size > apdu_len) {
-    /*  apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
+            len = cov_notify_decode_service_request(
-    *invoke_id = apdu[2]; /* invoke id - filled in by net layer */
+                &apdu[apdu_len], apdu_size - apdu_len, data);
-    if (apdu[3] != SERVICE_CONFIRMED_COV_NOTIFICATION) {
+            apdu_len += len;
-        return -3;
+        } else {
-    }
+            apdu_len = cov_notify_decode_service_request(apdu, 0, data);
-    offset = 4;
+        }
-
+    } else {
-    /* optional limits - must be used as a pair */
+        apdu_len = cov_notify_decode_service_request(NULL, apdu_size, data);
-    if (apdu_len > offset) {
-        len = cov_notify_decode_service_request(
-            &apdu[offset], apdu_len - offset, data);
     }
 
-    return len;
+    return apdu_len;
 }
 
 int ucov_notify_decode_apdu(
-    const uint8_t *apdu, unsigned apdu_len, BACNET_COV_DATA *data)
+    const uint8_t *apdu, unsigned apdu_size, BACNET_COV_DATA *data)
 {
     int len = 0;
-    unsigned offset = 0;
+    unsigned apdu_len = 0;
 
-    if (!apdu) {
+    if (apdu && (apdu_size >= 2)) {
-        return -1;
+        /* optional checking - most likely was already done prior to this call
-    }
+         */
-    /* optional checking - most likely was already done prior to this call */
+        if (apdu[0] != PDU_TYPE_UNCONFIRMED_SERVICE_REQUEST) {
-    if (apdu[0] != PDU_TYPE_UNCONFIRMED_SERVICE_REQUEST) {
+            return -2;
-        return -2;
+        }
-    }
+        if (apdu[1] != SERVICE_UNCONFIRMED_COV_NOTIFICATION) {
-    if (apdu[1] != SERVICE_UNCONFIRMED_COV_NOTIFICATION) {
+            return -3;
-        return -3;
+        }
     }
     /* optional limits - must be used as a pair */
-    offset = 2;
+    apdu_len = 2;
-    if (apdu_len > offset) {
+    if (apdu) {
-        len = cov_notify_decode_service_request(
+        if (apdu_size > apdu_len) {
-            &apdu[offset], apdu_len - offset, data);
+            len = cov_notify_decode_service_request(
+                &apdu[apdu_len], apdu_size - apdu_len, data);
+            apdu_len += len;
+        } else {
+            apdu_len = cov_notify_decode_service_request(apdu, 0, data);
+        }
+    } else {
+        apdu_len = cov_notify_decode_service_request(NULL, apdu_size, data);
     }
 
-    return len;
+    return apdu_len;
 }
 
 static int cov_subscribe_decode_apdu(
     const uint8_t *apdu,
-    unsigned apdu_len,
+    unsigned apdu_size,
     uint8_t *invoke_id,
     BACNET_SUBSCRIBE_COV_DATA *data)
 {
     int len = 0;
-    unsigned offset = 0;
+    unsigned apdu_len = 0;
 
-    if (!apdu) {
+    if (apdu && (apdu_size >= 4)) {
-        return -1;
+        /* optional checking - most likely was already done prior to this call
+         */
+        if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) {
+            return -2;
+        }
+        /*  apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
+        *invoke_id = apdu[2]; /* invoke id - filled in by net layer */
+        if (apdu[3] != SERVICE_CONFIRMED_SUBSCRIBE_COV) {
+            return -3;
+        }
     }
-    /* optional checking - most likely was already done prior to this call */
+    apdu_len = 4;
-    if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) {
-        return -2;
-    }
-    /*  apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
-    *invoke_id = apdu[2]; /* invoke id - filled in by net layer */
-    if (apdu[3] != SERVICE_CONFIRMED_SUBSCRIBE_COV) {
-        return -3;
-    }
-    offset = 4;
 
     /* optional limits - must be used as a pair */
-    if (apdu_len > offset) {
+    if (apdu) {
-        len = cov_subscribe_decode_service_request(
+        if (apdu_size > apdu_len) {
-            &apdu[offset], apdu_len - offset, data);
+            len = cov_subscribe_decode_service_request(
+                &apdu[apdu_len], apdu_size - apdu_len, data);
+            apdu_len += len;
+        } else {
+            apdu_len = cov_subscribe_decode_service_request(apdu, 0, data);
+        }
+    } else {
+        apdu_len = cov_subscribe_decode_service_request(NULL, apdu_size, data);
     }
 
-    return len;
+    return apdu_len;
 }
 
 static int cov_subscribe_property_decode_apdu(
     const uint8_t *apdu,
-    unsigned apdu_len,
+    unsigned apdu_size,
     uint8_t *invoke_id,
     BACNET_SUBSCRIBE_COV_DATA *data)
 {
     int len = 0;
-    unsigned offset = 0;
+    unsigned apdu_len = 0;
 
-    if (!apdu) {
+    if (apdu && (apdu_size >= 4)) {
-        return -1;
+        /* optional checking - most likely was already done prior to this call
+         */
+        if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) {
+            return -2;
+        }
+        /*  apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
+        *invoke_id = apdu[2]; /* invoke id - filled in by net layer */
+        if (apdu[3] != SERVICE_CONFIRMED_SUBSCRIBE_COV_PROPERTY) {
+            return -3;
+        }
     }
-    /* optional checking - most likely was already done prior to this call */
+    apdu_len = 4;
-    if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) {
-        return -2;
-    }
-    /*  apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */
-    *invoke_id = apdu[2]; /* invoke id - filled in by net layer */
-    if (apdu[3] != SERVICE_CONFIRMED_SUBSCRIBE_COV_PROPERTY) {
-        return -3;
-    }
-    offset = 4;
 
     /* optional limits - must be used as a pair */
-    if (apdu_len > offset) {
+    if (apdu) {
-        len = cov_subscribe_property_decode_service_request(
+        if (apdu_size > apdu_len) {
-            &apdu[offset], apdu_len - offset, data);
+            len = cov_subscribe_property_decode_service_request(
+                &apdu[apdu_len], apdu_size - apdu_len, data);
+            apdu_len += len;
+        } else {
+            apdu_len =
+                cov_subscribe_property_decode_service_request(apdu, 0, data);
+        }
+    } else {
+        apdu_len = cov_subscribe_property_decode_service_request(
+            NULL, apdu_size, data);
     }
 
-    return len;
+    return apdu_len;
 }
 
 /* dummy function stubs */
@@ -200,6 +227,12 @@ static void testUCOVNotifyData(const BACNET_COV_DATA *data)
 
     cov_data_value_list_link(
         &test_data, &value_list[0], ARRAY_SIZE(value_list));
+
+    null_len = ucov_notify_decode_apdu(NULL, apdu_len, &test_data);
+    zassert_true(null_len < 0, NULL);
+    null_len = ucov_notify_decode_apdu(&apdu[0], 0, &test_data);
+    zassert_true(null_len < 0, NULL);
+
     len = ucov_notify_decode_apdu(&apdu[0], apdu_len, &test_data);
     zassert_not_equal(len, -1, NULL);
     testCOVNotifyData(data, &test_data);
@@ -220,6 +253,14 @@ static void testCCOVNotifyData(uint8_t invoke_id, const BACNET_COV_DATA *data)
     apdu_len = len;
 
     cov_data_value_list_link(&test_data, &value_list[0], 2);
+
+    null_len =
+        ccov_notify_decode_apdu(NULL, apdu_len, &test_invoke_id, &test_data);
+    zassert_true(null_len < 0, NULL);
+    null_len =
+        ccov_notify_decode_apdu(&apdu[0], 0, &test_invoke_id, &test_data);
+    zassert_true(null_len < 0, NULL);
+
     len = ccov_notify_decode_apdu(
         &apdu[0], apdu_len, &test_invoke_id, &test_data);
     zassert_true(len > 0, NULL);
@@ -291,13 +332,11 @@ static void testCOVSubscribePropertyData(
     const BACNET_SUBSCRIBE_COV_DATA *data,
     const BACNET_SUBSCRIBE_COV_DATA *test_data)
 {
+    bool status = false;
     testCOVSubscribeData(data, test_data);
-    zassert_equal(
+    status = bacnet_property_reference_same(
-        test_data->monitoredProperty.propertyIdentifier,
+        &test_data->monitoredProperty, &data->monitoredProperty);
-        data->monitoredProperty.propertyIdentifier, NULL);
+    zassert_true(status, NULL);
-    zassert_equal(
-        test_data->monitoredProperty.propertyArrayIndex,
-        data->monitoredProperty.propertyArrayIndex, NULL);
     zassert_equal(
         test_data->covIncrementPresent, data->covIncrementPresent, NULL);
     if (test_data->covIncrementPresent) {
@@ -310,15 +349,22 @@ static void testCOVSubscribeEncoding(
     uint8_t invoke_id, const BACNET_SUBSCRIBE_COV_DATA *data)
 {
     uint8_t apdu[480] = { 0 };
-    int len = 0;
+    int len = 0, null_len = 0, apdu_len = 0;
-    int apdu_len = 0;
     BACNET_SUBSCRIBE_COV_DATA test_data = { 0 };
     uint8_t test_invoke_id = 0;
 
+    null_len = cov_subscribe_encode_apdu(NULL, sizeof(apdu), invoke_id, data);
     len = cov_subscribe_encode_apdu(&apdu[0], sizeof(apdu), invoke_id, data);
     zassert_not_equal(len, 0, NULL);
+    zassert_equal(null_len, len, NULL);
     apdu_len = len;
 
+    null_len = cov_subscribe_decode_apdu(apdu, 0, &test_invoke_id, &test_data);
+    zassert_true(null_len < 0, NULL);
+    null_len =
+        cov_subscribe_decode_apdu(NULL, apdu_len, &test_invoke_id, &test_data);
+    zassert_true(null_len < 0, NULL);
+
     len = cov_subscribe_decode_apdu(
         &apdu[0], apdu_len, &test_invoke_id, &test_data);
     zassert_true(len > 0, NULL);
@@ -330,16 +376,25 @@ static void testCOVSubscribePropertyEncoding(
     uint8_t invoke_id, const BACNET_SUBSCRIBE_COV_DATA *data)
 {
     uint8_t apdu[480] = { 0 };
-    int len = 0;
+    int len = 0, null_len = 0, apdu_len = 0;
-    int apdu_len = 0;
     BACNET_SUBSCRIBE_COV_DATA test_data;
     uint8_t test_invoke_id = 0;
 
+    null_len =
+        cov_subscribe_property_encode_apdu(NULL, sizeof(apdu), invoke_id, data);
     len = cov_subscribe_property_encode_apdu(
         &apdu[0], sizeof(apdu), invoke_id, data);
     zassert_not_equal(len, 0, NULL);
+    zassert_equal(null_len, len, NULL);
     apdu_len = len;
 
+    null_len = cov_subscribe_property_decode_apdu(
+        apdu, 0, &test_invoke_id, &test_data);
+    zassert_true(null_len < 0, NULL);
+    null_len = cov_subscribe_property_decode_apdu(
+        NULL, apdu_len, &test_invoke_id, &test_data);
+    zassert_true(null_len < 0, NULL);
+
     len = cov_subscribe_property_decode_apdu(
         &apdu[0], apdu_len, &test_invoke_id, &test_data);
     zassert_true(len > 0, NULL);
@@ -383,8 +438,8 @@ static void testCOVSubscribeProperty(void)
     data.cancellationRequest = false;
     data.issueConfirmedNotifications = true;
     data.lifetime = 456;
-    data.monitoredProperty.propertyIdentifier = PROP_FILE_SIZE;
+    data.monitoredProperty.property_identifier = PROP_FILE_SIZE;
-    data.monitoredProperty.propertyArrayIndex = BACNET_ARRAY_ALL;
+    data.monitoredProperty.property_array_index = BACNET_ARRAY_ALL;
     data.covIncrementPresent = true;
     data.covIncrement = 1.0;
 
@@ -397,6 +452,150 @@ static void testCOVSubscribeProperty(void)
     data.covIncrementPresent = false;
     testCOVSubscribePropertyEncoding(invoke_id, &data);
 }
+
+#if defined(CONFIG_ZTEST_NEW_API)
+ZTEST(cov_tests, test_COV_Value_List_Encode)
+#else
+static void test_COV_Value_List_Encode(void)
+#endif
+{
+    BACNET_PROPERTY_VALUE value_list[2] = { { 0 } };
+    bool status;
+    BACNET_CHARACTER_STRING char_string = { 0 };
+    BACNET_BIT_STRING bit_string = { 0 };
+
+    cov_property_value_list_link(&value_list[0], ARRAY_SIZE(value_list));
+    /* REAL */
+    status = cov_value_list_encode_real(NULL, 0.0f, false, false, false, false);
+    zassert_false(status, NULL);
+    status = cov_value_list_encode_real(
+        &value_list[0], 21.0f, false, false, false, false);
+    zassert_true(status, NULL);
+    zassert_equal(value_list[0].propertyIdentifier, PROP_PRESENT_VALUE, NULL);
+    zassert_equal(value_list[0].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[0].value.context_specific, false, NULL);
+    zassert_equal(value_list[0].value.tag, BACNET_APPLICATION_TAG_REAL, NULL);
+    zassert_equal(value_list[0].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_true(value_list[0].next != NULL, NULL);
+    zassert_equal(value_list[1].propertyIdentifier, PROP_STATUS_FLAGS, NULL);
+    zassert_equal(value_list[1].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[1].value.context_specific, false, NULL);
+    zassert_equal(
+        value_list[1].value.tag, BACNET_APPLICATION_TAG_BIT_STRING, NULL);
+    zassert_equal(value_list[1].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_equal(value_list[1].next, NULL, NULL);
+
+    /* ENUMERATED */
+    status =
+        cov_value_list_encode_enumerated(NULL, 0, false, false, false, false);
+    zassert_false(status, NULL);
+    status = cov_value_list_encode_enumerated(
+        &value_list[0], 21, false, false, false, false);
+    zassert_true(status, NULL);
+    zassert_equal(value_list[0].propertyIdentifier, PROP_PRESENT_VALUE, NULL);
+    zassert_equal(value_list[0].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[0].value.context_specific, false, NULL);
+    zassert_equal(
+        value_list[0].value.tag, BACNET_APPLICATION_TAG_ENUMERATED, NULL);
+    zassert_equal(value_list[0].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_true(value_list[0].next != NULL, NULL);
+    zassert_equal(value_list[1].propertyIdentifier, PROP_STATUS_FLAGS, NULL);
+    zassert_equal(value_list[1].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[1].value.context_specific, false, NULL);
+    zassert_equal(
+        value_list[1].value.tag, BACNET_APPLICATION_TAG_BIT_STRING, NULL);
+    zassert_equal(value_list[1].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_equal(value_list[1].next, NULL, NULL);
+
+    /* UNSIGNED */
+    status =
+        cov_value_list_encode_unsigned(NULL, 0, false, false, false, false);
+    zassert_false(status, NULL);
+    status = cov_value_list_encode_unsigned(
+        &value_list[0], 21, false, false, false, false);
+    zassert_true(status, NULL);
+    zassert_equal(value_list[0].propertyIdentifier, PROP_PRESENT_VALUE, NULL);
+    zassert_equal(value_list[0].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[0].value.context_specific, false, NULL);
+    zassert_equal(
+        value_list[0].value.tag, BACNET_APPLICATION_TAG_UNSIGNED_INT, NULL);
+    zassert_equal(value_list[0].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_true(value_list[0].next != NULL, NULL);
+    zassert_equal(value_list[1].propertyIdentifier, PROP_STATUS_FLAGS, NULL);
+    zassert_equal(value_list[1].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[1].value.context_specific, false, NULL);
+    zassert_equal(
+        value_list[1].value.tag, BACNET_APPLICATION_TAG_BIT_STRING, NULL);
+    zassert_equal(value_list[1].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_equal(value_list[1].next, NULL, NULL);
+
+    /* SIGNED */
+    status =
+        cov_value_list_encode_signed_int(NULL, 0, false, false, false, false);
+    zassert_false(status, NULL);
+    status = cov_value_list_encode_signed_int(
+        &value_list[0], 21, false, false, false, false);
+    zassert_true(status, NULL);
+    zassert_equal(value_list[0].propertyIdentifier, PROP_PRESENT_VALUE, NULL);
+    zassert_equal(value_list[0].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[0].value.context_specific, false, NULL);
+    zassert_equal(
+        value_list[0].value.tag, BACNET_APPLICATION_TAG_SIGNED_INT, NULL);
+    zassert_equal(value_list[0].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_true(value_list[0].next != NULL, NULL);
+    zassert_equal(value_list[1].propertyIdentifier, PROP_STATUS_FLAGS, NULL);
+    zassert_equal(value_list[1].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[1].value.context_specific, false, NULL);
+    zassert_equal(
+        value_list[1].value.tag, BACNET_APPLICATION_TAG_BIT_STRING, NULL);
+    zassert_equal(value_list[1].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_equal(value_list[1].next, NULL, NULL);
+
+    /* CHARACTERSTRING */
+    status = cov_value_list_encode_character_string(
+        NULL, &char_string, false, false, false, false);
+    zassert_false(status, NULL);
+    status = cov_value_list_encode_character_string(
+        &value_list[0], &char_string, false, false, false, false);
+    zassert_true(status, NULL);
+    zassert_equal(value_list[0].propertyIdentifier, PROP_PRESENT_VALUE, NULL);
+    zassert_equal(value_list[0].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[0].value.context_specific, false, NULL);
+    zassert_equal(
+        value_list[0].value.tag, BACNET_APPLICATION_TAG_CHARACTER_STRING, NULL);
+    zassert_equal(value_list[0].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_true(value_list[0].next != NULL, NULL);
+    zassert_equal(value_list[1].propertyIdentifier, PROP_STATUS_FLAGS, NULL);
+    zassert_equal(value_list[1].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[1].value.context_specific, false, NULL);
+    zassert_equal(
+        value_list[1].value.tag, BACNET_APPLICATION_TAG_BIT_STRING, NULL);
+    zassert_equal(value_list[1].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_equal(value_list[1].next, NULL, NULL);
+
+    /* CHARACTERSTRING */
+    status = cov_value_list_encode_bit_string(
+        NULL, &bit_string, false, false, false, false);
+    zassert_false(status, NULL);
+    status = cov_value_list_encode_bit_string(
+        &value_list[0], &bit_string, false, false, false, false);
+    zassert_true(status, NULL);
+    zassert_equal(value_list[0].propertyIdentifier, PROP_PRESENT_VALUE, NULL);
+    zassert_equal(value_list[0].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[0].value.context_specific, false, NULL);
+    zassert_equal(
+        value_list[0].value.tag, BACNET_APPLICATION_TAG_BIT_STRING, NULL);
+    zassert_equal(value_list[0].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_true(value_list[0].next != NULL, NULL);
+    zassert_equal(value_list[1].propertyIdentifier, PROP_STATUS_FLAGS, NULL);
+    zassert_equal(value_list[1].propertyArrayIndex, BACNET_ARRAY_ALL, NULL);
+    zassert_equal(value_list[1].value.context_specific, false, NULL);
+    zassert_equal(
+        value_list[1].value.tag, BACNET_APPLICATION_TAG_BIT_STRING, NULL);
+    zassert_equal(value_list[1].priority, BACNET_NO_PRIORITY, NULL);
+    zassert_equal(value_list[1].next, NULL, NULL);
+}
+
 /**
  * @}
  */
@@ -409,7 +608,8 @@ void test_main(void)
     ztest_test_suite(
         cov_tests, ztest_unit_test(testCOVNotify),
         ztest_unit_test(testCOVSubscribe),
-        ztest_unit_test(testCOVSubscribeProperty));
+        ztest_unit_test(testCOVSubscribeProperty),
+        ztest_unit_test(test_COV_Value_List_Encode));
 
     ztest_run_test_suite(cov_tests);
 }